Bug 73259 – [chromium] add event methods to WebFrame

• Component: WebKit API
• Status: RESOLVED FIXED

[Karl Koscher]

In preparation for getting cross-process postMessage working in Chromium, we need an API to send message events to frames. We also need an API for adding event handlers to frames so that we can intercept message events to our proxy frames. While we could dispatch the message event to the document and let it bubble, we can't capture message events on the document (they are directly dispatched to the DOMWindow), so we at least need methods to add and remove event handlers. While we're at it, we might as well add an API so we can dispatch messages directly as well to be consistent with the same-process behavior.

[Karl Koscher]

Created attachment 116845 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

[Darin Fisher (:fishd, Google)]

Comment on attachment 116845 [details]
Patch

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())

[Karl Koscher]

Created attachment 116857 [details]
Patch

[Karl Koscher]

Comment on attachment 116845 [details]
Patch

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.

Fixed.

>> 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?

Comment added.

I talked with Charlie and we both agree that it's an acceptable risk. WebKit already exposes these functions internally. Any users of this API will need to consider the fact that these events might be synthetic, though. I added a comment to that effect.

>> 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())

Fixed.

[WebKit Review Bot]

Comment on attachment 116857 [details]
Patch

Rejecting attachment 116857 [details] from commit-queue.

New failing tests:
svg/transforms/text-with-pattern-with-svg-transform.svg
Full output: http://queues.webkit.org/results/10696086

[WebKit Review Bot]

Comment on attachment 116857 [details]
Patch

Clearing flags on attachment: 116857

Committed r102495: <http://trac.webkit.org/changeset/102495>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.