WebKit Bugzilla
Attachment 357844 Details for
Bug 192912
: Unwanted page navigation after showing & dismissing contextual menu with control-click
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-192912-20181220114629.patch (text/plain), 3.40 KB, created by
BJ Burg
on 2018-12-20 11:46:29 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
BJ Burg
Created:
2018-12-20 11:46:29 PST
Size:
3.40 KB
patch
obsolete
>Subversion Revision: 239429 >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index 3f27a965cc6c3e2d2729bdf506e629e5267a8825..feee044a27d0a38f2e7991792a04511eb2a5c316 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,33 @@ >+2018-12-19 Brian Burg <bburg@apple.com> >+ >+ 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 NOBODY (OOPS!). >+ >+ 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. >+ > 2018-12-19 Chris Dumez <cdumez@apple.com> > > wtf/Optional.h: move-constructor and move-assignment operator should disengage the value being moved from >diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp >index 4b01252ebb8fe927c5aaf4c570da8e455f8ecb6d..63a1f160b3658390ec28c6d8cb670f0e39110b0b 100644 >--- a/Source/WebKit/UIProcess/WebPageProxy.cpp >+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp >@@ -5450,6 +5450,12 @@ void WebPageProxy::showContextMenu(ContextMenuContextData&& contextMenuContextDa > // 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);
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
timothy
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 192912
: 357844