[webkit-reviews] review granted: [Bug 163412] Introduce RunCallbackScope to implement backup incumbent settings object stack : [Attachment 449492] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jan 24 16:19:08 PST 2022

Geoffrey Garen <ggaren at apple.com> has granted Alexey Shvayka
<ashvayka at apple.com>'s request for review:
Bug 163412: Introduce RunCallbackScope to implement backup incumbent settings
object stack

Attachment 449492: Patch


--- Comment #62 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 449492
  --> https://bugs.webkit.org/attachment.cgi?id=449492

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


Seems like there's work to make EWS happy before landing.

> Source/JavaScriptCore/interpreter/Interpreter.cpp:757
> +	   SetForScope<JSGlobalObject*>
changeJSONPGlobalObject(vm.JSONPGlobalObject, globalObject);

Neat simplification.

> Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp:58
> +   
entGlobalObject(globalObject, vm.topCallFrame));

Makes me sad, but I guess we have to do it to match the spec.

> Source/JavaScriptCore/runtime/RunCallbackScope.cpp:52
> +	       if (isSameOriginDomain(topCallFrameGlobalObject,
m_incumbentGlobalObject) == TriState::True)
> +		   return m_incumbentGlobalObject;

I wonder if we should add an ASSERT_WITH_SECURITY_IMPLICATION here. We don't
expect these to be different. Something like:

auto isSameOriginDomain =
FrameGlobalObject, m_incumbentGlobalObject) == TriState::True;
if (isSameOriginDomain)
    return m_incumbentGlobalObject;

Anyway, I like this compromise. It seems to ensure that the disappointing
complexity of the incumbent global object spec can only create compatibility
issues, and not security issues.

> Source/JavaScriptCore/runtime/RunCallbackScope.h:33
> +class JS_EXPORT_PRIVATE RunCallbackScope final {

I wonder if we should name this "AuthorCallbackScope" to clarify that you
should deploy it any time you call into code that the spec would label "author"
code. Or some other word to indicate "I'm calling out to author-specific

I'm not sure that "Run" adds information, so "CallbackScope" might also be OK.

More information about the webkit-reviews mailing list