Bug 93322 - Don't re-use the same EventDispatcher instance to dispatch events.
Summary: Don't re-use the same EventDispatcher instance to dispatch events.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hayato Ito
URL:
Keywords:
Depends on:
Blocks: 91290
  Show dependency treegraph
 
Reported: 2012-08-06 19:12 PDT by Hayato Ito
Modified: 2012-08-07 23:33 PDT (History)
6 users (show)

See Also:


Attachments
Fix dblclick event and add ASSERT (5.74 KB, patch)
2012-08-06 22:38 PDT, Hayato Ito
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hayato Ito 2012-08-06 19:12:53 PDT
Three are some member functions in EventDispatcher, which assume the instance of EventDispatcher is used for only one event.

Example:
- EventDispatcher::ensureEventAncestor calculates ancestors of node. But once the calculation is done, it never recalculates. The result of the calculation depends on the given event's type().

So it is potentially dangerous to call EventDispatcher::dispatchEvent(Event) twice with different events against the same EventDispatcher instance.
We have to make sure the EventDspatcher instance is used only once per event. Currently dispatching 'dblclick' event violates this rule.
Comment 1 Hayato Ito 2012-08-06 22:38:54 PDT
Created attachment 156868 [details]
Fix dblclick event and add ASSERT
Comment 2 Dimitri Glazkov (Google) 2012-08-07 07:10:49 PDT
Comment on attachment 156868 [details]
Fix dblclick event and add ASSERT

ok
Comment 3 WebKit Review Bot 2012-08-07 20:09:44 PDT
Comment on attachment 156868 [details]
Fix dblclick event and add ASSERT

Clearing flags on attachment: 156868

Committed r124975: <http://trac.webkit.org/changeset/124975>
Comment 4 WebKit Review Bot 2012-08-07 20:09:47 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Darin Adler 2012-08-07 23:08:39 PDT
Comment on attachment 156868 [details]
Fix dblclick event and add ASSERT

View in context: https://bugs.webkit.org/attachment.cgi?id=156868&action=review

> Source/WebCore/dom/MouseEvent.cpp:-218
> -        doubleClickEvent->initMouseEvent(eventNames().dblclickEvent, event()->bubbles(), event()->cancelable(), event()->view(),
> -                event()->detail(), event()->screenX(), event()->screenY(), event()->clientX(), event()->clientY(),
> -                event()->ctrlKey(), event()->altKey(), event()->shiftKey(), event()->metaKey(),
> -                event()->button(), relatedTarget);

This is the correct formatting for a multiple line function call in the WebKit coding style.

> Source/WebCore/dom/MouseEvent.cpp:219
> +    doubleClickEvent->initMouseEvent(eventNames().dblclickEvent, event()->bubbles(), event()->cancelable(), event()->view(),
> +                                     event()->detail(), event()->screenX(), event()->screenY(), event()->clientX(), event()->clientY(),
> +                                     event()->ctrlKey(), event()->altKey(), event()->shiftKey(), event()->metaKey(),
> +                                     event()->button(), relatedTarget);

Whereas this is incorrect.
Comment 6 Hayato Ito 2012-08-07 23:33:16 PDT
Oh. Nice catch. Thank you.
I think WebKit doesn't like a patch which only fixes style. So I'll fix it if i have a chance later.

(In reply to comment #5)
> (From update of attachment 156868 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=156868&action=review
> 
> > Source/WebCore/dom/MouseEvent.cpp:-218
> > -        doubleClickEvent->initMouseEvent(eventNames().dblclickEvent, event()->bubbles(), event()->cancelable(), event()->view(),
> > -                event()->detail(), event()->screenX(), event()->screenY(), event()->clientX(), event()->clientY(),
> > -                event()->ctrlKey(), event()->altKey(), event()->shiftKey(), event()->metaKey(),
> > -                event()->button(), relatedTarget);
> 
> This is the correct formatting for a multiple line function call in the WebKit coding style.
> 
> > Source/WebCore/dom/MouseEvent.cpp:219
> > +    doubleClickEvent->initMouseEvent(eventNames().dblclickEvent, event()->bubbles(), event()->cancelable(), event()->view(),
> > +                                     event()->detail(), event()->screenX(), event()->screenY(), event()->clientX(), event()->clientY(),
> > +                                     event()->ctrlKey(), event()->altKey(), event()->shiftKey(), event()->metaKey(),
> > +                                     event()->button(), relatedTarget);
> 
> Whereas this is incorrect.