[webkit-reviews] review granted: [Bug 180826] self.importScripts() should obey updateViaCache inside service workers : [Attachment 329390] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 14 14:44:43 PST 2017


youenn fablet <youennf at gmail.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 180826: self.importScripts() should obey updateViaCache inside service
workers
https://bugs.webkit.org/show_bug.cgi?id=180826

Attachment 329390: Patch

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




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

r=me.
These tests would also be handy as WPT tests.
It would be good for future service worker tests that do not require any
internals to be in LayoutTests/wpt/service-workers.
That will make it easy to upload them to WPT if we are in hurry right now.

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

> Source/WebCore/workers/service/SWClientConnection.cpp:213
> +

This dual-pattern is handy but at the same time somehow wrong:
- In ServiceWorkerProcess, we will iterate through SWContextManager (good) and
all dummy documents (useless)
- In WebProcess, we will iterate through documents (good) and SWContextManager
(useless and we will create one for that reason).
Wonder whether there would be a better pattern here.

> Source/WebCore/workers/service/server/SWServerRegistration.h:63
> +    void setLastUpdateTime(WallTime);

It seems SWServerRegistration m_lastUpdateTime is only set when we install a
context.
If so, there might be a bug since it should also be set when creating a
SWServerRegistration from the database.

I wonder how we can test such things without waiting 24 hours.
This would probably require having internals API to reload a SWServer from
database and another to tweak SWServer registrations.


More information about the webkit-reviews mailing list