[webkit-changes] [295626] trunk

Thu, 16 Jun 2022 21:54:25 -0700

Title: [295626] trunk
Revision
295626
Author
[email protected]
Date
2022-06-16 21:53:11 -0700 (Thu, 16 Jun 2022)

Log Message

[Trackpad with iPad] Right-clicking on text sometimes doesn't show the correct list of actions
https://bugs.webkit.org/show_bug.cgi?id=241645
rdar://91792562

Reviewed by Devin Rousso.

This patch fixes a race condition in Reveal code where we may not have updated
the selection in UITextInteractionAssistant before calling the
completion handler for the Reveal menu. This results in an empty menu
appearing often during a right-click.

To fix this, this patch converts sendEditorStateUpdate to an async call and
makes sure that call has completed before returning the context menu.

Since this is a race condition, it was a bit tricky to test. This patch
adds a function _simulateSelectionStart which sets up the WKContentView
as if a selection has begun, but it doesn't actually contact the
UITextInteractionAssistant (to avoid flaky false positives). Then the test calls
the context menu code and makes sure that the UITextInteractionAssistant has
been contacted by the time it completes.

* Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h:
* Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm:
(-[WKWebView _simulateSelectionStart]):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::editorStateChanged):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.messages.in:
* Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:
* Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _simulateSelectionStart]):
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::deleteSurrounding):
(WebKit::WebPage::didApplyStyle):
(WebKit::WebPage::didChangeContents):
(WebKit::WebPage::didUpdateComposition):
(WebKit::WebPage::didEndUserTriggeredSelectionChanges):
(WebKit::WebPage::discardedComposition):
(WebKit::WebPage::canceledComposition):
(WebKit::WebPage::sendEditorStateUpdate):
(WebKit::WebPage::flushPendingEditorStateUpdate):
* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::prepareSelectionForContextMenuWithLocationInView):
(WebKit::WebPage::requestPositionInformation):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/iOSMouseSupport.mm:
(handleUpdatedSelection):
(TEST):
* Tools/TestWebKitAPI/ios/UIKitSPI.h:

Canonical link: https://commits.webkit.org/251631@main

Modified Paths

Diff

Modified: trunk/Source/WebKit/UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h (295625 => 295626)


--- trunk/Source/WebKit/UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h	2022-06-17 02:47:14 UTC (rev 295625)
+++ trunk/Source/WebKit/UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h	2022-06-17 04:53:11 UTC (rev 295626)
@@ -91,6 +91,7 @@
 - (NSString *)_serializedSelectionCaretBackgroundColorForTesting;
 
 - (BOOL)_hasResizeAssertion;
+- (void)_simulateSelectionStart;
 
 @end

Modified: trunk/Source/WebKit/UIProcess/API/ios/WKWebViewTestingIOS.mm (295625 => 295626)


--- trunk/Source/WebKit/UIProcess/API/ios/WKWebViewTestingIOS.mm	2022-06-17 02:47:14 UTC (rev 295625)
+++ trunk/Source/WebKit/UIProcess/API/ios/WKWebViewTestingIOS.mm	2022-06-17 04:53:11 UTC (rev 295626)
@@ -490,6 +490,11 @@
     return NO;
 }
 
+- (void)_simulateSelectionStart
+{
+    [_contentView _simulateSelectionStart];
+}
+
 @end
 
 #endif // PLATFORM(IOS_FAMILY)

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (295625 => 295626)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2022-06-17 02:47:14 UTC (rev 295625)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2022-06-17 04:53:11 UTC (rev 295626)
@@ -7696,10 +7696,12 @@
     }
 }
 
-void WebPageProxy::editorStateChanged(const EditorState& editorState)
+void WebPageProxy::editorStateChanged(const EditorState& editorState, CompletionHandler<void()>&& completionHandler)
 {
     if (updateEditorState(editorState))
         dispatchDidUpdateEditorState();
+
+    completionHandler();
 }
 
 bool WebPageProxy::updateEditorState(const EditorState& newEditorState)

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (295625 => 295626)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.h	2022-06-17 02:47:14 UTC (rev 295625)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h	2022-06-17 04:53:11 UTC (rev 295626)
@@ -1733,7 +1733,7 @@
 #if PLATFORM(COCOA)
     void createSandboxExtensionsIfNeeded(const Vector<String>& files, SandboxExtension::Handle& fileReadHandle, Vector<SandboxExtension::Handle>& fileUploadHandles);
 #endif
-    void editorStateChanged(const EditorState&);
+    void editorStateChanged(const EditorState&, CompletionHandler<void()>&&);
     bool updateEditorState(const EditorState& newEditorState);
     void scheduleFullEditorStateUpdate();
     void dispatchDidUpdateEditorState();

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in (295625 => 295626)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in	2022-06-17 02:47:14 UTC (rev 295625)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in	2022-06-17 04:53:11 UTC (rev 295626)
@@ -212,7 +212,7 @@
     LogScrollingEvent(uint32_t eventType, MonotonicTime timestamp, uint64_t data)
 
     # Editor notifications
