[webkit-changes] [290743] trunk

Wed, 02 Mar 2022 11:41:13 -0800

Title: [290743] trunk
Revision
290743
Author
[email protected]
Date
2022-03-02 11:39:49 -0800 (Wed, 02 Mar 2022)

Log Message

Mousemove events double-firing in Safari
https://bugs.webkit.org/show_bug.cgi?id=237342
<rdar://88025610>

Reviewed by Wenson Hsieh.

Source/WebKit:

When we constructed a WebViewImpl, we would add a mouse tracking area to the view,
so that mouseMoved/mouseEntered/mouseExited would get called and we would be able
to forward these mouse events to the WebContent process. However, when the view
becomes first responder, an implicit mouse tracking area also gets added. As a
result, we would get duplicate calls to mouseMoved/mouseEntered/mouseExited.

We consulted with the AppKit team and their recommendation was to use a different
owner object for our mouse tracking area and have that object forward the
mouseMoved/mouseEntered/mouseExited calls to our WebViewImpl. In doing so, we
can stop forwarding mouseMoved/mouseEntered/mouseExited calls from WKWebView &
WKView, which are NOT for our mouse tracking area.

No new tests, I tried but wasn't able to write an API test for this.
I had trouble making the test window key so that the view would receive
the (duplicate) mousemove events. I validated via logging that we are no longer
getting duplicate mousemove events. I also checked on
https://www.vsynctester.com/testing/mouse.html that the output now looks correct.

* UIProcess/API/Cocoa/WKViewPrivate.h:
* UIProcess/API/Cocoa/WKWebViewPrivate.h:
* UIProcess/API/mac/WKView.mm:
(-[WKView _simulateMouseMove:]):
(-[WKView mouseMoved:]): Deleted.
(-[WKView mouseEntered:]): Deleted.
(-[WKView mouseExited:]): Deleted.
* UIProcess/API/mac/WKWebViewMac.mm:
(-[WKWebView _simulateMouseMove:]):
(-[WKWebView mouseMoved:]): Deleted.
(-[WKWebView mouseEntered:]): Deleted.
(-[WKWebView mouseExited:]): Deleted.
* UIProcess/Cocoa/WebViewImpl.h:
* UIProcess/Cocoa/WebViewImpl.mm:
(-[WKMouseTrackingObserver initWithViewImpl:]):
(-[WKMouseTrackingObserver mouseMoved:]):
(-[WKMouseTrackingObserver mouseEntered:]):
(-[WKMouseTrackingObserver mouseExited:]):
(WebKit::WebViewImpl::WebViewImpl):
(WebKit::WebViewImpl::updatePrimaryTrackingAreaOptions):
(WebKit::WebViewImpl::setPrimaryTrackingArea): Deleted.
* UIProcess/mac/PageClientImplMac.mm:
(WebKit::PageClientImpl::recommendedScrollbarStyleDidChange):

Tools:

Call [WKWebView _simulateMouseMove:] SPI instead of calling [WKWebView mouseMoved:]
since the latter calls are now being ignored.

* TestWebKitAPI/cocoa/TestWKWebView.mm:
(-[TestWKWebView mouseMoveToPoint:withFlags:]):
* TestWebKitAPI/mac/PlatformWebViewMac.mm:
(TestWebKitAPI::PlatformWebView::simulateMouseMove):
* WebKitTestRunner/mac/EventSenderProxy.mm:
(WTR::EventSenderProxy::mouseMoveTo):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (290742 => 290743)


