PluginProxy::handleMouseEvent() forwards the generated mouse event to PluginControllerProxy using a synchronous message, but the recipient always reply immediately with the same value("true") even before handling the received message. So I think PluginProxy::handleMouseEvent() is perfectly OK to process its messages asynchronously.
Created attachment 255177 [details] Patch
Comment on attachment 255177 [details] Patch Attachment 255177 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5146996457340928 New failing tests: plugins/mouse-events.html
Created attachment 255179 [details] Archive of layout-test-results from ews105 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Hmm... plugins/mouse-events.html failed. But I'm just wondering why the expectation file in EWS differ from the one in ToT? // plugins/mouse-events-expected.txt in EWS (taken from the above result attachment) Test for bug 11517: Flash clicks/interactivity not working properly. PLUGIN: mouseDown at (12, 12) PLUGIN: mouseUp at (12, 12) PLUGIN: mouseDown at (22, 22) PLUGIN: mouseUp at (32, 22) PLUGIN: getFocusEvent PASS successfullyParsed is true TEST COMPLETE // plugins/mouse-events-expected.txt in ToT (https://trac.webkit.org/browser/trunk/LayoutTests/plugins/mouse-events-expected.txt) Test for bug 11517: Flash clicks/interactivity not working properly. PLUGIN: getFocusEvent PLUGIN: mouseDown at (12, 12) PLUGIN: mouseUp at (12, 12) PLUGIN: mouseDown at (22, 22) PLUGIN: mouseUp at (32, 22) PASS successfullyParsed is true TEST COMPLETE
EWS uses expected results from platform/mac-mavericks.
Or from platform/mac if there are no Mavericks specific results.
Created attachment 255410 [details] Patch
@Alexey Proskuryakov: Thank you for pointing that out! platform/mac-wk2/plugins/mouse-events-expected.txt was introduced by webkit.org/b/116665 to avoid flakey tests. I think from now on we can share the common expectations.
Anders, what do you think about this patch? It seems OK to me but I am not sure.
(In reply to comment #9) > Anders, what do you think about this patch? It seems OK to me but I am not > sure. I think it's a good patch. I'm going to review it.
Comment on attachment 255410 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255410&action=review Patch looks good overall, but I'd like to see a new patch without the comment. > Source/WebKit2/WebProcess/Plugins/PluginProxy.cpp:374 > + // Always let the web process think that we've handled this mouse event, even before the plug-in handles it. > + // This is a workaround for <rdar://problem/9299901>. If the web process send a HandleMouseEvent synchronously > + // like the other mouse events, the plug-in process spawns a nested run loop when showing a context menu, > + // so eventually the unresponsiveness timer kicks in in the UI process. > + // > + // FIXME: We should come up with a better way to do this. I don't think this comment here is needed anymore.
Created attachment 257187 [details] Patch
(In reply to comment #11) > Comment on attachment 255410 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=255410&action=review > > Patch looks good overall, but I'd like to see a new patch without the > comment. > > > Source/WebKit2/WebProcess/Plugins/PluginProxy.cpp:374 > > + // Always let the web process think that we've handled this mouse event, even before the plug-in handles it. > > + // This is a workaround for <rdar://problem/9299901>. If the web process send a HandleMouseEvent synchronously > > + // like the other mouse events, the plug-in process spawns a nested run loop when showing a context menu, > > + // so eventually the unresponsiveness timer kicks in in the UI process. > > + // > > + // FIXME: We should come up with a better way to do this. > > I don't think this comment here is needed anymore. Thanks Anders. I removed the comment. Would you please take a look again?
Comment on attachment 257187 [details] Patch We can probably do the same thing for the mouseEnter/mouseLeave events too.
Comment on attachment 257187 [details] Patch Clearing flags on attachment: 257187 Committed r187114: <http://trac.webkit.org/changeset/187114>
All reviewed patches have been landed. Closing bug.
(In reply to comment #14) > Comment on attachment 257187 [details] > Patch > > We can probably do the same thing for the mouseEnter/mouseLeave events too. OK. I'll investigate it. Thanks.