Attachment 368841 Details for Bug 197532 – Patch

[patch] Patch

bug-197532-20190502163052.patch (text/plain), 13.35 KB, created by Wenson Hsieh on 2019-05-02 16:30:53 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-05-02 16:30:53 PDT

Size: 13.35 KB

patch

obsolete

>Subversion Revision: 244893
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 2b51c2bfae339c6b9a78171e57c3e70304a922e6..37394e75341a6e184081ce87ff2ab8b795ba4581 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,73 @@
>+2019-05-02  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        REGRESSION: Layout test editing/selection/ios/selection-after-changing-text-with-callout-menu.html is failing
>+        https://bugs.webkit.org/show_bug.cgi?id=197532
>+        <rdar://problem/50177144>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Fixes layout tests that began failing after r244546. See below for details.
>+
>+        * UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:
>+        (WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):
>+
>+        Partially reverts a change in r244546, after which we commit the layer tree and scroll before updating the
>+        page's editor state. The purpose of this change was to ensure that UI process-side element focus scrolling logic
>+        would not conflict with web-process-driven scrolling logic.
>+
>+        Instead, we split the existing logic in WebPageProxy::editorStateChanged into two pieces: one that updates the
>+        editor state (by setting m_editorState), and a second that dispatches updates to PageClient when the first
>+        editor state is received after focusing an element. During a layer tree commit in the UI process, we first
>+        update the editor state, then commit the layer tree and apply scroll position changes, and finally scroll to
>+        reveal the focused element if necessary.
>+
>+        When an editor state is delivered to the UI process in an out-of-band update (i.e. not in a layer tree commit),
>+        simply dispatch the initial editor state for a focused element immediately.
>+
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::scheduleFullEditorStateUpdate):
>+
>+        Add an IPC message to schedule an editor state update in the next remote layer tree commit. See below for more
>+        details.
>+
>+        (WebKit::WebPageProxy::editorStateChanged):
>+        * UIProcess/WebPageProxy.h:
>+        * UIProcess/ios/WKContentViewInteraction.mm:
>+        (-[WKContentView willFinishIgnoringCalloutBarFadeAfterPerformingAction]):
>+
>+        Additionally ensure that an editor state update is scheduled. This addresses a potential source of flakiness in
>+        the layout test editing/selection/ios/selection-after-changing-text-with-callout-menu.html, where an editor
>+        state update may only be scheduled after the next layout timer fires (this is the case in custom callout menu
>+        actions that change the DOM but do not otherwise trigger any editing commands).
>+
>+        In the problematic scenario, the client could make a change that triggers layout soon; but before the layout
>+        timer fires, the timer for the next remote layer tree commit could fire, such that the next layer tree commit
>+        would not contain the relevant editor state.
>+
>+        This extra step ensures that we always *schedule* an editor state update when performing a callout menu action
>+        that does not automatically dismiss, so that we can prevent the callout bar from dismissing during the correct
>+        scope.
>+
>+        * UIProcess/ios/WebPageProxyIOS.mm:
>+        (WebKit::WebPageProxy::layerTreeCommitComplete):
>+        (WebKit::WebPageProxy::setEditorState):
>+
>+        Rename editorStateChanged to setEditorState, and make the editorStateChanged codepath only executed when an
>+        out-of-band editor state update is delivered to the UI process.
>+
>+        (WebKit::WebPageProxy::dispatchDidReceiveEditorStateAfterFocus):
>+
>+        Notify the UI process that the initial editor state has been received; this prompts us to scroll to reveal the
>+        focused element, if needed.
>+
>+        (WebKit::WebPageProxy::editorStateChanged): Deleted.
>+        (WebKit::WebPageProxy::dispatchDidReceiveEditorStateAfterFocus):
>+        * UIProcess/mac/WebPageProxyMac.mm:
>+        (WebKit::WebPageProxy::setEditorState):
>+        (WebKit::WebPageProxy::editorStateChanged): Deleted.
>+        * WebProcess/WebPage/WebPage.h:
>+        * WebProcess/WebPage/WebPage.messages.in:
>+
> 2019-05-02  Timothy Hatcher  <timothy@apple.com>
> 
>         NSAttributedString conversion in a loop returns nil and WKUnknownError every other time.
>diff --git a/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm b/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm
>index 0e298f51e0b03aa3515d3eaf577165b55ccb8236..f832efdc248bbcb5989fbf75ed76a14716fa034f 100644
>--- a/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm
>+++ b/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm
>@@ -199,6 +199,9 @@ void RemoteLayerTreeDrawingAreaProxy::commitLayerTree(const RemoteLayerTreeTrans
>     m_transactionIDForPendingCACommit = layerTreeTransaction.transactionID();
>     m_activityStateChangeID = layerTreeTransaction.activityStateChangeID();
> 
>+    if (layerTreeTransaction.hasEditorState())
>+        m_webPageProxy.setEditorState(layerTreeTransaction.editorState());
>+
>     if (m_remoteLayerTreeHost->updateLayerTree(layerTreeTransaction)) {
>         if (layerTreeTransaction.transactionID() >= m_transactionIDForUnhidingContent)
>             m_webPageProxy.setRemoteLayerTreeRootNode(m_remoteLayerTreeHost->rootNode());
>@@ -253,7 +256,7 @@ void RemoteLayerTreeDrawingAreaProxy::commitLayerTree(const RemoteLayerTreeTrans
> #endif
> 
>     if (layerTreeTransaction.hasEditorState())
>-        m_webPageProxy.editorStateChanged(layerTreeTransaction.editorState());
>+        m_webPageProxy.dispatchDidReceiveEditorStateAfterFocus();
> 
>     if (auto milestones = layerTreeTransaction.newlyReachedPaintingMilestones())
>         m_webPageProxy.didReachLayoutMilestone(milestones);
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 5324051b1040e543b6e6da62c38dd9ea7397b714..c452b831185524e150383f764dcf81b7c992b1c2 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -2006,6 +2006,14 @@ void WebPageProxy::setMaintainsInactiveSelection(bool newValue)
>     m_maintainsInactiveSelection = newValue;
> }
> 
>+void WebPageProxy::scheduleFullEditorStateUpdate()
>+{
>+    if (!hasRunningProcess())
>+        return;
>+
>+    m_process->send(Messages::WebPage::ScheduleFullEditorStateUpdate(), m_pageID);
>+}
>+
> void WebPageProxy::executeEditCommand(const String& commandName, const String& argument, WTF::Function<void(CallbackBase::Error)>&& callbackFunction)
> {
>     if (!hasRunningProcess()) {
>@@ -6503,6 +6511,20 @@ void WebPageProxy::editingRangeCallback(const EditingRange& range, CallbackID ca
>     callback->performCallbackWithReturnValue(range);
> }
> 
>+void WebPageProxy::editorStateChanged(const EditorState& editorState)
>+{
>+    setEditorState(editorState);
>+    dispatchDidReceiveEditorStateAfterFocus();
>+}
>+
>+#if !PLATFORM(IOS_FAMILY)
>+
>+void WebPageProxy::dispatchDidReceiveEditorStateAfterFocus()
>+{
>+}
>+
>+#endif
>+
> #if ENABLE(APPLICATION_MANIFEST)
> void WebPageProxy::applicationManifestCallback(const Optional<WebCore::ApplicationManifest>& manifestOrNull, CallbackID callbackID)
> {
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index 37658d8c0707e2a34cfdedf05088f4320c9a9655..0ebec354f3877a7a1e8c6fbcae3d9dc7622dc611 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -1423,6 +1423,9 @@ public:
>     void createSandboxExtensionsIfNeeded(const Vector<String>& files, SandboxExtension::Handle& fileReadHandle, SandboxExtension::HandleArray& fileUploadHandles);
> #endif
>     void editorStateChanged(const EditorState&);
>+    void setEditorState(const EditorState&);
>+    void scheduleFullEditorStateUpdate();
>+    void dispatchDidReceiveEditorStateAfterFocus();
> 
> #if PLATFORM(COCOA)
>     void touchBarMenuDataRemoved();
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index 1835a71836634b4d388457de94fad3e52149be36..db1dd7611dbe415d16eb6ec17006e55a4cf302c6 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -3920,6 +3920,7 @@ - (void)endSelectionChange
> - (void)willFinishIgnoringCalloutBarFadeAfterPerformingAction
> {
>     _ignoreSelectionCommandFadeCount++;
>+    _page->scheduleFullEditorStateUpdate();
>     _page->callAfterNextPresentationUpdate([weakSelf = WeakObjCPtr<WKContentView>(self)] (auto) {
>         if (auto strongSelf = weakSelf.get())
>             strongSelf->_ignoreSelectionCommandFadeCount--;
>diff --git a/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm b/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm
>index 7f55adb1b002192c9d1c1e0f63beb7152d0270e9..ea24936ce7e53a2fa2cc01bac5e6907a755404d4 100644
>--- a/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm
>+++ b/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm
>@@ -430,6 +430,11 @@ bool WebPageProxy::updateLayoutViewportParameters(const WebKit::RemoteLayerTreeT
> void WebPageProxy::layerTreeCommitComplete()
> {
>     pageClient().layerTreeCommitComplete();
>+
>+    if (m_waitingForPostLayoutEditorStateUpdateAfterFocusingElement && !m_editorState.isMissingPostLayoutData) {
>+        pageClient().didReceiveEditorStateUpdateAfterFocus();
>+        m_waitingForPostLayoutEditorStateUpdateAfterFocusingElement = false;
>+    }
> }
> 
> void WebPageProxy::selectWithGesture(const WebCore::IntPoint point, WebCore::TextGranularity granularity, uint32_t gestureType, uint32_t gestureState, bool isInteractingWithFocusedElement, WTF::Function<void(const WebCore::IntPoint&, uint32_t, uint32_t, uint32_t, CallbackBase::Error)>&& callbackFunction)
>@@ -1102,7 +1107,7 @@ void WebPageProxy::contentSizeCategoryDidChange(const String& contentSizeCategor
>     process().send(Messages::WebPage::ContentSizeCategoryDidChange(contentSizeCategory), m_pageID);
> }
> 
>-void WebPageProxy::editorStateChanged(const EditorState& editorState)
>+void WebPageProxy::setEditorState(const EditorState& editorState)
> {
>     bool couldChangeSecureInputState = m_editorState.isInPasswordField != editorState.isInPasswordField || m_editorState.selectionIsNone;
>     
>@@ -1119,11 +1124,15 @@ void WebPageProxy::editorStateChanged(const EditorState& editorState)
>     // even during composition to support phrase boundary gesture.
>     pageClient().selectionDidChange();
>     updateFontAttributesAfterEditorStateChange();
>+}
> 
>-    if (m_waitingForPostLayoutEditorStateUpdateAfterFocusingElement && !m_editorState.isMissingPostLayoutData) {
>-        pageClient().didReceiveEditorStateUpdateAfterFocus();
>-        m_waitingForPostLayoutEditorStateUpdateAfterFocusingElement = false;
>-    }
>+void WebPageProxy::dispatchDidReceiveEditorStateAfterFocus()
>+{
>+    if (!m_waitingForPostLayoutEditorStateUpdateAfterFocusingElement || m_editorState.isMissingPostLayoutData)
>+        return;
>+
>+    pageClient().didReceiveEditorStateUpdateAfterFocus();
>+    m_waitingForPostLayoutEditorStateUpdateAfterFocusingElement = false;
> }
> 
> void WebPageProxy::showValidationMessage(const IntRect& anchorClientRect, const String& message)
>diff --git a/Source/WebKit/UIProcess/mac/WebPageProxyMac.mm b/Source/WebKit/UIProcess/mac/WebPageProxyMac.mm
>index 138d132ac1f316294cd63db89417933e8c5ffe54..4734aea0c11f8670fb98e48321aa83c33e848359 100644
>--- a/Source/WebKit/UIProcess/mac/WebPageProxyMac.mm
>+++ b/Source/WebKit/UIProcess/mac/WebPageProxyMac.mm
>@@ -607,7 +607,7 @@ bool WebPageProxy::appleMailLinesClampEnabled()
>     return MacApplication::isAppleMail();
> }
> 
>-void WebPageProxy::editorStateChanged(const EditorState& editorState)
>+void WebPageProxy::setEditorState(const EditorState& editorState)
> {
>     bool couldChangeSecureInputState = m_editorState.isInPasswordField != editorState.isInPasswordField || m_editorState.selectionIsNone;
>     
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
>index 8fbee418f6c038ca116d7f70450d8f040d4e6d16..cc4d7973d2446a657354ddb9bbef56a8ed9c8c5c 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.h
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.h
>@@ -1183,6 +1183,7 @@ public:
>     void setRemoteObjectRegistry(RemoteObjectRegistry&);
> 
>     void updateIntrinsicContentSizeIfNeeded(const WebCore::IntSize&);
>+    void scheduleFullEditorStateUpdate();
> 
> private:
>     WebPage(uint64_t pageID, WebPageCreationParameters&&);
>@@ -1199,7 +1200,6 @@ private:
>     void platformDetach();
>     void platformEditorState(WebCore::Frame&, EditorState& result, IncludePostLayoutDataHint) const;
>     void sendEditorStateUpdate();
>-    void scheduleFullEditorStateUpdate();
> 
> #if PLATFORM(COCOA)
>     void sendTouchBarMenuDataAddedUpdate(WebCore::HTMLMenuElement&);
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>index 15b60195234af253708f0916f2143faa4a9cb130..0123fd9d4fb4b0a19097677046c4dceed6c980ac 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>@@ -202,6 +202,8 @@ messages -> WebPage LegacyReceiver {
>     RunJavaScriptInFrame(uint64_t frameID, String script, bool forceUserGesture, WebKit::CallbackID callbackID)
>     ForceRepaint(WebKit::CallbackID callbackID)
> 
>+    ScheduleFullEditorStateUpdate()
>+
> #if PLATFORM(COCOA)
>     # Dictionary support.
>     PerformDictionaryLookupOfCurrentSelection()

Flags:

rniwa: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 197532: 368841 | 368850