Attachment 362478 Details for Bug 194601 – Patch

[patch] Patch

bug-194601-20190219204854.patch (text/plain), 9.43 KB, created by Wenson Hsieh on 2019-02-19 20:48:55 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-02-19 20:48:55 PST

Size: 9.43 KB

patch

obsolete

>Subversion Revision: 241649
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 8522ca45ea81707e2fc853bee7a09b30c9b6015e..e75f21076369311a34ab6d087186c479e04452ca 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,48 @@
>+2019-02-19  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        REGRESSION: [ iOS ] Layout Test editing/input/ios/rtl-keyboard-input-on-focus.html is a Timeout
>+        https://bugs.webkit.org/show_bug.cgi?id=194601
>+        <rdar://problem/48080316>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Following r241311, if a web view becomes first responder and is then moved offscreen (or obscured, hidden, or in
>+        the case of WebKitTestRunner, its UIWindow loses its status as keyWindow), we end up holding on to the input
>+        view update deferral token indefinitely, waiting for the current focused element to be blurred or refocused.
>+
>+        This also manifests other user-facing bugs, the most common of which is the keyboard occasionally remaining
>+        onscreen after typing a URL in the unified field in MobileSafari and hitting Return, in the case where there is
>+        no autofocused element on the page.
>+
>+        To fix this, we ensure that the input view deferral token is cleared out after the web process has had a chance
>+        to process the activity state change (due to becoming the first responder) and notify the UI process about the
>+        currently focused element. Since the activity state change is dispatched on a runloop timer, we wait until at
>+        least one round trip to the web process and back *after* the next runloop. If the input view deferral token has
>+        not yet been cleared out by this point, do clear it out here in the completion block.
>+
>+        Changes covered by currently failing layout tests.
>+
>+        * UIProcess/AuxiliaryProcessProxy.h:
>+        (WebKit::AuxiliaryProcessProxy::sendWithAsyncReply):
>+
>+        Add the destination identifier as an optional argument when sending with async reply.
>+
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::cleanUpAfterActivityStateChangeDueToBecomingFirstResponder):
>+
>+        Add a new async IPC message to notify the web process after the web view has dispatched an activity state change
>+        message after becoming the first responder. While the web process currently doesn't need to perform any work
>+        here, its response to the UI process allows the UI process to know when it's safe to remove the input view
>+        deferral token.
>+
>+        * UIProcess/WebPageProxy.h:
>+        * UIProcess/ios/WKContentViewInteraction.mm:
>+        (-[WKContentView becomeFirstResponderForWebView]):
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::WebPage::cleanUpAfterActivityStateChangeDueToBecomingFirstResponder):
>+        * WebProcess/WebPage/WebPage.h:
>+        * WebProcess/WebPage/WebPage.messages.in:
>+
> 2019-02-15  Youenn Fablet  <youenn@apple.com>
> 
>         NetworkDataTask should check its client before calling shouldCaptureExtraNetworkLoadMetrics
>diff --git a/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h b/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h
>index fcd3fabd46fb13524d46bd17f4a9070a7a46ca66..fad69bcc44bf985985ec26d21ea101b70778793b 100644
>--- a/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h
>+++ b/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h
>@@ -50,7 +50,7 @@ public:
> 
>     template<typename T> bool send(T&& message, uint64_t destinationID, OptionSet<IPC::SendOption> sendOptions = { });
>     template<typename T> bool sendSync(T&& message, typename T::Reply&&, uint64_t destinationID, Seconds timeout = 1_s, OptionSet<IPC::SendSyncOption> sendSyncOptions = { });
>-    template<typename T, typename... Args> void sendWithAsyncReply(T&&, CompletionHandler<void(Args...)>&&);
>+    template<typename T, typename... Args> void sendWithAsyncReply(T&&, CompletionHandler<void(Args...)>&&, uint64_t destinationID = 0);
> 
>     IPC::Connection* connection() const
>     {
>@@ -133,14 +133,14 @@ bool AuxiliaryProcessProxy::sendSync(U&& message, typename U::Reply&& reply, uin
> }
> 
> template<typename T, typename... Args>
>-void AuxiliaryProcessProxy::sendWithAsyncReply(T&& message, CompletionHandler<void(Args...)>&& completionHandler)
>+void AuxiliaryProcessProxy::sendWithAsyncReply(T&& message, CompletionHandler<void(Args...)>&& completionHandler, uint64_t destinationID)
> {
>     if (!m_connection) {
>         T::cancelReply(WTFMove(completionHandler));
>         return;
>     }
> 
>-    connection()->sendWithAsyncReply(std::forward<T>(message), WTFMove(completionHandler));
>+    connection()->sendWithAsyncReply(std::forward<T>(message), WTFMove(completionHandler), destinationID);
> }
>     
> } // namespace WebKit
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index f0a3d4c8fafaa111337f87e2dd058c81260baade..9f8fb98e335b6d12d9ec86099f0d2ca214c73eb9 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -1687,6 +1687,15 @@ void WebPageProxy::activityStateDidChange(OptionSet<ActivityState::Flag> mayHave
> #endif
> }
> 
>+void WebPageProxy::cleanUpAfterActivityStateChangeDueToBecomingFirstResponder(CompletionHandler<void()>&& callback)
>+{
>+    if (!isValid()) {
>+        callback();
>+        return;
>+    }
>+    m_process->sendWithAsyncReply(Messages::WebPage::CleanUpAfterActivityStateChangeDueToBecomingFirstResponder(), WTFMove(callback), m_pageID);
>+}
>+
> void WebPageProxy::viewDidLeaveWindow()
> {
>     closeOverlayedViews();
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index b4d8af3cd2e397441e6380cc4c6d1e0a839d649b..94eec453083614b1992d29e53bc50ce98fa2588f 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -603,6 +603,8 @@ public:
>     void requestFontAttributesAtSelectionStart(Function<void(const WebCore::FontAttributes&, CallbackBase::Error)>&&);
>     void fontAttributesCallback(const WebCore::FontAttributes&, CallbackID);
> 
>+    void cleanUpAfterActivityStateChangeDueToBecomingFirstResponder(CompletionHandler<void()>&&);
>+
> #if PLATFORM(IOS_FAMILY)
>     double displayedContentScale() const { return m_lastVisibleContentRectUpdate.scale(); }
>     const WebCore::FloatRect& exposedContentRect() const { return m_lastVisibleContentRectUpdate.exposedContentRect(); }
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index e7587403c96aae3d9a5490240326e864f504b688..2e4533e582d57a70f153d951334f4ba24db07482 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -1107,6 +1107,19 @@ - (BOOL)becomeFirstResponderForWebView
> 
>         if ([self canShowNonEmptySelectionView])
>             [_textSelectionAssistant activateSelection];
>+
>+        dispatch_async(dispatch_get_main_queue(), [weakSelf = WeakObjCPtr<WKContentView>(self)] {
>+            if (!weakSelf)
>+                return;
>+
>+            auto strongSelf = weakSelf.get();
>+            strongSelf->_page->cleanUpAfterActivityStateChangeDueToBecomingFirstResponder([weakSelf = WeakObjCPtr<WKContentView>(strongSelf.get())] {
>+                if (!weakSelf)
>+                    return;
>+
>+                weakSelf.get()->_inputViewUpdateDeferrer = nullptr;
>+            });
>+        });
>     } else
>         _inputViewUpdateDeferrer = nullptr;
> 
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index c932b51625939c99cfd6a980fffab3218c94d87c..e8f01676c5ea17fdc56104c9e48d0e38faa40fb8 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -1141,6 +1141,11 @@ PluginView* WebPage::pluginViewForFrame(Frame* frame)
>     return static_cast<PluginView*>(document.pluginWidget());
> }
> 
>+void WebPage::cleanUpAfterActivityStateChangeDueToBecomingFirstResponder(CompletionHandler<void()>&& callback)
>+{
>+    callback();
>+}
>+
> void WebPage::executeEditingCommand(const String& commandName, const String& argument)
> {
>     Frame& frame = m_page->focusController().focusedOrMainFrame();
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
>index ba949ba709f1519b1c860629bbf6ea9644be1e10..97565db03422ed09c2e688eaf1bbaf8a7d69f91c 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.h
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.h
>@@ -594,6 +594,8 @@ public:
>     void viewportPropertiesDidChange(const WebCore::ViewportArguments&);
>     void executeEditCommandWithCallback(const String&, const String& argument, CallbackID);
> 
>+    void cleanUpAfterActivityStateChangeDueToBecomingFirstResponder(CompletionHandler<void()>&&);
>+
> #if PLATFORM(IOS_FAMILY)
>     WebCore::FloatSize screenSize() const;
>     WebCore::FloatSize availableScreenSize() const;
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>index 2d5b7b5a6bcd09307183f449fad16c27cd64b681..cfe91321f5ecf4b7588d11ce752c214a54e9ac1c 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>@@ -110,6 +110,8 @@ messages -> WebPage LegacyReceiver {
>     HardwareKeyboardAvailabilityChanged()
> #endif
> 
>+    CleanUpAfterActivityStateChangeDueToBecomingFirstResponder() -> () Async
>+
>     SetControlledByAutomation(bool controlled)
> 
>     ConnectInspector(String targetId, Inspector::FrontendChannel::ConnectionType connectionType)

Actions: View | Formatted Diff | Diff

Attachments on bug 194601: 362478 | 362531 | 362548