[webkit-reviews] review granted: [Bug 217980] Merge WorkerScriptController and WorkletScriptController into WorkerOrWorkletScriptController : [Attachment 411905] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 20 14:43:32 PDT 2020

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 217980: Merge WorkerScriptController and WorkletScriptController into

Attachment 411905: Patch


--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 411905
  --> https://bugs.webkit.org/attachment.cgi?id=411905

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

> Source/WebCore/bindings/js/JSDOMGlobalObject.cpp:299
> +    auto guardedObjects = m_guardedObjects;

We are copying this so it does the right thing even if some objects are removed
while we iterate. Three questions:

1) Can we make clearer that this is a copy, perhaps by including copy in the
name of the local variable or adding a comment?
2) Is it OK to call clear if this object is removed and then perhaps reused in
some way?
3) Is it OK that this does not clear objects that are added during the

Looks like this code was just moved, so none of this is new.

> Source/WebCore/bindings/js/JSDOMGlobalObject.h:87
> +    JSC::JSProxy* proxy() const { ASSERT(m_proxy); return m_proxy.get(); }

If we can assert this, it seems we should return a reference, not a pointer.

> Source/WebCore/bindings/js/JSDOMWindowBase.cpp:253
>  JSWindowProxy* JSDOMWindowBase::proxy() const


More information about the webkit-reviews mailing list