--- trunk/Source/WebKit/ChangeLog	2022-03-02 19:26:32 UTC (rev 290742)
+++ trunk/Source/WebKit/ChangeLog	2022-03-02 19:39:49 UTC (rev 290743)
@@ -1,3 +1,53 @@
+2022-03-02  Chris Dumez  <[email protected]>
+
+        Mousemove events double-firing in Safari
+        https://bugs.webkit.org/show_bug.cgi?id=237342
+        <rdar://88025610>
+
+        Reviewed by Wenson Hsieh.
+
+        When we constructed a WebViewImpl, we would add a mouse tracking area to the view,
+        so that mouseMoved/mouseEntered/mouseExited would get called and we would be able
+        to forward these mouse events to the WebContent process. However, when the view
+        becomes first responder, an implicit mouse tracking area also gets added. As a
+        result, we would get duplicate calls to mouseMoved/mouseEntered/mouseExited.
+
+        We consulted with the AppKit team and their recommendation was to use a different
+        owner object for our mouse tracking area and have that object forward the
+        mouseMoved/mouseEntered/mouseExited calls to our WebViewImpl. In doing so, we
+        can stop forwarding mouseMoved/mouseEntered/mouseExited calls from WKWebView &
+        WKView, which are NOT for our mouse tracking area.
+
+        No new tests, I tried but wasn't able to write an API test for this.
+        I had trouble making the test window key so that the view would receive
+        the (duplicate) mousemove events. I validated via logging that we are no longer
+        getting duplicate mousemove events. I also checked on
+        https://www.vsynctester.com/testing/mouse.html that the output now looks correct.
+
+        * UIProcess/API/Cocoa/WKViewPrivate.h:
+        * UIProcess/API/Cocoa/WKWebViewPrivate.h:
+        * UIProcess/API/mac/WKView.mm:
+        (-[WKView _simulateMouseMove:]):
+        (-[WKView mouseMoved:]): Deleted.
+        (-[WKView mouseEntered:]): Deleted.
+        (-[WKView mouseExited:]): Deleted.
+        * UIProcess/API/mac/WKWebViewMac.mm:
+        (-[WKWebView _simulateMouseMove:]):
+        (-[WKWebView mouseMoved:]): Deleted.
+        (-[WKWebView mouseEntered:]): Deleted.
+        (-[WKWebView mouseExited:]): Deleted.
+        * UIProcess/Cocoa/WebViewImpl.h:
+        * UIProcess/Cocoa/WebViewImpl.mm:
+        (-[WKMouseTrackingObserver initWithViewImpl:]):
+        (-[WKMouseTrackingObserver mouseMoved:]):
+        (-[WKMouseTrackingObserver mouseEntered:]):
+        (-[WKMouseTrackingObserver mouseExited:]):
+        (WebKit::WebViewImpl::WebViewImpl):
+        (WebKit::WebViewImpl::updatePrimaryTrackingAreaOptions):
+        (WebKit::WebViewImpl::setPrimaryTrackingArea): Deleted.
+        * UIProcess/mac/PageClientImplMac.mm:
+        (WebKit::PageClientImpl::recommendedScrollbarStyleDidChange):
+
 2022-03-02  Sihui Liu  <[email protected]>
 
         Add assertion that no two sessions share the same general storage directory

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKViewPrivate.h (290742 => 290743)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKViewPrivate.h	2022-03-02 19:26:32 UTC (rev 290742)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKViewPrivate.h	2022-03-02 19:39:49 UTC (rev 290743)
@@ -131,6 +131,7 @@
 - (void)_didChangeContentSize:(NSSize)newSize;
 
 - (void)_gestureEventWasNotHandledByWebCore:(NSEvent *)event;
+- (void)_simulateMouseMove:(NSEvent *)event;
 
 - (void)_setShouldSuppressFirstResponderChanges:(BOOL)shouldSuppress WK_API_AVAILABLE(macos(10.13.4));

Modified: trunk/Source/WebKit/UIProcess/API/mac/WKView.mm (290742 => 290743)


--- trunk/Source/WebKit/UIProcess/API/mac/WKView.mm	2022-03-02 19:26:32 UTC (rev 290742)
+++ trunk/Source/WebKit/UIProcess/API/mac/WKView.mm	2022-03-02 19:39:49 UTC (rev 290743)
@@ -516,11 +516,6 @@
     _data->_impl->swipeWithEvent(event);
 }
 
-- (void)mouseMoved:(NSEvent *)event
-{
-    _data->_impl->mouseMoved(event);
-}
-
 - (void)mouseDown:(NSEvent *)event
 {
     _data->_impl->mouseDown(event);
@@ -536,16 +531,6 @@
     _data->_impl->mouseDragged(event);
 }
 
-- (void)mouseEntered:(NSEvent *)event
-{
-    _data->_impl->mouseEntered(event);
-}
-
-- (void)mouseExited:(NSEvent *)event
-{
-    _data->_impl->mouseExited(event);
-}
-
 - (void)otherMouseDown:(NSEvent *)event
 {
     _data->_impl->otherMouseDown(event);
@@ -1546,6 +1531,11 @@
     _data->_impl->gestureEventWasNotHandledByWebCoreFromViewOnly(event);
 }
 
