[webkit-reviews] review granted: [Bug 178187] SW "Hello world" : [Attachment 323625] Patch (Changelog to come, still reviewable now)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 12 20:55:20 PDT 2017


Andy Estes <aestes at apple.com> has granted Brady Eidson <beidson at apple.com>'s
request for review:
Bug 178187: SW "Hello world"
https://bugs.webkit.org/show_bug.cgi?id=178187

Attachment 323625: Patch (Changelog to come, still reviewable now)

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




--- Comment #6 from Andy Estes <aestes at apple.com> ---
Comment on attachment 323625
  --> https://bugs.webkit.org/attachment.cgi?id=323625
Patch (Changelog to come, still reviewable now)

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

> Source/WebCore/bindings/js/WorkerScriptController.cpp:112
> +	   Structure* contextPrototypeStructure =
JSServiceWorkerGlobalScopePrototype::createStructure(*m_vm, nullptr, jsNull());
> +	   Strong<JSServiceWorkerGlobalScopePrototype> contextPrototype(*m_vm,
JSServiceWorkerGlobalScopePrototype::create(*m_vm, nullptr,
contextPrototypeStructure));
> +	   Structure* structure =
JSServiceWorkerGlobalScope::createStructure(*m_vm, nullptr,
contextPrototype.get());
> +	   auto* proxyStructure = JSProxy::createStructure(*m_vm, nullptr,
jsNull(), PureForwardingProxyType);
> +	   auto* proxy = JSProxy::create(*m_vm, proxyStructure);
> +    
> +	   m_workerGlobalScopeWrapper.set(*m_vm,
JSServiceWorkerGlobalScope::create(*m_vm, structure,
static_cast<ServiceWorkerGlobalScope&>(*m_workerGlobalScope), proxy));
> +	   contextPrototypeStructure->setGlobalObject(*m_vm,
m_workerGlobalScopeWrapper.get());
> +	   ASSERT(structure->globalObject() == m_workerGlobalScopeWrapper);
> +	   ASSERT(m_workerGlobalScopeWrapper->structure()->globalObject() ==
m_workerGlobalScopeWrapper);
> +	   contextPrototype->structure()->setGlobalObject(*m_vm,
m_workerGlobalScopeWrapper.get());
> +	   contextPrototype->structure()->setPrototypeWithoutTransition(*m_vm,
JSWorkerGlobalScope::prototype(*m_vm, *m_workerGlobalScopeWrapper.get()));
> +
> +	   proxy->setTarget(*m_vm, m_workerGlobalScopeWrapper.get());
> +	   proxy->structure()->setGlobalObject(*m_vm,
m_workerGlobalScopeWrapper.get());

Since there's very little difference between this and the the code for
dedicated workers above, I'd consider factoring this out into a function
template that takes the prototype and global scope class names as parameters.

> Source/WebCore/workers/service/ServiceWorkerGlobalScope.h:38
> +    

whitespace

> Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:76
> +static uint64_t nextUniqueContextIdentifier()
> +{
> +    ASSERT(isMainThread());
> +    static uint64_t currentUniqueContextIdentifier;
> +    return ++currentUniqueContextIdentifier;
> +}

Can ServiceWorkerThread use Identified<> instead?


More information about the webkit-reviews mailing list