[webkit-reviews] review granted: [Bug 163412] REGRESSION: MessageEvent.source for bound postMessage() is wrong for same-origin iframe : [Attachment 307623] Patch and layout tests
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Apr 21 14:03:12 PDT 2017
Geoffrey Garen <ggaren at apple.com> has granted Daniel Bates
<dbates at webkit.org>'s request for review:
Bug 163412: REGRESSION: MessageEvent.source for bound postMessage() is wrong
for same-origin iframe
https://bugs.webkit.org/show_bug.cgi?id=163412
Attachment 307623: Patch and layout tests
https://bugs.webkit.org/attachment.cgi?id=307623&action=review
--- Comment #22 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 307623
--> https://bugs.webkit.org/attachment.cgi?id=307623
Patch and layout tests
View in context: https://bugs.webkit.org/attachment.cgi?id=307623&action=review
It seems like we have two names for this new kind of global object: "incumbent"
and "caller". I think we should try to use "incumbent" in as many places as
possible, if that's the spec name.
> Source/WebCore/bindings/js/JSDOMWindowBase.cpp:306
> + return StackVisitor::Status::Done;
Seems like it would be better to continue in this case, and treat WASM like
bound functions, rather than giving up. Giving up will always result in a less
accurate result.
> Source/WebCore/bindings/js/JSDOMWindowBase.cpp:336
> + // The currently running code was scheduled to run, say via
setTimeout(). Return the window
> + // associated with the code that scheduled us.
This comment is not the same as the comment in VM.h. I prefer the comment in
VM.h. I suggest removing this comment, and if there's something unique you like
about it, moving that to VM.h, so there's a single place where we explain the
concept of incumbentGlobalObject.
> Source/WebCore/bindings/js/ScheduledAction.cpp:135
> + vm.setIncumbentGlobalObject(nullptr);
I wonder if you can make this automatic using something like WTF::SetForScope
or bmalloc::ScopeExit. This is a strong reference, so it would be nice to have
an automatic idiom for clearing it. You could also make a dedicated
IncumbentGlobalObjectScope class.
More information about the webkit-reviews
mailing list