+- (void)_simulateMouseMove:(NSEvent *)event
+{
+    _data->_impl->mouseMoved(event);
+}
+
 - (void)smartMagnifyWithEvent:(NSEvent *)event
 {
     _data->_impl->smartMagnifyWithEvent(event);

Modified: trunk/Source/WebKit/UIProcess/API/mac/WKWebViewMac.mm (290742 => 290743)


--- trunk/Source/WebKit/UIProcess/API/mac/WKWebViewMac.mm	2022-03-02 19:26:32 UTC (rev 290742)
+++ trunk/Source/WebKit/UIProcess/API/mac/WKWebViewMac.mm	2022-03-02 19:39:49 UTC (rev 290743)
@@ -474,11 +474,6 @@
     _impl->swipeWithEvent(event);
 }
 
-- (void)mouseMoved:(NSEvent *)event
-{
-    _impl->mouseMoved(event);
-}
-
 - (void)mouseDown:(NSEvent *)event
 {
     _impl->mouseDown(event);
@@ -494,16 +489,6 @@
     _impl->mouseDragged(event);
 }
 
-- (void)mouseEntered:(NSEvent *)event
-{
-    _impl->mouseEntered(event);
-}
-
-- (void)mouseExited:(NSEvent *)event
-{
-    _impl->mouseExited(event);
-}
-
 - (void)otherMouseDown:(NSEvent *)event
 {
     _impl->otherMouseDown(event);

Modified: trunk/Source/WebKit/UIProcess/API/mac/WKWebViewPrivateForTestingMac.h (290742 => 290743)


--- trunk/Source/WebKit/UIProcess/API/mac/WKWebViewPrivateForTestingMac.h	2022-03-02 19:26:32 UTC (rev 290742)
+++ trunk/Source/WebKit/UIProcess/API/mac/WKWebViewPrivateForTestingMac.h	2022-03-02 19:39:49 UTC (rev 290743)
@@ -52,6 +52,8 @@
 - (void)_setFooterBannerHeight:(int)height;
 - (NSSet<NSView *> *)_pdfHUDs;
 
+- (void)_simulateMouseMove:(NSEvent *)event;
+
 @end
 
 #endif // !TARGET_OS_IPHONE

Modified: trunk/Source/WebKit/UIProcess/API/mac/WKWebViewTestingMac.mm (290742 => 290743)


--- trunk/Source/WebKit/UIProcess/API/mac/WKWebViewTestingMac.mm	2022-03-02 19:26:32 UTC (rev 290742)
+++ trunk/Source/WebKit/UIProcess/API/mac/WKWebViewTestingMac.mm	2022-03-02 19:39:49 UTC (rev 290743)
@@ -121,6 +121,11 @@
     return nil;
 }
 
+- (void)_simulateMouseMove:(NSEvent *)event
+{
+    return _impl->mouseMoved(event);
+}
+
 @end
 
 #endif // PLATFORM(MAC)

Modified: trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.h (290742 => 290743)


--- trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.h	2022-03-02 19:26:32 UTC (rev 290742)
+++ trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.h	2022-03-02 19:39:49 UTC (rev 290743)
@@ -62,6 +62,7 @@
 OBJC_CLASS WKEditorUndoTarget;
 OBJC_CLASS WKFullScreenWindowController;
 OBJC_CLASS WKImmediateActionController;
+OBJC_CLASS WKMouseTrackingObserver;
 OBJC_CLASS WKRevealItemPresenter;
 OBJC_CLASS WKSafeBrowsingWarning;
 OBJC_CLASS WKShareSheet;
@@ -446,7 +447,7 @@
     id accessibilityAttributeValue(NSString *, id parameter = nil);
 
     NSTrackingArea *primaryTrackingArea() const { return m_primaryTrackingArea.get(); }
-    void setPrimaryTrackingArea(NSTrackingArea *);
+    void updatePrimaryTrackingAreaOptions(NSTrackingAreaOptions);
 
     NSTrackingRectTag addTrackingRect(CGRect, id owner, void* userData, bool assumeInside);
     NSTrackingRectTag addTrackingRectWithTrackingNum(CGRect, id owner, void* userData, bool assumeInside, int tag);
@@ -815,6 +816,7 @@
 
     bool m_allowsLinkPreview { true };
 
+    RetainPtr<WKMouseTrackingObserver> m_mouseTrackingObserver;
     RetainPtr<NSTrackingArea> m_primaryTrackingArea;
 
     NSToolTipTag m_lastToolTipTag { 0 };

Modified: trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm (290742 => 290743)


--- trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm	2022-03-02 19:26:32 UTC (rev 290742)
+++ trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm	2022-03-02 19:39:49 UTC (rev 290743)
@@ -181,6 +181,43 @@
 @end
 #endif
 
+// We use a WKMouseTrackingObserver as tracking area owner instead of the WKWebView. This is because WKWebView
+// gets an implicit tracking area when it is first responder and we only want to process mouse events from our
+// tracking area. Otherwise, it would lead to duplicate mouse events (rdar://88025610).
+@interface WKMouseTrackingObserver : NSObject
+@end
+
+@implementation WKMouseTrackingObserver {
+    WeakPtr<WebKit::WebViewImpl> _impl;
+}
+
+- (instancetype)initWithViewImpl:(WebKit::WebViewImpl&)impl
+{
+    if ((self = [super init]))
+        _impl = impl;
+    return self;
+}
+
+- (void)mouseMoved:(NSEvent *)event
+{
+    if (_impl)
+        _impl->mouseMoved(event);
+}
+
+- (void)mouseEntered:(NSEvent *)event
+{
+    if (_impl)
+        _impl->mouseEntered(event);
+}
+
+- (void)mouseExited:(NSEvent *)event
+{
+    if (_impl)
+        _impl->mouseExited(event);
+}
+
+@end
+
 #if ENABLE(IMAGE_ANALYSIS)
 
 namespace WebKit {
@@ -1487,7 +1524,8 @@
     , m_undoTarget(adoptNS([[WKEditorUndoTarget alloc] init]))
     , m_windowVisibilityObserver(adoptNS([[WKWindowVisibilityObserver alloc] initWithView:view impl:*this]))
     , m_accessibilitySettingsObserver(adoptNS([[WKAccessibilitySettingsObserver alloc] initWithImpl:*this]))
-    , m_primaryTrackingArea(adoptNS([[NSTrackingArea alloc] initWithRect:view.frame options:trackingAreaOptions() owner:view userInfo:nil]))
+    , m_mouseTrackingObserver(adoptNS([[WKMouseTrackingObserver alloc] initWithViewImpl:*this]))
+    , m_primaryTrackingArea(adoptNS([[NSTrackingArea alloc] initWithRect:view.frame options:trackingAreaOptions() owner:m_mouseTrackingObserver.get() userInfo:nil]))
 {
     static_cast<PageClientImpl&>(*m_pageClient).setImpl(*this);
 
@@ -3831,11 +3869,13 @@
     return [m_view _web_superAccessibilityAttributeValue:attribute];
 }
 
-void WebViewImpl::setPrimaryTrackingArea(NSTrackingArea *trackingArea)
+void WebViewImpl::updatePrimaryTrackingAreaOptions(NSTrackingAreaOptions options)
 {
+    auto trackingArea = adoptNS([[NSTrackingArea alloc] initWithRect:[m_view frame] options:options owner:m_mouseTrackingObserver.get() userInfo:nil]);
     [m_view removeTrackingArea:m_primaryTrackingArea.get()];
     m_primaryTrackingArea = trackingArea;
-    [m_view addTrackingArea:trackingArea];
+    [m_view addTrackingArea:trackingArea.get()];
+
 }
 
 // Any non-zero value will do, but using something recognizable might help us debug some day.

Modified: trunk/Source/WebKit/UIProcess/mac/PageClientImplMac.mm (290742 => 290743)


--- trunk/Source/WebKit/UIProcess/mac/PageClientImplMac.mm	2022-03-02 19:26:32 UTC (rev 290742)
+++ trunk/Source/WebKit/UIProcess/mac/PageClientImplMac.mm	2022-03-02 19:39:49 UTC (rev 290743)
@@ -719,8 +719,7 @@
     else
         options |= NSTrackingActiveInKeyWindow;
 
-    RetainPtr<NSTrackingArea> trackingArea = adoptNS([[NSTrackingArea alloc] initWithRect:[m_view frame] options:options owner:m_view userInfo:nil]);
-    m_impl->setPrimaryTrackingArea(trackingArea.get());
+    m_impl->updatePrimaryTrackingAreaOptions(options);
 }
 
 void PageClientImpl::intrinsicContentSizeDidChange(const IntSize& intrinsicContentSize)

Modified: trunk/Tools/ChangeLog (290742 => 290743)


