Bug 102468 - Simulated events instances do not all have the same underlying event
Summary: Simulated events instances do not all have the same underlying event
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.8
: P2 Normal
Assignee: Jon Lee
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-11-16 00:22 PST by Jon Lee
Modified: 2012-11-17 22:10 PST (History)
6 users (show)

See Also:


Attachments
Patch (3.04 KB, patch)
2012-11-16 01:16 PST, Jon Lee
no flags Details | Formatted Diff | Diff
Patch (4.32 KB, patch)
2012-11-16 09:49 PST, Jon Lee
no flags Details | Formatted Diff | Diff
Patch (4.58 KB, patch)
2012-11-16 12:02 PST, Jon Lee
ap: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jon Lee 2012-11-16 00:22:29 PST
I believe this is a bug in how simulated events are constructed. The PassRefPtr with the underlying event is included as an argument for the mouse down, up, and click events. But the PassRefPtr loses its underlying pointer after the first simulated mouse down event because it gets assigned to that event's private m_underlyingEvent variable. We therefore send NULL to the other events.
Comment 1 Jon Lee 2012-11-16 00:58:30 PST
<rdar://problem/12716331>
Comment 2 Jon Lee 2012-11-16 01:16:13 PST
Created attachment 174630 [details]
Patch
Comment 3 mitz 2012-11-16 08:38:50 PST
Please follow the advice of <http://www.webkit.org/coding/RefPtr.html> and rename the function parameter prpUnderlyingEvent, and transfer to a RefPtr with the old name at the beginning of the function.
Comment 4 Brady Eidson 2012-11-16 08:58:32 PST
(In reply to comment #3)
> Please follow the advice of <http://www.webkit.org/coding/RefPtr.html> and rename the function parameter prpUnderlyingEvent, and transfer to a RefPtr with the old name at the beginning of the function.

Yay!  Idioms!!!
Comment 5 Brady Eidson 2012-11-16 08:59:10 PST
Comment on attachment 174630 [details]
Patch

Yay idioms! (do the prp thing)
Comment 6 Alexey Proskuryakov 2012-11-16 09:45:21 PST
A better solution (and one you should use) would be to change the argument type form PassRefPtr to a plain pointer.
Comment 7 Brady Eidson 2012-11-16 09:47:22 PST
(In reply to comment #6)
> A better solution (and one you should use) would be to change the argument type form PassRefPtr to a plain pointer.

Great point - ownership (a reference) is not actually being passed in this case, and the arguments to the 3 callees are all plain ptrs.
Comment 8 Jon Lee 2012-11-16 09:49:12 PST
Created attachment 174711 [details]
Patch
Comment 9 Alexey Proskuryakov 2012-11-16 10:06:08 PST
Comment on attachment 174711 [details]
Patch

Please do use plain pointers.
Comment 10 Jon Lee 2012-11-16 12:02:54 PST
Created attachment 174736 [details]
Patch
Comment 11 Jon Lee 2012-11-16 13:42:17 PST
Committed 134995: http://trac.webkit.org/changeset/134995
Comment 12 WebKit Review Bot 2012-11-16 13:42:20 PST
Comment on attachment 174736 [details]
Patch

Attachment 174736 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14878003
Comment 13 Jon Lee 2012-11-17 22:10:16 PST
Chromium fix in r135000.