-    EditorStateChanged(struct WebKit::EditorState editorState)
+    EditorStateChanged(struct WebKit::EditorState editorState) -> ()
     CompositionWasCanceled()
     SetHasHadSelectionChangesFromUserInteraction(bool hasHadUserSelectionChanges)

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h (295625 => 295626)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h	2022-06-17 02:47:14 UTC (rev 295625)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h	2022-06-17 04:53:11 UTC (rev 295626)
@@ -827,6 +827,7 @@
 - (NSDictionary *)_contentsOfUserInterfaceItem:(NSString *)userInterfaceItem;
 - (void)_doAfterReceivingEditDragSnapshotForTesting:(dispatch_block_t)action;
 - (void)_dismissContactPickerWithContacts:(NSArray *)contacts;
+- (void)_simulateSelectionStart;
 
 #if ENABLE(DATALIST_ELEMENT)
 - (void)_selectDataListOption:(NSInteger)optionIndex;

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (295625 => 295626)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2022-06-17 02:47:14 UTC (rev 295625)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2022-06-17 04:53:11 UTC (rev 295626)
@@ -11430,6 +11430,12 @@
 #endif
 }
 
+- (void)_simulateSelectionStart
+{
+    _usingGestureForSelection = YES;
+    _lastSelectionDrawingInfo.type = WebKit::WKSelectionDrawingInfo::SelectionType::Range;
+}
+
 #if ENABLE(DATALIST_ELEMENT)
 - (void)_selectDataListOption:(NSInteger)optionIndex
 {

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (295625 => 295626)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2022-06-17 02:47:14 UTC (rev 295625)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2022-06-17 04:53:11 UTC (rev 295626)
@@ -6194,7 +6194,7 @@
     targetFrame->selection().setSelection(VisibleSelection(selectionRange));
     targetFrame->editor().deleteSelectionWithSmartDelete(false);
     targetFrame->editor().setIgnoreSelectionChanges(false);
-    sendEditorStateUpdate();
+    sendEditorStateUpdate([] { });
 }
 
 #endif
@@ -6201,12 +6201,12 @@
 
 void WebPage::didApplyStyle()
 {
-    sendEditorStateUpdate();
+    sendEditorStateUpdate([] { });
 }
 
 void WebPage::didChangeContents()
 {
-    sendEditorStateUpdate();
+    sendEditorStateUpdate([] { });
 }
 
 void WebPage::didScrollSelection()
@@ -6403,7 +6403,7 @@
 
 void WebPage::didUpdateComposition()
 {
-    sendEditorStateUpdate();
+    sendEditorStateUpdate([] { });
 }
 
 void WebPage::didEndUserTriggeredSelectionChanges()
@@ -6410,19 +6410,19 @@
 {
     Ref frame = CheckedRef(m_page->focusController())->focusedOrMainFrame();
     if (!frame->editor().ignoreSelectionChanges())
-        sendEditorStateUpdate();
+        sendEditorStateUpdate([] { });
 }
 
 void WebPage::discardedComposition()
 {
     send(Messages::WebPageProxy::CompositionWasCanceled());
-    sendEditorStateUpdate();
+    sendEditorStateUpdate([] { });
 }
 
 void WebPage::canceledComposition()
 {
     send(Messages::WebPageProxy::CompositionWasCanceled());
-    sendEditorStateUpdate();
+    sendEditorStateUpdate([] { });
 }
 
 void WebPage::navigateServiceWorkerClient(ScriptExecutionContextIdentifier documentIdentifier, const URL& url, CompletionHandler<void(bool)>&& callback)
@@ -6819,11 +6819,13 @@
     m_loaderClient->featuresUsedInPage(*this, namedFeatures);
 }
 
-void WebPage::sendEditorStateUpdate()
+void WebPage::sendEditorStateUpdate(CompletionHandler<void()>&& completionHandler)
 {
     Ref frame = CheckedRef(m_page->focusController())->focusedOrMainFrame();
-    if (frame->editor().ignoreSelectionChanges() || !frame->document() || !frame->document()->hasLivingRenderTree())
+    if (frame->editor().ignoreSelectionChanges() || !frame->document() || !frame->document()->hasLivingRenderTree()) {
+        completionHandler();
         return;
+    }
 
     m_pendingEditorStateUpdateStatus = PendingEditorStateUpdateStatus::NotScheduled;
 
@@ -6831,7 +6833,7 @@
     // If that is the case, just send what we have (i.e. don't include post-layout data) and wait until the
     // next layer tree commit to compute and send the complete EditorState over.
     auto state = editorState();
-    send(Messages::WebPageProxy::EditorStateChanged(state));
+    sendWithAsyncReply(Messages::WebPageProxy::EditorStateChanged(state), WTFMove(completionHandler));
     if (state.isMissingPostLayoutData && !shouldAvoidComputingPostLayoutDataForEditorState())
         scheduleFullEditorStateUpdate();
 }
@@ -6913,7 +6915,7 @@
     if (frame->editor().ignoreSelectionChanges())
         return;
 
-    sendEditorStateUpdate();
+    sendEditorStateUpdate([] { });
 }
 
 void WebPage::updateWebsitePolicies(WebsitePoliciesData&& websitePolicies)

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (295625 => 295626)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h	2022-06-17 02:47:14 UTC (rev 295625)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h	2022-06-17 04:53:11 UTC (rev 295626)
@@ -1566,7 +1566,8 @@
     void getPlatformEditorState(WebCore::Frame&, EditorState&) const;
     bool requiresPostLayoutDataForEditorState(const WebCore::Frame&) const;
     void platformWillPerformEditingCommand();
-    void sendEditorStateUpdate();
+    void sendEditorStateUpdate(CompletionHandler<void()>&&);
+
     void getPlatformEditorStateCommon(const WebCore::Frame&, EditorState&) const;
 
 #if HAVE(TOUCH_BAR)

Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (295625 => 295626)


--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2022-06-17 02:47:14 UTC (rev 295625)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2022-06-17 04:53:11 UTC (rev 295626)
@@ -2422,13 +2422,20 @@
         auto range = makeRangeSelectingNode(*hitNode);
         if (range && frame->selection().setSelectedRange(range, Affinity::Upstream, FrameSelection::ShouldCloseTyping::Yes, UserTriggered)) {
             layoutIfNeeded();
-            sendEditorStateUpdate();
+            sendEditorStateUpdate([] { });
             return completionHandler(true, { });
         }
     }
 
     frame->eventHandler().selectClosestContextualWordOrLinkFromHitTestResult(result, DontAppendTrailingWhitespace);
-    completionHandler(true, { revealItemForCurrentSelection() });
+    sendEditorStateUpdate([weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler)] () mutable {
+        RefPtr strongThis = weakThis.get();
+        if (!strongThis) {
+            completionHandler(true, { });
+            return;
+        }
+        completionHandler(true, { strongThis->revealItemForCurrentSelection() });
+    });
 }
 #endif
 
@@ -3285,7 +3292,7 @@
 
 void WebPage::requestPositionInformation(const InteractionInformationRequest& request)
 {
-    sendEditorStateUpdate();
+    sendEditorStateUpdate([] { });
     send(Messages::WebPageProxy::DidReceivePositionInformation(positionInformation(request)));
 }

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/iOSMouseSupport.mm (295625 => 295626)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/iOSMouseSupport.mm	2022-06-17 02:47:14 UTC (rev 295625)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/iOSMouseSupport.mm	2022-06-17 04:53:11 UTC (rev 295626)
@@ -27,6 +27,7 @@
 
 #if PLATFORM(IOS) || PLATFORM(MACCATALYST)
 
+#import "InstanceMethodSwizzler.h"
 #import "PlatformUtilities.h"
 #import "Test.h"
 #import "TestNavigationDelegate.h"
@@ -378,6 +379,32 @@
     EXPECT_NOT_NULL(hitTestResult);
 }
 
+static bool selectionUpdated = false;
+static void handleUpdatedSelection(id, SEL)
+{
+    selectionUpdated = true;
+}
+
+TEST(iOSMouseSupport, SelectionUpdatesBeforeContextMenuAppears)
+{
+    InstanceMethodSwizzler swizzler { UIWKTextInteractionAssistant.class, @selector(selectionChanged), reinterpret_cast<IMP>(handleUpdatedSelection) };
+
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+    [webView synchronouslyLoadTestPageNamed:@"simple"];
+    [webView objectByEvaluatingJavaScript:@"document.body.setAttribute('contenteditable','');"];
+
+    auto contentView = [webView wkContentView];
+    [webView _simulateSelectionStart];
+    __block bool done = false;
+    [contentView prepareSelectionForContextMenuWithLocationInView:CGPointZero completionHandler:^(BOOL, RVItem *) {
+        EXPECT_TRUE(selectionUpdated);
+        done = true;
+    }];
+
+    TestWebKitAPI::Util::run(&done);
+}
+
 #if ENABLE(IOS_TOUCH_EVENTS)
 
 TEST(iOSMouseSupport, WebsiteMouseEventPolicies)

Modified: trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h (295625 => 295626)


--- trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h	2022-06-17 02:47:14 UTC (rev 295625)
+++ trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h	2022-06-17 04:53:11 UTC (rev 295626)
@@ -282,6 +282,7 @@
 
 @interface UIWKTextInteractionAssistant : UITextInteractionAssistant
 - (void)lookup:(NSString *)textWithContext withRange:(NSRange)range fromRect:(CGRect)presentationRect;
+- (void)selectionChanged;
 @end
 
 @interface UIAction ()

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to