[webkit-reviews] review granted: [Bug 178497] Implement ServiceWorkerRegistration.scope / updateViaCache : [Attachment 324194] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Oct 18 18:57:52 PDT 2017


youenn fablet <youennf at gmail.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 178497: Implement ServiceWorkerRegistration.scope / updateViaCache
https://bugs.webkit.org/show_bug.cgi?id=178497

Attachment 324194: Patch

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




--- Comment #2 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 324194
  --> https://bugs.webkit.org/attachment.cgi?id=324194
Patch

LGTM once the bots are happy.

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

> Source/WebCore/workers/service/ServiceWorkerRegistration.h:66
>      virtual ScriptExecutionContext* scriptExecutionContext() const;

Can you mark these two as final as well?

> Source/WebCore/workers/service/ServiceWorkerRegistrationData.h:74
> +    decoder >> updateViaCache;

I wonder whether decoder >> key >> identifier >> scopeURL >> updateViaCache
would be something we want.
That would match the encoder version nicely but we do not return as early and
it is less debuggable.

> Source/WebCore/workers/service/ServiceWorkerRegistrationData.h:78
> +    return {{ WTFMove(*key), WTFMove(*identifier), WTFMove(*scopeURL),
WTFMove(*updateViaCache) }};

Does style say to put a space between {{ or }}?


More information about the webkit-reviews mailing list