[webkit-reviews] review denied: [Bug 234155] [Shadow Realms] Use WebCore module loaders for shadow realm importValue : [Attachment 446754] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has denied Joseph Griego
<jgriego at igalia.com>'s request for review:
Bug 234155: [Shadow Realms] Use WebCore module loaders for shadow realm
importValue
https://bugs.webkit.org/show_bug.cgi?id=234155

Attachment 446754: Patch

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




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

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

Quick comment.

> Source/WebCore/page/ShadowRealmGlobalScope.h:61
> +    RefPtr<JSC::VM> m_vm;

I don't think we need to hold vm since it should be the same to the original
JSGlobalObject.

> Source/WebCore/page/ShadowRealmGlobalScope.h:62
> +    JSC::Strong<JSDOMGlobalObject> m_incubatingWrapper;

It will leak JSGlobalObject if we have cycles. So, this is not OK.