Changeset 290743 in webkit

Timestamp:

Mar 2, 2022 11:39:49 AM (5 months ago)

Author:

Chris Dumez

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):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKViewPrivate.h (modified) (1 diff)
• Source/WebKit/UIProcess/API/mac/WKView.mm (modified) (3 diffs)
• Source/WebKit/UIProcess/API/mac/WKWebViewMac.mm (modified) (2 diffs)
• Source/WebKit/UIProcess/API/mac/WKWebViewPrivateForTestingMac.h (modified) (1 diff)
• Source/WebKit/UIProcess/API/mac/WKWebViewTestingMac.mm (modified) (1 diff)
• Source/WebKit/UIProcess/Cocoa/WebViewImpl.h (modified) (3 diffs)
• Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm (modified) (3 diffs)
• Source/WebKit/UIProcess/mac/PageClientImplMac.mm (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/cocoa/TestWKWebView.mm (modified) (1 diff)
• Tools/TestWebKitAPI/mac/PlatformWebViewMac.mm (modified) (1 diff)
• Tools/WebKitTestRunner/mac/EventSenderProxy.mm (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2022-03-02  Chris Dumez  <cdumez@apple.com>
+
+        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  <sihui_liu@apple.com>
 

trunk/Source/WebKit/UIProcess/API/Cocoa/WKViewPrivate.h

@@ -132 +132 @@
 
 - (void)_gestureEventWasNotHandledByWebCore:(NSEvent *)event;
+- (void)_simulateMouseMove:(NSEvent *)event;
 
 - (void)_setShouldSuppressFirstResponderChanges:(BOOL)shouldSuppress WK_API_AVAILABLE(macos(10.13.4));

trunk/Source/WebKit/UIProcess/API/mac/WKView.mm

@@ -517 +517 @@
 }
 
-- (void)mouseMoved:(NSEvent *)event
-{
-    _data->_impl->mouseMoved(event);
-}
-
 - (void)mouseDown:(NSEvent *)event
 {
@@ -535 +530 @@
 {
     _data->_impl->mouseDragged(event);
-}
-
-- (void)mouseEntered:(NSEvent *)event
-{
-    _data->_impl->mouseEntered(event);
-}
-
-- (void)mouseExited:(NSEvent *)event
-{
-    _data->_impl->mouseExited(event);
 }
 
@@ -1547 +1532 @@
 }
 
+- (void)_simulateMouseMove:(NSEvent *)event
+{
+    _data->_impl->mouseMoved(event);
+}
+
 - (void)smartMagnifyWithEvent:(NSEvent *)event
 {

trunk/Source/WebKit/UIProcess/API/mac/WKWebViewMac.mm

@@ -475 +475 @@
 }
 
-- (void)mouseMoved:(NSEvent *)event
-{
-    _impl->mouseMoved(event);
-}
-
 - (void)mouseDown:(NSEvent *)event
 {
@@ -493 +488 @@
 {
     _impl->mouseDragged(event);
-}
-
-- (void)mouseEntered:(NSEvent *)event
-{
-    _impl->mouseEntered(event);
-}
-
-- (void)mouseExited:(NSEvent *)event
-{
-    _impl->mouseExited(event);
 }
 

trunk/Source/WebKit/UIProcess/API/mac/WKWebViewPrivateForTestingMac.h

@@ -53 +53 @@
 - (NSSet<NSView *> *)_pdfHUDs;
 
+- (void)_simulateMouseMove:(NSEvent *)event;
+
 @end
 

trunk/Source/WebKit/UIProcess/API/mac/WKWebViewTestingMac.mm

@@ -122 +122 @@
 }
 
+- (void)_simulateMouseMove:(NSEvent *)event
+{
+    return _impl->mouseMoved(event);
+}
+
 @end
 

trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.h

@@ -63 +63 @@
 OBJC_CLASS WKFullScreenWindowController;
 OBJC_CLASS WKImmediateActionController;
+OBJC_CLASS WKMouseTrackingObserver;
 OBJC_CLASS WKRevealItemPresenter;
 OBJC_CLASS WKSafeBrowsingWarning;
@@ -447 +448 @@
 
     NSTrackingArea *primaryTrackingArea() const { return m_primaryTrackingArea.get(); }
-    void setPrimaryTrackingArea(NSTrackingArea *);
+    void updatePrimaryTrackingAreaOptions(NSTrackingAreaOptions);
 
     NSTrackingRectTag addTrackingRect(CGRect, id owner, void* userData, bool assumeInside);
@@ -816 +817 @@
     bool m_allowsLinkPreview { true };
 
+    RetainPtr<WKMouseTrackingObserver> m_mouseTrackingObserver;
     RetainPtr<NSTrackingArea> m_primaryTrackingArea;
 

trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm

@@ -182 +182 @@
 #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)
 
@@ -1488 +1525 @@
     , 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);
@@ -3832 +3870 @@
 }
 
-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()];
+
 }
 

trunk/Source/WebKit/UIProcess/mac/PageClientImplMac.mm

@@ -720 +720 @@
         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);
 }
 

trunk/Tools/ChangeLog

@@ +1 @@
+2022-03-02  Chris Dumez  <cdumez@apple.com>
+
+        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  <sihui_liu@apple.com>
 

trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm

@@ -838 +838 @@
 - (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]];
 }
 

trunk/Tools/TestWebKitAPI/mac/PlatformWebViewMac.mm

@@ -196 +196 @@
 {
     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];
 }
 

trunk/Tools/WebKitTestRunner/mac/EventSenderProxy.mm

@@ -39 +39 @@
 #import <WebKit/WKPagePrivate.h>
 #import <WebKit/WKWebView.h>
+#import <WebKit/WKWebViewPrivateForTestingMac.h>
 #import <pal/spi/cocoa/IOKitSPI.h>
 #import <wtf/RetainPtr.h>
@@ -614 +615 @@
             [targetView mouseDragged:event];
         else
-            [targetView mouseMoved:event];
+            [static_cast<WKWebView*>(targetView) _simulateMouseMove:event];
         [NSApp _setCurrentEvent:nil];
     } else