[webkit-reviews] review granted: [Bug 239332] [JSC] ShadowRealm global object has a mutable prototype : [Attachment 457622] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has granted Caitlin Potter (:caitp)
<caitp at igalia.com>'s request for review:
Bug 239332: [JSC] ShadowRealm global object has a mutable prototype
https://bugs.webkit.org/show_bug.cgi?id=239332

Attachment 457622: Patch

https://bugs.webkit.org/attachment.cgi?id=457622&action=review




--- Comment #5 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 457622
  --> https://bugs.webkit.org/attachment.cgi?id=457622
Patch

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

> Source/JavaScriptCore/runtime/JSObject.cpp:1921
> +    // Default realm global objects should have mutable prototypes despite
having
> +    // a Proxy globalThis.
> +    ASSERT((this->isGlobalObject() &&
this->globalObject()->isOrdinaryObject())
> +	   || methodTable(vm)->toThis(this, globalObject, ECMAMode::sloppy())
== this);

How about just checking `this->isGlobalObject() ||
(methodTable(vm)->toThis(this, globalObject, ECMAMode::sloppy()) == this)`?
GlobalObject has isImmutablePrototypeExoticObject bit by default. So, reaching
here only when we have an exceptional global object (ShadowRealmGlobalObject).
If we have a comment about it, then I think `this->isGlobalObject()` is enough
and we do not need to have isOrdinaryObject() and its bool field :)