Bug 71478 – Fix up chromium API for creating events, initializing message events

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

[Dave Michael]

Fix up chromium API for creating events, initializing message events

[Dave Michael]

Created attachment 113506 [details]
Patch

[Dave Michael]

Comment on attachment 113506 [details]
Patch

Note: initMessageEvent is not used yet in Chromium, so it's safe to change until the referenced Chromium CL lands.

[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 113506 [details]
Patch

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

> Source/WebKit/chromium/src/WebDOMMessageEvent.cpp:58
> +        static_cast<const WebFrameImpl*>(sourceFrame)->frame()->domWindow();

don't you need to assign domWindow() to 'window' ?

[Dave Michael]

Created attachment 113657 [details]
Patch

[Dave Michael]

(In reply to comment #4)
> (From update of attachment 113506 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=113506&action=review
> 
> > Source/WebKit/chromium/src/WebDOMMessageEvent.cpp:58
> > +        static_cast<const WebFrameImpl*>(sourceFrame)->frame()->domWindow();
> 
> don't you need to assign domWindow() to 'window' ?
Definitely. Nice catch. Speaking of, is there a good place to add a unit test for this? Should I do that, or does the ui_test in chromium (which doesn't cover this code path right now) suffice?

[Dave Michael]

Created attachment 115228 [details]
Patch

[Dave Michael]

Comment on attachment 115228 [details]
Patch

Merged. PTAL.

[Darin Fisher (:fishd, Google)]

Comment on attachment 115228 [details]
Patch

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

> Source/WebKit/chromium/ChangeLog:6
> +- Fix a mistake when checking the exception code during event creation.

nit: fix indentation here

> Source/WebKit/chromium/src/WebDOMMessageEvent.cpp:58
> +        window =

nit: webkit style does not enforce line length limits, so this should just be on one line.

[Dave Michael]

Created attachment 115393 [details]
Patch

[WebKit Review Bot]

Comment on attachment 115393 [details]
Patch

Clearing flags on attachment: 115393

Committed r100497: <http://trac.webkit.org/changeset/100497>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.