Title: [239796] branches/safari-607-branch/Source/WebKit
- Revision
- 239796
- Author
- [email protected]
- Date
- 2019-01-09 17:37:56 -0800 (Wed, 09 Jan 2019)
Log Message
Cherry-pick r239686. rdar://problem/47158659
Unwanted page navigation after showing & dismissing contextual menu with control-click
https://bugs.webkit.org/show_bug.cgi?id=192912
<rdar://problem/46318508>
Reviewed by Timothy Hatcher.
After the conversion to use a mouse event queue, this behavior was observed rarely, especially
when CPU is under load and lots of things are going on in the page. In other words, it's racy.
Based on NSEvent logging, it seems that when the system is under load, we simply take too long
to enter the nested runloop that AppKit uses to handle events when the context menu is present.
AppKit doesn't care whether or not the MouseDown triggered a nested runloop; on my machine it delivers
the MouseUp event about 130ms after the MouseDown event. If we haven't show the context menu in
that time, then the MouseUp is enqueued as a normal mouse event. If the Ctrl-click is on a link,
then the MouseUp will be interpreted by EventHandler as a click event, which in the simplest case
will initiate a main frame navigation. When the context menu is dismissed and the nested runloop
is torn down, the navigation IPC message is handled in UIProcess and the page navigates away.
We can't do much to change AppKit's inherently racy behavior, but we can avoid processing
mouse events that are delivered whilst we are processing the context menu-initiating event.
From the WebProcess point of view, there is no race anymore because it does not receive the
MouseUp event.
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::showContextMenu):
If new events come in, drop them on the floor. Based on testing, the nested runloop will discard
the unpaired MouseUp event anyway, so this does not cause a change in behavior.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239686 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-607-branch/Source/WebKit/ChangeLog (239795 => 239796)
--- branches/safari-607-branch/Source/WebKit/ChangeLog 2019-01-10 01:37:52 UTC (rev 239795)
+++ branches/safari-607-branch/Source/WebKit/ChangeLog 2019-01-10 01:37:56 UTC (rev 239796)
@@ -1,3 +1,67 @@
+2019-01-09 Kocsen Chung <[email protected]>
+
+ Cherry-pick r239686. rdar://problem/47158659
+
+ Unwanted page navigation after showing & dismissing contextual menu with control-click
+ https://bugs.webkit.org/show_bug.cgi?id=192912
+ <rdar://problem/46318508>
+
+ Reviewed by Timothy Hatcher.
+
+ After the conversion to use a mouse event queue, this behavior was observed rarely, especially
+ when CPU is under load and lots of things are going on in the page. In other words, it's racy.
+
+ Based on NSEvent logging, it seems that when the system is under load, we simply take too long
+ to enter the nested runloop that AppKit uses to handle events when the context menu is present.
+ AppKit doesn't care whether or not the MouseDown triggered a nested runloop; on my machine it delivers
+ the MouseUp event about 130ms after the MouseDown event. If we haven't show the context menu in
+ that time, then the MouseUp is enqueued as a normal mouse event. If the Ctrl-click is on a link,
+ then the MouseUp will be interpreted by EventHandler as a click event, which in the simplest case
+ will initiate a main frame navigation. When the context menu is dismissed and the nested runloop
+ is torn down, the navigation IPC message is handled in UIProcess and the page navigates away.
+
+ We can't do much to change AppKit's inherently racy behavior, but we can avoid processing
+ mouse events that are delivered whilst we are processing the context menu-initiating event.
+ From the WebProcess point of view, there is no race anymore because it does not receive the
+ MouseUp event.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::showContextMenu):
+ If new events come in, drop them on the floor. Based on testing, the nested runloop will discard
+ the unpaired MouseUp event anyway, so this does not cause a change in behavior.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239686 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-01-07 Brian Burg <[email protected]>
+
+ Unwanted page navigation after showing & dismissing contextual menu with control-click
+ https://bugs.webkit.org/show_bug.cgi?id=192912
+ <rdar://problem/46318508>
+
+ Reviewed by Timothy Hatcher.
+
+ After the conversion to use a mouse event queue, this behavior was observed rarely, especially
+ when CPU is under load and lots of things are going on in the page. In other words, it's racy.
+
+ Based on NSEvent logging, it seems that when the system is under load, we simply take too long
+ to enter the nested runloop that AppKit uses to handle events when the context menu is present.
+ AppKit doesn't care whether or not the MouseDown triggered a nested runloop; on my machine it delivers
+ the MouseUp event about 130ms after the MouseDown event. If we haven't show the context menu in
+ that time, then the MouseUp is enqueued as a normal mouse event. If the Ctrl-click is on a link,
+ then the MouseUp will be interpreted by EventHandler as a click event, which in the simplest case
+ will initiate a main frame navigation. When the context menu is dismissed and the nested runloop
+ is torn down, the navigation IPC message is handled in UIProcess and the page navigates away.
+
+ We can't do much to change AppKit's inherently racy behavior, but we can avoid processing
+ mouse events that are delivered whilst we are processing the context menu-initiating event.
+ From the WebProcess point of view, there is no race anymore because it does not receive the
+ MouseUp event.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::showContextMenu):
+ If new events come in, drop them on the floor. Based on testing, the nested runloop will discard
+ the unpaired MouseUp event anyway, so this does not cause a change in behavior.
+
2019-01-09 Alan Coon <[email protected]>
Cherry-pick r239776. rdar://problem/47147610
Modified: branches/safari-607-branch/Source/WebKit/UIProcess/WebPageProxy.cpp (239795 => 239796)
--- branches/safari-607-branch/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-01-10 01:37:52 UTC (rev 239795)
+++ branches/safari-607-branch/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-01-10 01:37:56 UTC (rev 239796)
@@ -5555,6 +5555,12 @@
// Showing a context menu runs a nested runloop, which can handle messages that cause |this| to get closed.
Ref<WebPageProxy> protect(*this);
+ // Discard any enqueued mouse events that have been delivered to the UIProcess whilst the WebProcess is still processing the
+ // MouseDown event that triggered this ShowContextMenu message. This can happen if we take too long to enter the nested runloop.
+ ASSERT(isProcessingMouseEvents());
+ while (m_mouseEventQueue.size() > 1)
+ m_mouseEventQueue.takeLast();
+
m_activeContextMenuContextData = contextMenuContextData;
m_activeContextMenu = pageClient().createContextMenuProxy(*this, WTFMove(contextMenuContextData), userData);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
