[webkit-reviews] review granted: [Bug 57045] Eliminate Node::dispatchGenericEvent. : [Attachment 86816] Removed unnecessary extra ref and stale comment.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Dimitri Glazkov (Google)
<dglazkov at chromium.org>'s request for review:
Bug 57045: Eliminate Node::dispatchGenericEvent.
https://bugs.webkit.org/show_bug.cgi?id=57045

Attachment 86816: Removed unnecessary extra ref and stale comment.
https://bugs.webkit.org/attachment.cgi?id=86816&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=86816&action=review

Wow, this is a really great thing to do. Who on earth every understood the
difference between dispatchGenericEvent and dispatchEvent!?

> Source/WebCore/dom/Node.cpp:2699
> +    RefPtr<FrameView> view = document()->view();

Since this is never used, I think this should be named protectView instead of
just view. I also think it makes sense to put this with the other protect
before doing any work with the event.

> Source/WebCore/svg/SVGElementInstance.cpp:142
>      SVGElement* element = shadowTreeElement();
>      if (!element)
>	   return false;
>  
> -    RefPtr<FrameView> view = element->document()->view();
> -    return element->dispatchGenericEvent(event.release());
> +    return element->dispatchEvent(event);

I can’t resist pointing out the way I like to write these:

    SVGElement* element = shadowTreeElement();
    return element && element->dispatchEvent(event);

But I will set commit-queue+ anyway, because it probably is clearer the way
it’s written here anyway.