Bug 122742 – Reintroduce PassRefPtr<Event> copy in ScopedEventQueue::dispatchEvent

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Zan Dobersek]

Reintroduce PassRefPtr<Event> copy in ScopedEventQueue::dispatchEvent

[Zan Dobersek]

Created attachment 214136 [details]
Patch

[Alberto Garcia]

lgtm

[Zan Dobersek]

Comment on attachment 214136 [details]
Patch

Clearing flags on attachment: 214136

Committed r157401: <http://trac.webkit.org/changeset/157401>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 214136 [details]
Patch

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

> Source/WebCore/dom/ScopedEventQueue.cpp:86
> +    // Passing the PassRefPtr<Event> object into the method call creates a new copy and also nullifies
> +    // the original object, which is causing crashes in GCC-compiled code that only after that goes on
> +    // to retrieve the Event's target, calling Event::target() on the now-null PassRefPtr<Event> object.
> +    Node* node = event->target()->toNode();
> +    EventDispatcher::dispatchEvent(node, event);

This is a totally confusing comment.

The bug is no surprise. If we put the node computation into the function call we’d be depending on undefined behavior. But the comment should say something more like this:

    // Put node in local variable to make sure we don’t dereference the event after it's nulled out to pass it in.

The mentions of things like "create a new copy" and "crashes in GCC-compiled code" make the comment just landed confusing and it's *so* long for such a minor issue.