Bug 180826 – self.importScripts() should obey updateViaCache inside service workers

• Status: RESOLVED
• Resolution: FIXED
• Component: Service Workers

[Chris Dumez]

self.importScripts() should obey updateViaCache inside service workers:
- https://html.spec.whatwg.org/multipage/workers.html#dom-workerglobalscope-importscripts

[Chris Dumez]

Created attachment 329390 [details]
Patch

[youenn fablet]

Comment on attachment 329390 [details]
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.

[Chris Dumez]

Comment on attachment 329390 [details]
Patch

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.

Not a new issue but I agree it'd be nice to find a better pattern.

>> 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.

This is not true, lastUpdateTime is saved to disk and restored from disk.

[Chris Dumez]

Comment on attachment 329390 [details]
Patch

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

>>> 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.
> 
> This is not true, lastUpdateTime is saved to disk and restored from disk.

Actually, it looks like even though we save it to the database, it is not properly restored. I'll look into fixing this in a follow-up.

[Chris Dumez]

Created attachment 329406 [details]
Patch

[Chris Dumez]

Comment on attachment 329406 [details]
Patch

Clearing flags on attachment: 329406

Committed r225940: <https://trac.webkit.org/changeset/225940>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/36061334>