Attachment 360871 Details for Bug 194140 – Patch

[patch] Patch

bug-194140-20190201103003.patch (text/plain), 40.28 KB, created by Wenson Hsieh on 2019-02-01 10:30:04 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-02-01 10:30:04 PST

Size: 40.28 KB

patch

obsolete

>Subversion Revision: 240715
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 11d679ebbcab423c02e3f231904033eabe96c9a0..3eade42b9c1eee66efd6a51a105d2cb6c317301c 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,89 @@
>+2019-02-01  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Consistent 1 sec hang when triggering modal alerts while handling synchronous touch events
>+        https://bugs.webkit.org/show_bug.cgi?id=194140
>+        <rdar://problem/47728098>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Currently, the UI process hangs when attempting to synchronously present modal UI from the web process while the
>+        UI process is waiting for sync IPC in the web process. While we have logic to generally mitigate IPC deadlock in
>+        this scenario by dispatching the web process' sync IPC immediately with the intention of allowing the web
>+        process to finish processing sync IPC (and consequently unblock the UI process), this fails in the scenario
>+        where the sync IPC message from the web process to the UI process requires main thread execution for an
>+        arbitrary amount of time (for instance, modal alert dialogs). In this case, we'll end up in a state where we've
>+        handled the web process' sync IPC in the UI process, yet we can't resume execution since the web process is
>+        still blocked.
>+
>+        By far the most egregious scenario in which this manifests is during synchronous gesture recognizer IPC, i.e.
>+        grabbing position information from the UI process, and handling touch events synchronously. Luckily, these are
>+        also scenarios where (1) we know sync IPC may safely time out, and (2) the presentation of modal UI from the web
>+        process should cause the gesture recognizers to fail anyways. As such, we can mitigate these scenarios in the
>+        web process by responding to the these pending sync IPC messages *before* sending our own sync IPC to the UI
>+        process.
>+
>+        Test: fast/events/touch/ios/show-modal-alert-during-touch-start.html
>+
>+        * Shared/ios/InteractionInformationAtPosition.h:
>+        (WebKit::InteractionInformationAtPosition::invalidInformation):
>+        * Shared/ios/InteractionInformationAtPosition.mm:
>+        (WebKit::InteractionInformationAtPosition::encode const):
>+        (WebKit::InteractionInformationAtPosition::decode):
>+
>+        Add a new flag to indicate whether an interaction information response can be valid. Interaction information
>+        cannot be valid in the case where the interaction information request was interrupted by certain sync IPC
>+        messages from the web process.
>+
>+        * UIProcess/API/C/WKContextConfigurationRef.cpp:
>+        (WKContextConfigurationIgnoreSynchronousMessagingTimeoutsForTesting):
>+        (WKContextConfigurationSetIgnoreSynchronousMessagingTimeoutsForTesting):
>+
>+        Add some testing SPI to ignore sync IPC timeouts, for the purposes of testing. Rather than use the existing
>+        Objective-C SPI in WKWebProcessPoolConfiguration, I decided to add C API plumbing for this flag, so that other
>+        non-Cocoa ports may also support the new layout test option to ignore sync IPC timeouts.
>+
>+        * UIProcess/API/C/WKContextConfigurationRef.h:
>+        * UIProcess/ios/WKContentViewInteraction.mm:
>+        (-[WKContentView ensurePositionInformationIsUpToDate:]):
>+        (-[WKContentView _positionInformationDidChange:]):
>+        * WebProcess/WebCoreSupport/WebChromeClient.cpp:
>+        (WebKit::WebChromeClient::runBeforeUnloadConfirmPanel):
>+        (WebKit::WebChromeClient::runJavaScriptAlert):
>+        (WebKit::WebChromeClient::runJavaScriptConfirm):
>+        (WebKit::WebChromeClient::runJavaScriptPrompt):
>+        (WebKit::WebChromeClient::print):
>+        (WebKit::WebChromeClient::exceededDatabaseQuota):
>+        (WebKit::WebChromeClient::reachedApplicationCacheOriginQuota):
>+
>+        Cancel any pending sync IPC replies prior to sending sync IPC messages to the UI process which may result in
>+        sync IPC deadlock.
>+
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::WebPage::cancelGesturesBlockedOnSynchronousReplies):
>+
>+        Add a helper to cancel pending sync messages coming in from the UI process that are being called from within
>+        gesture recognizer delegate hooks.
>+
>+        (WebKit::WebPage::touchEventSync):
>+        * WebProcess/WebPage/WebPage.h:
>+        * WebProcess/WebPage/WebPage.messages.in:
>+
>+        Change these from LegacySync to Delayed messages.
>+
>+        * WebProcess/WebPage/ios/WebPageIOS.mm:
>+        (WebKit::WebPage::getPositionInformation):
>+
>+        Make this sync IPC handler (as well as WebPage::touchEventSync) store the IPC reply during the scope of the
>+        method, and invoke the stored reply at the end of the method if it wasn't interrupted due to something calling
>+        cancelGesturesBlockedOnSynchronousReplies().
>+
>+        (WebKit::WebPage::positionInformation):
>+
>+        Refactor getPositionInformation by pulling out the logic for building an InteractionInformationAtPosition into
>+        a separate helper.
>+
>+        (WebKit::WebPage::requestPositionInformation):
>+
> 2019-01-30  Simon Fraser  <simon.fraser@apple.com>
> 
>         Add some basic geometry information to the scrolling tree
>diff --git a/Source/WebKit/Shared/ios/InteractionInformationAtPosition.h b/Source/WebKit/Shared/ios/InteractionInformationAtPosition.h
>index 71bce803c8f4d2c29d135db79e7d9aaf15d45b68..cdb454e64ab32761a6111decb73731108bb0302e 100644
>--- a/Source/WebKit/Shared/ios/InteractionInformationAtPosition.h
>+++ b/Source/WebKit/Shared/ios/InteractionInformationAtPosition.h
>@@ -39,8 +39,16 @@
> namespace WebKit {
> 
> struct InteractionInformationAtPosition {
>+    static InteractionInformationAtPosition invalidInformation()
>+    {
>+        InteractionInformationAtPosition response;
>+        response.canBeValid = false;
>+        return response;
>+    }
>+
>     InteractionInformationRequest request;
> 
>+    bool canBeValid { true };
>     bool nodeAtPositionIsFocusedElement { false };
> #if ENABLE(DATA_INTERACTION)
>     bool hasSelectionAtPosition { false };
>diff --git a/Source/WebKit/Shared/ios/InteractionInformationAtPosition.mm b/Source/WebKit/Shared/ios/InteractionInformationAtPosition.mm
>index acda81915bcd80aed92f3986aa01c273100828cd..73f3df8762820e6813980ff955651c24ed2750bc 100644
>--- a/Source/WebKit/Shared/ios/InteractionInformationAtPosition.mm
>+++ b/Source/WebKit/Shared/ios/InteractionInformationAtPosition.mm
>@@ -43,6 +43,7 @@ void InteractionInformationAtPosition::encode(IPC::Encoder& encoder) const
> {
>     encoder << request;
> 
>+    encoder << canBeValid;
>     encoder << nodeAtPositionIsFocusedElement;
> #if ENABLE(DATA_INTERACTION)
>     encoder << hasSelectionAtPosition;
>@@ -89,6 +90,9 @@ bool InteractionInformationAtPosition::decode(IPC::Decoder& decoder, Interaction
>     if (!decoder.decode(result.request))
>         return false;
> 
>+    if (!decoder.decode(result.canBeValid))
>+        return false;
>+
>     if (!decoder.decode(result.nodeAtPositionIsFocusedElement))
>         return false;
> 
>diff --git a/Source/WebKit/UIProcess/API/C/WKContextConfigurationRef.cpp b/Source/WebKit/UIProcess/API/C/WKContextConfigurationRef.cpp
>index 86d41299dfaab3d64e7c41e060c02f661cd1c37e..338c0ddd7696c32d5d5c0ae1da865bf325c1b37e 100644
>--- a/Source/WebKit/UIProcess/API/C/WKContextConfigurationRef.cpp
>+++ b/Source/WebKit/UIProcess/API/C/WKContextConfigurationRef.cpp
>@@ -138,6 +138,16 @@ void WKContextConfigurationSetFullySynchronousModeIsAllowedForTesting(WKContextC
>     toImpl(configuration)->setFullySynchronousModeIsAllowedForTesting(allowed);
> }
> 
>+bool WKContextConfigurationIgnoreSynchronousMessagingTimeoutsForTesting(WKContextConfigurationRef configuration)
>+{
>+    return toImpl(configuration)->ignoreSynchronousMessagingTimeoutsForTesting();
>+}
>+
>+void WKContextConfigurationSetIgnoreSynchronousMessagingTimeoutsForTesting(WKContextConfigurationRef configuration, bool ignore)
>+{
>+    toImpl(configuration)->setIgnoreSynchronousMessagingTimeoutsForTesting(ignore);
>+}
>+
> WKArrayRef WKContextConfigurationCopyOverrideLanguages(WKContextConfigurationRef configuration)
> {
>     return toAPI(&API::Array::createStringArray(toImpl(configuration)->overrideLanguages()).leakRef());
>diff --git a/Source/WebKit/UIProcess/API/C/WKContextConfigurationRef.h b/Source/WebKit/UIProcess/API/C/WKContextConfigurationRef.h
>index 32694172c5784d9380a3d4ca06924a5f1fd4efaa..ad3e7a172f80354eca982403a49fa885142fe575 100644
>--- a/Source/WebKit/UIProcess/API/C/WKContextConfigurationRef.h
>+++ b/Source/WebKit/UIProcess/API/C/WKContextConfigurationRef.h
>@@ -63,6 +63,9 @@ WK_EXPORT void WKContextConfigurationSetResourceLoadStatisticsDirectory(WKContex
> WK_EXPORT bool WKContextConfigurationFullySynchronousModeIsAllowedForTesting(WKContextConfigurationRef configuration);
> WK_EXPORT void WKContextConfigurationSetFullySynchronousModeIsAllowedForTesting(WKContextConfigurationRef configuration, bool allowed);
> 
>+WK_EXPORT bool WKContextConfigurationIgnoreSynchronousMessagingTimeoutsForTesting(WKContextConfigurationRef configuration);
>+WK_EXPORT void WKContextConfigurationSetIgnoreSynchronousMessagingTimeoutsForTesting(WKContextConfigurationRef configuration, bool ignore);
>+
> WK_EXPORT WKArrayRef WKContextConfigurationCopyOverrideLanguages(WKContextConfigurationRef configuration);
> WK_EXPORT void WKContextConfigurationSetOverrideLanguages(WKContextConfigurationRef configuration, WKArrayRef overrideLanguages);
> 
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index 16b06dc8abf34a130dfbb173a2a57f8df9f4508b..da37eee3dceabebe6aab5ff652ab0ad616f1771a 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -1716,7 +1716,8 @@ - (BOOL)ensurePositionInformationIsUpToDate:(WebKit::InteractionInformationReque
>     if ([self _hasValidOutstandingPositionInformationRequest:request])
>         return connection->waitForAndDispatchImmediately<Messages::WebPageProxy::DidReceivePositionInformation>(_page->pageID(), 1_s, IPC::WaitForOption::InterruptWaitingIfSyncMessageArrives);
> 
>-    _hasValidPositionInformation = _page->process().sendSync(Messages::WebPage::GetPositionInformation(request), Messages::WebPage::GetPositionInformation::Reply(_positionInformation), _page->pageID(), 1_s);
>+    bool receivedResponse = _page->process().sendSync(Messages::WebPage::GetPositionInformation(request), Messages::WebPage::GetPositionInformation::Reply(_positionInformation), _page->pageID(), 1_s);
>+    _hasValidPositionInformation = receivedResponse && _positionInformation.canBeValid;
>     
>     // FIXME: We need to clean up these handlers in the event that we are not able to collect data, or if the WebProcess crashes.
>     if (_hasValidPositionInformation)
>@@ -2237,7 +2238,7 @@ - (void)_positionInformationDidChange:(const WebKit::InteractionInformationAtPos
>     newInfo.mergeCompatibleOptionalInformation(_positionInformation);
> 
>     _positionInformation = newInfo;
>-    _hasValidPositionInformation = YES;
>+    _hasValidPositionInformation = _positionInformation.canBeValid;
>     if (_actionSheetAssistant)
>         [_actionSheetAssistant updateSheetPosition];
>     [self _invokeAndRemovePendingHandlersValidForCurrentPositionInformation];
>diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
>index 6c8245fcdf8287ef7c57885be886c77d810a81c9..044262dd8e4c00cca9d2a22f772fd52b124e851f 100644
>--- a/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
>+++ b/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
>@@ -405,6 +405,7 @@ bool WebChromeClient::runBeforeUnloadConfirmPanel(const String& message, Frame&
> 
>     HangDetectionDisabler hangDetectionDisabler;
> 
>+    m_page.cancelGesturesBlockedOnSynchronousReplies();
>     if (!WebProcess::singleton().parentProcessConnection()->sendSync(Messages::WebPageProxy::RunBeforeUnloadConfirmPanel(webFrame->frameID(), SecurityOriginData::fromFrame(&frame), message), Messages::WebPageProxy::RunBeforeUnloadConfirmPanel::Reply(shouldClose), m_page.pageID(), Seconds::infinity(), IPC::SendSyncOption::InformPlatformProcessWillSuspend))
>         return false;
> 
>@@ -451,6 +452,7 @@ void WebChromeClient::runJavaScriptAlert(Frame& frame, const String& alertText)
> 
>     HangDetectionDisabler hangDetectionDisabler;
> 
>+    m_page.cancelGesturesBlockedOnSynchronousReplies();
>     WebProcess::singleton().parentProcessConnection()->sendSync(Messages::WebPageProxy::RunJavaScriptAlert(webFrame->frameID(), SecurityOriginData::fromFrame(&frame), alertText), Messages::WebPageProxy::RunJavaScriptAlert::Reply(), m_page.pageID(), Seconds::infinity(), IPC::SendSyncOption::InformPlatformProcessWillSuspend);
> }
> 
>@@ -467,6 +469,7 @@ bool WebChromeClient::runJavaScriptConfirm(Frame& frame, const String& message)
> 
>     HangDetectionDisabler hangDetectionDisabler;
> 
>+    m_page.cancelGesturesBlockedOnSynchronousReplies();
>     bool result = false;
>     if (!WebProcess::singleton().parentProcessConnection()->sendSync(Messages::WebPageProxy::RunJavaScriptConfirm(webFrame->frameID(), SecurityOriginData::fromFrame(&frame), message), Messages::WebPageProxy::RunJavaScriptConfirm::Reply(result), m_page.pageID(), Seconds::infinity(), IPC::SendSyncOption::InformPlatformProcessWillSuspend))
>         return false;
>@@ -487,6 +490,7 @@ bool WebChromeClient::runJavaScriptPrompt(Frame& frame, const String& message, c
> 
>     HangDetectionDisabler hangDetectionDisabler;
> 
>+    m_page.cancelGesturesBlockedOnSynchronousReplies();
>     if (!WebProcess::singleton().parentProcessConnection()->sendSync(Messages::WebPageProxy::RunJavaScriptPrompt(webFrame->frameID(), SecurityOriginData::fromFrame(&frame), message, defaultValue), Messages::WebPageProxy::RunJavaScriptPrompt::Reply(result), m_page.pageID(), Seconds::infinity(), IPC::SendSyncOption::InformPlatformProcessWillSuspend))
>         return false;
> 
>@@ -714,6 +718,7 @@ void WebChromeClient::print(Frame& frame)
>     }
> #endif
> 
>+    m_page.cancelGesturesBlockedOnSynchronousReplies();
>     m_page.sendSync(Messages::WebPageProxy::PrintFrame(webFrame->frameID()), Messages::WebPageProxy::PrintFrame::Reply(), Seconds::infinity(), IPC::SendSyncOption::InformPlatformProcessWillSuspend);
> }
> 
>@@ -732,6 +737,7 @@ void WebChromeClient::exceededDatabaseQuota(Frame& frame, const String& database
>     newQuota = m_page.injectedBundleUIClient().didExceedDatabaseQuota(&m_page, securityOrigin.ptr(), databaseName, details.displayName(), currentQuota, currentOriginUsage, details.currentUsage(), details.expectedUsage());
> 
>     if (!newQuota) {
>+        m_page.cancelGesturesBlockedOnSynchronousReplies();
>         WebProcess::singleton().parentProcessConnection()->sendSync(
>             Messages::WebPageProxy::ExceededDatabaseQuota(webFrame->frameID(), originData.databaseIdentifier(), databaseName, details.displayName(), currentQuota, currentOriginUsage, details.currentUsage(), details.expectedUsage()),
>             Messages::WebPageProxy::ExceededDatabaseQuota::Reply(newQuota), m_page.pageID(), Seconds::infinity(), IPC::SendSyncOption::InformPlatformProcessWillSuspend);
>@@ -756,6 +762,8 @@ void WebChromeClient::reachedApplicationCacheOriginQuota(SecurityOrigin& origin,
>     if (!cacheStorage.calculateQuotaForOrigin(origin, currentQuota))
>         return;
> 
>+    m_page.cancelGesturesBlockedOnSynchronousReplies();
>+
>     uint64_t newQuota = 0;
>     WebProcess::singleton().parentProcessConnection()->sendSync(
>         Messages::WebPageProxy::ReachedApplicationCacheOriginQuota(origin.data().databaseIdentifier(), currentQuota, totalBytesNeeded),
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index 23a9780a8afe98180bf3703f65cc4e03f3319bff..91be173bd5e692ed2610f1cdb8aa523683e0e6b7 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -261,6 +261,8 @@
> #endif
> 
> #if PLATFORM(IOS_FAMILY)
>+#include "InteractionInformationAtPosition.h"
>+#include "InteractionInformationRequest.h"
> #include "RemoteLayerTreeDrawingArea.h"
> #include <CoreGraphics/CoreGraphics.h>
> #include <WebCore/Icon.h>
>@@ -2723,6 +2725,19 @@ void WebPage::requestFontAttributesAtSelectionStart(CallbackID callbackID)
>     send(Messages::WebPageProxy::FontAttributesCallback(attributes, callbackID));
> }
> 
>+void WebPage::cancelGesturesBlockedOnSynchronousReplies()
>+{
>+#if ENABLE(IOS_TOUCH_EVENTS)
>+    if (auto reply = WTFMove(m_pendingSynchronousTouchEventReply))
>+        reply(true);
>+#endif
>+
>+#if PLATFORM(IOS_FAMILY)
>+    if (auto reply = WTFMove(m_pendingSynchronousPositionInformationReply))
>+        reply(InteractionInformationAtPosition::invalidInformation());
>+#endif
>+}
>+
> #if ENABLE(TOUCH_EVENTS)
> static bool handleTouchEvent(const WebTouchEvent& touchEvent, Page* page)
> {
>@@ -2744,13 +2759,19 @@ void WebPage::dispatchTouchEvent(const WebTouchEvent& touchEvent, bool& handled)
>     updatePotentialTapSecurityOrigin(touchEvent, handled);
> }
> 
>-void WebPage::touchEventSync(const WebTouchEvent& touchEvent, bool& handled)
>+void WebPage::touchEventSync(const WebTouchEvent& touchEvent, CompletionHandler<void(bool)>&& reply)
> {
>+    m_pendingSynchronousTouchEventReply = WTFMove(reply);
>+
>     EventDispatcher::TouchEventQueue queuedEvents;
>     WebProcess::singleton().eventDispatcher().getQueuedTouchEventsForPage(*this, queuedEvents);
>     dispatchAsynchronousTouchEvents(queuedEvents);
> 
>+    bool handled = true;
>     dispatchTouchEvent(touchEvent, handled);
>+
>+    if (auto reply = WTFMove(m_pendingSynchronousTouchEventReply))
>+        reply(handled);
> }
> 
> void WebPage::updatePotentialTapSecurityOrigin(const WebTouchEvent& touchEvent, bool wasHandled)
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
>index 1be5906c95d518d3651b9a76c326b100d2f76115..3284c61f499bd66f3af8361c5b4577504175147d 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.h
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.h
>@@ -596,6 +596,8 @@ public:
>     void viewportPropertiesDidChange(const WebCore::ViewportArguments&);
>     void executeEditCommandWithCallback(const String&, const String& argument, CallbackID);
> 
>+    void cancelGesturesBlockedOnSynchronousReplies();
>+
> #if PLATFORM(IOS_FAMILY)
>     WebCore::FloatSize screenSize() const;
>     WebCore::FloatSize availableScreenSize() const;
>@@ -647,7 +649,7 @@ public:
>     void syncApplyAutocorrection(const String& correction, const String& originalText, bool& correctionApplied);
>     void requestAutocorrectionContext(CallbackID);
>     void getAutocorrectionContext(String& beforeText, String& markedText, String& selectedText, String& afterText, uint64_t& location, uint64_t& length);
>-    void getPositionInformation(const InteractionInformationRequest&, InteractionInformationAtPosition&);
>+    void getPositionInformation(const InteractionInformationRequest&, CompletionHandler<void(InteractionInformationAtPosition&&)>&&);
>     void requestPositionInformation(const InteractionInformationRequest&);
>     void startInteractionWithElementAtPosition(const WebCore::IntPoint&);
>     void stopInteraction();
>@@ -1178,6 +1180,9 @@ private:
>     void resetTextAutosizing();
>     WebCore::VisiblePosition visiblePositionInFocusedNodeForPoint(const WebCore::Frame&, const WebCore::IntPoint&, bool isInteractingWithFocusedElement);
>     RefPtr<WebCore::Range> rangeForGranularityAtPoint(WebCore::Frame&, const WebCore::IntPoint&, uint32_t granularity, bool isInteractingWithFocusedElement);
>+
>+    void sendPositionInformation(InteractionInformationAtPosition&&);
>+    InteractionInformationAtPosition positionInformation(const InteractionInformationRequest&);
> #endif
> 
> #if PLATFORM(IOS_FAMILY) && ENABLE(DATA_INTERACTION)
>@@ -1234,7 +1239,7 @@ private:
>     void keyEvent(const WebKeyboardEvent&);
> 
> #if ENABLE(IOS_TOUCH_EVENTS)
>-    void touchEventSync(const WebTouchEvent&, bool& handled);
>+    void touchEventSync(const WebTouchEvent&, CompletionHandler<void(bool)>&&);
>     void updatePotentialTapSecurityOrigin(const WebTouchEvent&, bool wasHandled);
> #elif ENABLE(TOUCH_EVENTS)
>     void touchEvent(const WebTouchEvent&);
>@@ -1695,6 +1700,10 @@ private:
>     RefPtr<WebCore::Element> m_focusedElement;
>     bool m_hasPendingBlurNotification { false };
>     bool m_hasPendingEditorStateUpdate { false };
>+
>+#if ENABLE(IOS_TOUCH_EVENTS)
>+    CompletionHandler<void(bool)> m_pendingSynchronousTouchEventReply;
>+#endif
>     
> #if PLATFORM(IOS_FAMILY)
>     RefPtr<WebCore::Range> m_currentWordRange;
>@@ -1740,6 +1749,8 @@ private:
>     Optional<DynamicViewportSizeUpdateID> m_pendingDynamicViewportSizeUpdateID;
>     double m_lastTransactionPageScaleFactor { 0 };
>     uint64_t m_lastTransactionIDWithScaleChange { 0 };
>+
>+    CompletionHandler<void(InteractionInformationAtPosition&&)> m_pendingSynchronousPositionInformationReply;
> #endif
> 
>     WebCore::Timer m_layerVolatilityTimer;
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>index 47e9ce6ba7c6992b1d8796b27e0dc898a702f7e8..414f0182429a87c1d209679d26a8f909bf42541a 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>@@ -80,7 +80,7 @@ messages -> WebPage LegacyReceiver {
>     SyncApplyAutocorrection(String correction, String originalText) -> (bool autocorrectionApplied) LegacySync
>     RequestAutocorrectionContext(WebKit::CallbackID callbackID)
>     GetAutocorrectionContext() -> (String beforeContext, String markedText, String selectedText, String afterContext, uint64_t location, uint64_t length) LegacySync
>-    GetPositionInformation(struct WebKit::InteractionInformationRequest request) -> (struct WebKit::InteractionInformationAtPosition information) LegacySync
>+    GetPositionInformation(struct WebKit::InteractionInformationRequest request) -> (struct WebKit::InteractionInformationAtPosition information) Delayed
>     RequestPositionInformation(struct WebKit::InteractionInformationRequest request)
>     StartInteractionWithElementAtPosition(WebCore::IntPoint point)
>     StopInteraction()
>@@ -121,7 +121,7 @@ messages -> WebPage LegacyReceiver {
> #endif
> 
> #if ENABLE(IOS_TOUCH_EVENTS)
>-    TouchEventSync(WebKit::WebTouchEvent event) -> (bool handled) LegacySync
>+    TouchEventSync(WebKit::WebTouchEvent event) -> (bool handled) Delayed
> #endif
> #if !ENABLE(IOS_TOUCH_EVENTS) && ENABLE(TOUCH_EVENTS)
>     TouchEvent(WebKit::WebTouchEvent event)
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index d894467266bb50bf50575bedcbdddd672638e9f2..5d1b1dd90697cf80c4819a2a3e6964041b8eb825 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>@@ -2085,8 +2085,19 @@ static inline bool isAssistableElement(Element& node)
>     return node.isContentEditable();
> }
> 
>-void WebPage::getPositionInformation(const InteractionInformationRequest& request, InteractionInformationAtPosition& info)
>+void WebPage::getPositionInformation(const InteractionInformationRequest& request, CompletionHandler<void(InteractionInformationAtPosition&&)>&& reply)
> {
>+    m_pendingSynchronousPositionInformationReply = WTFMove(reply);
>+
>+    auto information = positionInformation(request);
>+
>+    if (auto reply = WTFMove(m_pendingSynchronousPositionInformationReply))
>+        reply(WTFMove(information));
>+}
>+
>+InteractionInformationAtPosition WebPage::positionInformation(const InteractionInformationRequest& request)
>+{
>+    InteractionInformationAtPosition info;
>     info.request = request;
> 
>     FloatPoint adjustedPoint;
>@@ -2262,14 +2273,13 @@ void WebPage::getPositionInformation(const InteractionInformationRequest& reques
>     info.hasSelectionAtPosition = m_page->hasSelectionAtPosition(adjustedPoint);
> #endif
>     info.adjustedPointForNodeRespondingToClickEvents = adjustedPoint;
>+
>+    return info;
> }
> 
> void WebPage::requestPositionInformation(const InteractionInformationRequest& request)
> {
>-    InteractionInformationAtPosition info;
>-
>-    getPositionInformation(request, info);
>-    send(Messages::WebPageProxy::DidReceivePositionInformation(info));
>+    send(Messages::WebPageProxy::DidReceivePositionInformation(positionInformation(request)));
> }
> 
> void WebPage::startInteractionWithElementAtPosition(const WebCore::IntPoint& point)
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 9b48791eb108c0b3fb78af7d052d7b6618f23b98..b9d14285289975affabc2a334ee2cd4225a3edef 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,45 @@
>+2019-02-01  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Consistent 1 sec hang when triggering modal alerts while handling synchronous touch events
>+        https://bugs.webkit.org/show_bug.cgi?id=194140
>+        <rdar://problem/47728098>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
>+        * WebKitTestRunner/InjectedBundle/TestRunner.cpp:
>+        (WTR::TestRunner::setShouldDismissJavaScriptAlertsAsynchronously):
>+
>+        Add a new TestRunner hook to make modal JavaScript alerts dismiss asynchronously. This is used by the new layout
>+        test to induce an IPC deadlock when presenting a modal alert during touch start.
>+
>+        * WebKitTestRunner/InjectedBundle/TestRunner.h:
>+        * WebKitTestRunner/TestController.cpp:
>+        (WTR::runJavaScriptAlert):
>+
>+        Add a client callback function for running JavaScript alerts.
>+
>+        (WTR::TestController::createOtherPage):
>+        (WTR::TestController::generateContextConfiguration const):
>+
>+        Add a test option to disable IPC timeouts for a layout test. This forces the test to reliably time out without
>+        the fix in this patch.
>+
>+        (WTR::TestController::createWebViewWithOptions):
>+
>+        Plumb TestOptions to generateContextConfiguration.
>+
>+        (WTR::TestController::resetPreferencesToConsistentValues):
>+        (WTR::TestController::resetStateToConsistentValues):
>+        (WTR::updateTestOptionsFromTestHeader):
>+        (WTR::TestController::setShouldDismissJavaScriptAlertsAsynchronously):
>+        (WTR::TestController::handleJavaScriptAlert):
>+        * WebKitTestRunner/TestController.h:
>+        * WebKitTestRunner/TestInvocation.cpp:
>+        (WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):
>+        * WebKitTestRunner/TestOptions.h:
>+        (WTR::TestOptions::hasSameInitializationOptions const):
>+
> 2019-01-30  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r240708.
>diff --git a/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl b/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl
>index f671a0abe7e65bd71542df20fd77818a697eed1f..e4f2cf850ea8881e80b9bb1db1050989aeea8453 100644
>--- a/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl
>+++ b/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl
>@@ -329,6 +329,9 @@ interface TestRunner {
>     // Open panel
>     void setOpenPanelFiles(object filesArray);
> 
>+    // Modal alerts
>+    void setShouldDismissJavaScriptAlertsAsynchronously(boolean value);
>+
>     void setWebRTCMDNSICECandidatesEnabled(boolean value);
>     void setWebRTCUnifiedPlanEnabled(boolean value);
>     void setCustomUserAgent(DOMString userAgent);
>diff --git a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
>index a659d169aef2ff0f6fd2dd7806137e2631a4e526..0ec92eb4918348db6e5962bf6d3494adef69f52d 100644
>--- a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
>+++ b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
>@@ -2715,4 +2715,11 @@ unsigned long TestRunner::serverTrustEvaluationCallbackCallsCount()
>     return WKUInt64GetValue(adoptWK(static_cast<WKUInt64Ref>(returnData)).get());
> }
> 
>+void TestRunner::setShouldDismissJavaScriptAlertsAsynchronously(bool shouldDismissAsynchronously)
>+{
>+    WKRetainPtr<WKStringRef> messageName(AdoptWK, WKStringCreateWithUTF8CString("ShouldDismissJavaScriptAlertsAsynchronously"));
>+    WKRetainPtr<WKBooleanRef> messageBody(AdoptWK, WKBooleanCreate(shouldDismissAsynchronously));
>+    WKBundlePostSynchronousMessage(InjectedBundle::singleton().bundle(), messageName.get(), messageBody.get(), nullptr);
>+}
>+
> } // namespace WTR
>diff --git a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h
>index 94a9a153fdef7c6c10ac744ad267f5c67c526792..372c9f078855db2fb19cf850f46b7e93b7b8b90f 100644
>--- a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h
>+++ b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h
>@@ -448,6 +448,9 @@ public:
>     // Open panel
>     void setOpenPanelFiles(JSValueRef);
> 
>+    // Modal alerts
>+    void setShouldDismissJavaScriptAlertsAsynchronously(bool);
>+
>     void terminateNetworkProcess();
>     void terminateServiceWorkerProcess();
> 
>diff --git a/Tools/WebKitTestRunner/TestController.cpp b/Tools/WebKitTestRunner/TestController.cpp
>index 92e160a615890d336924c560f433ab0ed255a944..98bc693f7fc48879f0419299d283f79514b901ef 100644
>--- a/Tools/WebKitTestRunner/TestController.cpp
>+++ b/Tools/WebKitTestRunner/TestController.cpp
>@@ -239,6 +239,11 @@ static void decidePolicyForUserMediaPermissionRequest(WKPageRef, WKFrameRef fram
>     TestController::singleton().handleUserMediaPermissionRequest(frame, userMediaDocumentOrigin, topLevelDocumentOrigin, permissionRequest);
> }
> 
>+static void runJavaScriptAlert(WKPageRef page, WKStringRef alertText, WKFrameRef frame, WKSecurityOriginRef securityOrigin, WKPageRunJavaScriptAlertResultListenerRef listener, const void *clientInfo)
>+{
>+    TestController::singleton().handleJavaScriptAlert(listener);
>+}
>+
> static void checkUserMediaPermissionForOrigin(WKPageRef, WKFrameRef frame, WKSecurityOriginRef userMediaDocumentOrigin, WKSecurityOriginRef topLevelDocumentOrigin, WKUserMediaPermissionCheckRef checkRequest, const void*)
> {
>     TestController::singleton().handleCheckOfUserMediaPermissionForOrigin(frame, userMediaDocumentOrigin, topLevelDocumentOrigin, checkRequest);
>@@ -326,7 +331,7 @@ WKPageRef TestController::createOtherPage(PlatformWebView* parentView, WKPageCon
>         0, // runJavaScriptPrompt
>         0, // mediaSessionMetadataDidChange
>         createOtherPage,
>-        0, // runJavaScriptAlert
>+        runJavaScriptAlert,
>         0, // runJavaScriptConfirm
>         0, // runJavaScriptPrompt
>         checkUserMediaPermissionForOrigin,
>@@ -442,11 +447,12 @@ void TestController::initialize(int argc, const char* argv[])
>     m_pageGroup.adopt(WKPageGroupCreateWithIdentifier(pageGroupIdentifier.get()));
> }
> 
>-WKRetainPtr<WKContextConfigurationRef> TestController::generateContextConfiguration() const
>+WKRetainPtr<WKContextConfigurationRef> TestController::generateContextConfiguration(const TestOptions& options) const
> {
>     auto configuration = adoptWK(WKContextConfigurationCreate());
>     WKContextConfigurationSetInjectedBundlePath(configuration.get(), injectedBundlePath());
>     WKContextConfigurationSetFullySynchronousModeIsAllowedForTesting(configuration.get(), true);
>+    WKContextConfigurationSetIgnoreSynchronousMessagingTimeoutsForTesting(configuration.get(), options.ignoreSynchronousMessagingTimeoutsForTesting);
> 
>     if (const char* dumpRenderTreeTemp = libraryPathForTesting()) {
>         String temporaryFolder = String::fromUTF8(dumpRenderTreeTemp);
>@@ -534,7 +540,7 @@ WKRetainPtr<WKPageConfigurationRef> TestController::generatePageConfiguration(WK
> 
> void TestController::createWebViewWithOptions(const TestOptions& options)
> {
>-    auto contextConfiguration = generateContextConfiguration();
>+    auto contextConfiguration = generateContextConfiguration(options);
> 
>     WKRetainPtr<WKMutableArrayRef> overrideLanguages = adoptWK(WKMutableArrayCreate());
>     for (auto& language : options.overrideLanguages)
>@@ -614,7 +620,7 @@ void TestController::createWebViewWithOptions(const TestOptions& options)
>         0, // runJavaScriptPrompt
>         0, // mediaSessionMetadataDidChange
>         createOtherPage,
>-        0, // runJavaScriptAlert
>+        runJavaScriptAlert,
>         0, // runJavaScriptConfirm
>         0, // runJavaScriptPrompt
>         checkUserMediaPermissionForOrigin,
>@@ -824,8 +830,6 @@ void TestController::resetPreferencesToConsistentValues(const TestOptions& optio
> 
>     WKPreferencesSetWebSQLDisabled(preferences, false);
> 
>-    m_serverTrustEvaluationCallbackCallsCount = 0;
>-
>     platformResetPreferencesToConsistentValues();
> }
> 
>@@ -957,6 +961,8 @@ bool TestController::resetStateToConsistentValues(const TestOptions& options, Re
>     statisticsResetToConsistentState();
> 
>     m_didReceiveServerRedirectForProvisionalNavigation = false;
>+    m_serverTrustEvaluationCallbackCallsCount = 0;
>+    m_shouldDismissJavaScriptAlertsAsynchronously = false;
> 
>     // Reset main page back to about:blank
>     m_doneResetting = false;
>@@ -1273,6 +1279,8 @@ static void updateTestOptionsFromTestHeader(TestOptions& testOptions, const std:
>             testOptions.enableUndoManagerAPI = parseBooleanTestHeaderValue(value);
>         else if (key == "contentInset.top")
>             testOptions.contentInsetTop = std::stod(value);
>+        else if (key == "ignoreSynchronousMessagingTimeoutsForTesting")
>+            testOptions.ignoreSynchronousMessagingTimeoutsForTesting = parseBooleanTestHeaderValue(value);
>         pairStart = pairEnd + 1;
>     }
> }
>@@ -2325,6 +2333,25 @@ void TestController::resetUserMediaPermission()
>     m_isUserMediaPermissionSet = false;
> }
> 
>+void TestController::setShouldDismissJavaScriptAlertsAsynchronously(bool value)
>+{
>+    m_shouldDismissJavaScriptAlertsAsynchronously = value;
>+}
>+
>+void TestController::handleJavaScriptAlert(WKPageRunJavaScriptAlertResultListenerRef listener)
>+{
>+    if (!m_shouldDismissJavaScriptAlertsAsynchronously) {
>+        WKPageRunJavaScriptAlertResultListenerCall(listener);
>+        return;
>+    }
>+
>+    WKRetain(listener);
>+    callOnMainThread([listener] {
>+        WKPageRunJavaScriptAlertResultListenerCall(listener);
>+        WKRelease(listener);
>+    });
>+}
>+
> class OriginSettings : public RefCounted<OriginSettings> {
> public:
>     explicit OriginSettings()
>diff --git a/Tools/WebKitTestRunner/TestController.h b/Tools/WebKitTestRunner/TestController.h
>index 25ec904be42b2cdc3094e1ebeab3d7f97d4a95e2..e93917125f2ed1e70ed9f535da09957f83ecb03a 100644
>--- a/Tools/WebKitTestRunner/TestController.h
>+++ b/Tools/WebKitTestRunner/TestController.h
>@@ -284,9 +284,12 @@ public:
>     bool canDoServerTrustEvaluationInNetworkProcess() const;
>     uint64_t serverTrustEvaluationCallbackCallsCount() const { return m_serverTrustEvaluationCallbackCallsCount; }
> 
>+    void setShouldDismissJavaScriptAlertsAsynchronously(bool);
>+    void handleJavaScriptAlert(WKPageRunJavaScriptAlertResultListenerRef);
>+
> private:
>     WKRetainPtr<WKPageConfigurationRef> generatePageConfiguration(WKContextConfigurationRef);
>-    WKRetainPtr<WKContextConfigurationRef> generateContextConfiguration() const;
>+    WKRetainPtr<WKContextConfigurationRef> generateContextConfiguration(const TestOptions&) const;
>     void initialize(int argc, const char* argv[]);
>     void createWebViewWithOptions(const TestOptions&);
>     void run();
>@@ -539,6 +542,7 @@ private:
>     HashMap<uint64_t, AbandonedDocumentInfo> m_abandonedDocumentInfo;
> 
>     uint64_t m_serverTrustEvaluationCallbackCallsCount { 0 };
>+    bool m_shouldDismissJavaScriptAlertsAsynchronously { false };
> };
> 
> struct TestCommand {
>diff --git a/Tools/WebKitTestRunner/TestInvocation.cpp b/Tools/WebKitTestRunner/TestInvocation.cpp
>index 429ce29b520118df248a031af7e45382b3587474..89656fbcdaeebbf5d46435be531df9b876596334 100644
>--- a/Tools/WebKitTestRunner/TestInvocation.cpp
>+++ b/Tools/WebKitTestRunner/TestInvocation.cpp
>@@ -1539,6 +1539,13 @@ WKRetainPtr<WKTypeRef> TestInvocation::didReceiveSynchronousMessageFromInjectedB
>         return result;
>     }
> 
>+    if (WKStringIsEqualToUTF8CString(messageName, "ShouldDismissJavaScriptAlertsAsynchronously")) {
>+        ASSERT(WKGetTypeID(messageBody) == WKBooleanGetTypeID());
>+        WKBooleanRef value = static_cast<WKBooleanRef>(messageBody);
>+        TestController::singleton().setShouldDismissJavaScriptAlertsAsynchronously(WKBooleanGetValue(value));
>+        return nullptr;
>+    }
>+
>     ASSERT_NOT_REACHED();
>     return nullptr;
> }
>diff --git a/Tools/WebKitTestRunner/TestOptions.h b/Tools/WebKitTestRunner/TestOptions.h
>index 614ce7b5818597ea9a4813adc7b2e1fc46a46079..f91dd50c9d206b03d3f380b75a81d09523eefcbc 100644
>--- a/Tools/WebKitTestRunner/TestOptions.h
>+++ b/Tools/WebKitTestRunner/TestOptions.h
>@@ -68,6 +68,7 @@ struct TestOptions {
>     bool enableEditableImages { false };
>     bool editable { false };
>     bool enableUndoManagerAPI { false };
>+    bool ignoreSynchronousMessagingTimeoutsForTesting { false };
> 
>     double contentInsetTop { 0 };
> 
>@@ -116,7 +117,8 @@ struct TestOptions {
>             || enableEditableImages != options.enableEditableImages
>             || editable != options.editable
>             || enableUndoManagerAPI != options.enableUndoManagerAPI
>-            || contentInsetTop != options.contentInsetTop)
>+            || contentInsetTop != options.contentInsetTop
>+            || ignoreSynchronousMessagingTimeoutsForTesting != options.ignoreSynchronousMessagingTimeoutsForTesting)
>             return false;
> 
>         if (experimentalFeatures != options.experimentalFeatures)
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 89a857cc3fc1ea6defa62e5f08d8f3620a2e68df..a95d86f3bf1b4546c6719791bcbcb8adc4e17ee8 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-01-31  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Consistent 1 sec hang when triggering modal alerts while handling synchronous touch events
>+        https://bugs.webkit.org/show_bug.cgi?id=194140
>+        <rdar://problem/47728098>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a test that induces sync IPC deadlock by presenting a modal alert while handling touch start. This test
>+        forces sync IPC timeouts to be disabled, and passes if we do not time out while handling a touch.
>+
>+        * fast/events/touch/ios/show-modal-alert-during-touch-start-expected.txt: Added.
>+        * fast/events/touch/ios/show-modal-alert-during-touch-start.html: Added.
>+
> 2019-01-30  Simon Fraser  <simon.fraser@apple.com>
> 
>         Add some basic geometry information to the scrolling tree
>diff --git a/LayoutTests/fast/events/touch/ios/show-modal-alert-during-touch-start-expected.txt b/LayoutTests/fast/events/touch/ios/show-modal-alert-during-touch-start-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..681709ac00093e0375d6bea8df64d69697799f8c
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/show-modal-alert-during-touch-start-expected.txt
>@@ -0,0 +1,3 @@
>+ALERT: This is a modal alert.
>+Tap to show an alert
>+This test verifies that presenting a modal alert while handling a synchronous touch event doesn't cause the application process to hang. To verify manually, tap the red area above, and check that the JavaScript alert is presented with no significant delay.
>diff --git a/LayoutTests/fast/events/touch/ios/show-modal-alert-during-touch-start.html b/LayoutTests/fast/events/touch/ios/show-modal-alert-during-touch-start.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..9b3abc04b5250d42bc649b787d9ca1e23e2fadfd
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/show-modal-alert-during-touch-start.html
>@@ -0,0 +1,38 @@
>+<!DOCTYPE html> 
>+<html>
>+<head>
>+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
>+<script src="../../../../resources/ui-helper.js"></script>
>+<style>
>+body, html {
>+    width: 100%;
>+    height: 100%;
>+    margin: 0;
>+}
>+
>+#target {
>+    background-color: tomato;
>+    color: white;
>+    width: 100%;
>+    height: 200px;
>+    text-align: center;
>+    font-size: 20px;
>+}
>+</style>
>+</head>
>+<body>
>+<div id="target">Tap to show an alert</div>
>+<p>This test verifies that presenting a modal alert while handling a synchronous touch event doesn't cause the application process to hang. To verify manually, tap the red area above, and check that the JavaScript alert is presented with no significant delay.</p>
>+<script>
>+if (window.testRunner) {
>+    testRunner.setShouldDismissJavaScriptAlertsAsynchronously(true);
>+    testRunner.waitUntilDone();
>+    testRunner.dumpAsText();
>+}
>+
>+target.addEventListener("touchstart", () => alert("This is a modal alert."));
>+target.addEventListener("touchend", () => testRunner.notifyDone());
>+addEventListener("load", async () => await UIHelper.activateAt(100, 100));
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 194140: 360832 | 360843 | 360871 | 360884 | 360892