--- trunk/Tools/ChangeLog	2022-03-02 19:26:32 UTC (rev 290742)
+++ trunk/Tools/ChangeLog	2022-03-02 19:39:49 UTC (rev 290743)
@@ -1,3 +1,21 @@
+2022-03-02  Chris Dumez  <[email protected]>
+
+        Mousemove events double-firing in Safari
+        https://bugs.webkit.org/show_bug.cgi?id=237342
+        <rdar://88025610>
+
+        Reviewed by Wenson Hsieh.
+
+        Call [WKWebView _simulateMouseMove:] SPI instead of calling [WKWebView mouseMoved:]
+        since the latter calls are now being ignored.
+
+        * TestWebKitAPI/cocoa/TestWKWebView.mm:
+        (-[TestWKWebView mouseMoveToPoint:withFlags:]):
+        * TestWebKitAPI/mac/PlatformWebViewMac.mm:
+        (TestWebKitAPI::PlatformWebView::simulateMouseMove):
+        * WebKitTestRunner/mac/EventSenderProxy.mm:
+        (WTR::EventSenderProxy::mouseMoveTo):
+
 2022-03-02  Sihui Liu  <[email protected]>
 
         Add assertion that no two sessions share the same general storage directory

Modified: trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm (290742 => 290743)


--- trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm	2022-03-02 19:26:32 UTC (rev 290742)
+++ trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm	2022-03-02 19:39:49 UTC (rev 290743)
@@ -837,7 +837,7 @@
 
 - (void)mouseMoveToPoint:(NSPoint)pointInWindow withFlags:(NSEventModifierFlags)flags
 {
-    [self mouseMoved:[self _mouseEventWithType:NSEventTypeMouseMoved atLocation:pointInWindow flags:flags timestamp:self.eventTimestamp clickCount:0]];
+    [self _simulateMouseMove:[self _mouseEventWithType:NSEventTypeMouseMoved atLocation:pointInWindow flags:flags timestamp:self.eventTimestamp clickCount:0]];
 }
 
 - (void)sendClicksAtPoint:(NSPoint)pointInWindow numberOfClicks:(NSUInteger)numberOfClicks

Modified: trunk/Tools/TestWebKitAPI/mac/PlatformWebViewMac.mm (290742 => 290743)


--- trunk/Tools/TestWebKitAPI/mac/PlatformWebViewMac.mm	2022-03-02 19:26:32 UTC (rev 290742)
+++ trunk/Tools/TestWebKitAPI/mac/PlatformWebViewMac.mm	2022-03-02 19:39:49 UTC (rev 290743)
@@ -195,7 +195,7 @@
 void PlatformWebView::simulateMouseMove(unsigned x, unsigned y, WKEventModifiers modifiers)
 {
     NSEvent *event = [NSEvent mouseEventWithType:NSEventTypeMouseMoved location:NSMakePoint(x, y) modifierFlags:modifierFlagsForWKModifiers(modifiers) timestamp:GetCurrentEventTime() windowNumber:[m_window windowNumber] context:[NSGraphicsContext currentContext] eventNumber:0 clickCount:0 pressure:0];
-    [m_view mouseMoved:event];
+    [m_view _simulateMouseMove:event];
 }
 
 void PlatformWebView::simulateButtonClick(WKEventMouseButton button, unsigned x, unsigned y, WKEventModifiers modifiers)

Modified: trunk/Tools/WebKitTestRunner/mac/EventSenderProxy.mm (290742 => 290743)


--- trunk/Tools/WebKitTestRunner/mac/EventSenderProxy.mm	2022-03-02 19:26:32 UTC (rev 290742)
+++ trunk/Tools/WebKitTestRunner/mac/EventSenderProxy.mm	2022-03-02 19:39:49 UTC (rev 290743)
@@ -38,6 +38,7 @@
 #import <WebKit/WKString.h>
 #import <WebKit/WKPagePrivate.h>
 #import <WebKit/WKWebView.h>
+#import <WebKit/WKWebViewPrivateForTestingMac.h>
 #import <pal/spi/cocoa/IOKitSPI.h>
 #import <wtf/RetainPtr.h>
 
@@ -613,7 +614,7 @@
         if (isDrag)
             [targetView mouseDragged:event];
         else
-            [targetView mouseMoved:event];
+            [static_cast<WKWebView*>(targetView) _simulateMouseMove:event];
         [NSApp _setCurrentEvent:nil];
     } else
         WTFLogAlways("mouseMoveTo failed to find a target view at %f,%f\n", windowLocation.x, windowLocation.y);

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

Reply via email to