[webkit-reviews] review denied: [Bug 73259] [chromium] add event methods to WebFrame : [Attachment 116845] Patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Karl Koscher
<supersat at chromium.org>'s request for review:
Bug 73259: [chromium] add event methods to WebFrame
https://bugs.webkit.org/show_bug.cgi?id=73259

Attachment 116845: Patch
https://bugs.webkit.org/attachment.cgi?id=116845&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=116845&action=review


> Source/WebKit/chromium/public/WebFrame.h:546
> +    // Events --------------------------------------------------------------


nit: please preserve two new lines above section headers and one new line
below.

> Source/WebKit/chromium/public/WebFrame.h:547
> +    virtual void addEventListener(const WebString& eventType,

Please add a comment indicating that these operate on the DOM Window.

Do you care if web content is able to generate synthetic DOM events that you
intercept?  Do we need to worry about that introducing security holes through
privilege escalation?

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1861
> +    if (!event.isNull())

I think you should be able to assume that you are given a non-null event.  I
would just ASSERT(!event.isNull())