WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
146142
Make PluginProxy::handleMouseEvent() asynchronous
https://bugs.webkit.org/show_bug.cgi?id=146142
Summary
Make PluginProxy::handleMouseEvent() asynchronous
Sungmann Cho
Reported
2015-06-18 22:59:41 PDT
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.
Attachments
Patch
(5.76 KB, patch)
2015-06-18 23:02 PDT
,
Sungmann Cho
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews105 for mac-mavericks-wk2
(570.29 KB, application/zip)
2015-06-18 23:38 PDT
,
Build Bot
no flags
Details
Patch
(7.62 KB, patch)
2015-06-23 06:47 PDT
,
Sungmann Cho
no flags
Details
Formatted Diff
Diff
Patch
(7.10 KB, patch)
2015-07-21 11:09 PDT
,
Sungmann Cho
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Sungmann Cho
Comment 1
2015-06-18 23:02:08 PDT
Created
attachment 255177
[details]
Patch
Build Bot
Comment 2
2015-06-18 23:38:50 PDT
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
Build Bot
Comment 3
2015-06-18 23:38:52 PDT
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
Sungmann Cho
Comment 4
2015-06-19 17:43:24 PDT
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
Alexey Proskuryakov
Comment 5
2015-06-22 21:31:19 PDT
EWS uses expected results from platform/mac-mavericks.
Alexey Proskuryakov
Comment 6
2015-06-22 21:32:13 PDT
Or from platform/mac if there are no Mavericks specific results.
Sungmann Cho
Comment 7
2015-06-23 06:47:30 PDT
Created
attachment 255410
[details]
Patch
Sungmann Cho
Comment 8
2015-06-23 06:51:22 PDT
@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.
Darin Adler
Comment 9
2015-07-20 18:31:00 PDT
Anders, what do you think about this patch? It seems OK to me but I am not sure.
Anders Carlsson
Comment 10
2015-07-21 10:38:38 PDT
(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.
Anders Carlsson
Comment 11
2015-07-21 10:39:41 PDT
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.
Sungmann Cho
Comment 12
2015-07-21 11:09:46 PDT
Created
attachment 257187
[details]
Patch
Sungmann Cho
Comment 13
2015-07-21 11:20:09 PDT
(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?
Anders Carlsson
Comment 14
2015-07-21 11:35:44 PDT
Comment on
attachment 257187
[details]
Patch We can probably do the same thing for the mouseEnter/mouseLeave events too.
WebKit Commit Bot
Comment 15
2015-07-21 12:53:47 PDT
Comment on
attachment 257187
[details]
Patch Clearing flags on attachment: 257187 Committed
r187114
: <
http://trac.webkit.org/changeset/187114
>
WebKit Commit Bot
Comment 16
2015-07-21 12:53:53 PDT
All reviewed patches have been landed. Closing bug.
Sungmann Cho
Comment 17
2015-07-21 15:50:55 PDT
(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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug