Bug 179668 - WebSWClientConnection should do IPC to StorageProcess if its WebSWOriginTable is not yet initialized
Summary: WebSWClientConnection should do IPC to StorageProcess if its WebSWOriginTable...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-11-14 07:21 PST by youenn fablet
Modified: 2017-11-15 09:30 PST (History)
5 users (show)

See Also:


Attachments
Patch (15.98 KB, patch)
2017-11-14 07:28 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (19.69 KB, patch)
2017-11-14 10:45 PST, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2017-11-14 07:21:06 PST
WebSWClientConnection should do IPC to StorageProcess if its WebSWOriginTable is not yet initialized
Comment 1 youenn fablet 2017-11-14 07:28:52 PST
Created attachment 326877 [details]
Patch
Comment 2 Chris Dumez 2017-11-14 09:18:47 PST
Comment on attachment 326877 [details]
Patch

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

> Source/WebKit/WebProcess/Storage/WebSWOriginTable.h:43
> +    bool isInitialized() const { return m_serviceWorkerOriginTable.sharedMemory(); }

The StorageProcess only sends a SharedMemoryHandle once the store actually contains at least one item. A store can be initialized and have no shared memory, in which case it merely means it is empty. Going to the StorageProcess in such cases is wrong and unnecessarily inefficient.

> LayoutTests/ChangeLog:8
> +        * http/tests/workers/service/basic-unregister.https-expected.txt:

Please explain the test changes.
Comment 3 youenn fablet 2017-11-14 10:34:17 PST
(In reply to Chris Dumez from comment #2)
> Comment on attachment 326877 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=326877&action=review
> 
> > Source/WebKit/WebProcess/Storage/WebSWOriginTable.h:43
> > +    bool isInitialized() const { return m_serviceWorkerOriginTable.sharedMemory(); }
> 
> The StorageProcess only sends a SharedMemoryHandle once the store actually
> contains at least one item. A store can be initialized and have no shared
> memory, in which case it merely means it is empty. Going to the
> StorageProcess in such cases is wrong and unnecessarily inefficient.

OK, let's make StorageProcess do an IPC to state that the table is empty then.
Comment 4 youenn fablet 2017-11-14 10:45:53 PST
Created attachment 326889 [details]
Patch
Comment 5 Chris Dumez 2017-11-14 10:56:23 PST
Comment on attachment 326889 [details]
Patch

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

> LayoutTests/ChangeLog:8
> +        Updated tests to use hasServiceWorkerRegistration instead of hasServiceWorkerRegisteredForOrigin.

Since you kill all the call sites to hasServiceWorkerRegisteredForOrigin(), please kill internals.hasServiceWorkerRegisteredForOrigin() too.
Comment 6 youenn fablet 2017-11-14 11:13:57 PST
> > LayoutTests/ChangeLog:8
> > +        Updated tests to use hasServiceWorkerRegistration instead of hasServiceWorkerRegisteredForOrigin.
> 
> Since you kill all the call sites to hasServiceWorkerRegisteredForOrigin(),
> please kill internals.hasServiceWorkerRegisteredForOrigin() too.

It is removed from Internals.cpp/h/idl.
Comment 7 Chris Dumez 2017-11-14 11:48:56 PST
Comment on attachment 326889 [details]
Patch

r=me
Comment 8 WebKit Commit Bot 2017-11-14 12:28:38 PST
Comment on attachment 326889 [details]
Patch

Clearing flags on attachment: 326889

Committed r224832: <https://trac.webkit.org/changeset/224832>
Comment 9 WebKit Commit Bot 2017-11-14 12:28:40 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2017-11-15 09:30:50 PST
<rdar://problem/35561838>