Bug 175650 - Bounce ServiceWorker jobs to the Storage Process
Summary: Bounce ServiceWorker jobs to the Storage Process
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-08-16 17:08 PDT by Brady Eidson
Modified: 2022-03-01 02:33 PST (History)
6 users (show)

See Also:


Attachments
WIP (110.53 KB, patch)
2017-08-16 17:09 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
EWS run (56.67 KB, patch)
2017-08-16 22:32 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
EWS run (57.65 KB, patch)
2017-08-16 22:35 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
EWS (57.99 KB, patch)
2017-08-16 23:01 PDT, Brady Eidson
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-elcapitan-wk2 (1.07 MB, application/zip)
2017-08-17 00:03 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (1007.50 KB, application/zip)
2017-08-17 00:29 PDT, Build Bot
no flags Details
EWS (57.99 KB, patch)
2017-08-17 09:18 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
EWS (58.50 KB, patch)
2017-08-17 10:59 PDT, Brady Eidson
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (1.26 MB, application/zip)
2017-08-17 11:37 PDT, Build Bot
no flags Details
EWS (111.18 KB, patch)
2017-08-17 12:34 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
EWS (110.11 KB, patch)
2017-08-17 12:48 PDT, Brady Eidson
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-elcapitan (1.29 MB, application/zip)
2017-08-17 14:05 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (1.39 MB, application/zip)
2017-08-17 14:09 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-elcapitan (1.92 MB, application/zip)
2017-08-17 14:16 PDT, Build Bot
no flags Details
EWS (110.78 KB, patch)
2017-08-17 14:30 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
EWS (accidentally rebased some tests I didn't mean to!) (86.75 KB, patch)
2017-08-17 15:12 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Full set of EWS bots, maybe? (86.75 KB, patch)
2017-08-17 15:45 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
EWS (90.11 KB, patch)
2017-08-17 16:31 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
EWS (90.08 KB, patch)
2017-08-17 16:42 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (100.29 KB, patch)
2017-08-17 17:13 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (102.00 KB, patch)
2017-08-18 07:28 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (102.04 KB, patch)
2017-08-18 08:51 PDT, Brady Eidson
aestes: review+
Details | Formatted Diff | Diff
PFL (104.69 KB, patch)
2017-08-18 10:11 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
PFL (104.78 KB, patch)
2017-08-18 10:21 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2017-08-16 17:08:50 PDT
Bounce SW jobs to the Storage Process
Comment 1 Brady Eidson 2017-08-16 17:09:47 PDT Comment hidden (obsolete)
Comment 2 Brady Eidson 2017-08-16 22:32:50 PDT Comment hidden (obsolete)
Comment 3 Brady Eidson 2017-08-16 22:35:39 PDT Comment hidden (obsolete)
Comment 4 Build Bot 2017-08-16 22:59:01 PDT Comment hidden (obsolete)
Comment 5 Brady Eidson 2017-08-16 23:01:24 PDT Comment hidden (obsolete)
Comment 6 Build Bot 2017-08-16 23:03:06 PDT Comment hidden (obsolete)
Comment 7 Build Bot 2017-08-17 00:03:43 PDT Comment hidden (obsolete)
Comment 8 Build Bot 2017-08-17 00:03:44 PDT Comment hidden (obsolete)
Comment 9 Build Bot 2017-08-17 00:29:30 PDT Comment hidden (obsolete)
Comment 10 Build Bot 2017-08-17 00:29:31 PDT Comment hidden (obsolete)
Comment 11 Brady Eidson 2017-08-17 09:18:34 PDT Comment hidden (obsolete)
Comment 12 Brady Eidson 2017-08-17 10:59:47 PDT Comment hidden (obsolete)
Comment 13 Build Bot 2017-08-17 11:37:48 PDT Comment hidden (obsolete)
Comment 14 Build Bot 2017-08-17 11:37:51 PDT Comment hidden (obsolete)
Comment 15 Brady Eidson 2017-08-17 12:34:50 PDT Comment hidden (obsolete)
Comment 16 Brady Eidson 2017-08-17 12:48:53 PDT Comment hidden (obsolete)
Comment 17 Build Bot 2017-08-17 14:05:55 PDT Comment hidden (obsolete)
Comment 18 Build Bot 2017-08-17 14:05:58 PDT Comment hidden (obsolete)
Comment 19 Build Bot 2017-08-17 14:09:54 PDT Comment hidden (obsolete)
Comment 20 Build Bot 2017-08-17 14:09:56 PDT Comment hidden (obsolete)
Comment 21 Build Bot 2017-08-17 14:16:26 PDT Comment hidden (obsolete)
Comment 22 Build Bot 2017-08-17 14:16:28 PDT Comment hidden (obsolete)
Comment 23 Brady Eidson 2017-08-17 14:26:21 PDT Comment hidden (obsolete)
Comment 24 Brady Eidson 2017-08-17 14:28:42 PDT Comment hidden (obsolete)
Comment 25 Brady Eidson 2017-08-17 14:30:52 PDT Comment hidden (obsolete)
Comment 26 Brady Eidson 2017-08-17 15:12:22 PDT Comment hidden (obsolete)
Comment 27 Brady Eidson 2017-08-17 15:45:29 PDT Comment hidden (obsolete)
Comment 28 Brady Eidson 2017-08-17 16:31:20 PDT Comment hidden (obsolete)
Comment 29 Brady Eidson 2017-08-17 16:42:06 PDT Comment hidden (obsolete)
Comment 30 Brady Eidson 2017-08-17 17:05:51 PDT Comment hidden (obsolete)
Comment 31 Brady Eidson 2017-08-17 17:13:15 PDT Comment hidden (obsolete)
Comment 32 Brady Eidson 2017-08-18 07:28:40 PDT Comment hidden (obsolete)
Comment 33 Brady Eidson 2017-08-18 07:29:54 PDT Comment hidden (obsolete)
Comment 34 Build Bot 2017-08-18 07:30:35 PDT Comment hidden (obsolete)
Comment 35 Brady Eidson 2017-08-18 08:51:23 PDT
Created attachment 318501 [details]
Patch
Comment 36 Andy Estes 2017-08-18 09:21:19 PDT
Comment on attachment 318501 [details]
Patch

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

> Source/WebKit/StorageProcess/StorageToWebProcessConnection.cpp:99
> +    auto idbConnections = m_webIDBConnections;

It's not clear to me why you have to make a copy of m_webIDBConnections.

> Source/WebKit/StorageProcess/StorageToWebProcessConnection.cpp:107
> +    auto serviceWorkerConnections = m_webServiceWorkerConnections;

Ditto for m_webServiceWorkerConnections.

> Source/WebKit/WebProcess/Storage/WebSWServerConnection.h:50
> +    static RefPtr<WebSWServerConnection> create(const PAL::SessionID& sessionID)
> +    {
> +        return adoptRef(new WebSWServerConnection(sessionID));
> +    }
> +    static RefPtr<WebSWServerConnection> create(IPC::Connection& connection, uint64_t connectionIdentifier, const PAL::SessionID& sessionID)
> +    {
> +        return adoptRef(new WebSWServerConnection(connection, connectionIdentifier, sessionID));
> +    }

These create methods should probably return Refs, since the return value is always non-null (even though you might store them in RefPtrs elsewhere).
Comment 37 Brady Eidson 2017-08-18 09:57:58 PDT
(In reply to Andy Estes from comment #36)
> Comment on attachment 318501 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=318501&action=review
> 
> > Source/WebKit/StorageProcess/StorageToWebProcessConnection.cpp:99
> > +    auto idbConnections = m_webIDBConnections;
> 
> It's not clear to me why you have to make a copy of m_webIDBConnections.
> 
> > Source/WebKit/StorageProcess/StorageToWebProcessConnection.cpp:107
> > +    auto serviceWorkerConnections = m_webServiceWorkerConnections;
> 
> Ditto for m_webServiceWorkerConnections.

Answer for both of these - Otherwise we'd be mutating the map while iterating.

> > Source/WebKit/WebProcess/Storage/WebSWServerConnection.h:50
> > +    static RefPtr<WebSWServerConnection> create(const PAL::SessionID& sessionID)
> > +    {
> > +        return adoptRef(new WebSWServerConnection(sessionID));
> > +    }
> > +    static RefPtr<WebSWServerConnection> create(IPC::Connection& connection, uint64_t connectionIdentifier, const PAL::SessionID& sessionID)
> > +    {
> > +        return adoptRef(new WebSWServerConnection(connection, connectionIdentifier, sessionID));
> > +    }
> 
> These create methods should probably return Refs, since the return value is
> always non-null (even though you might store them in RefPtrs elsewhere).

Okay.
Comment 38 Brady Eidson 2017-08-18 10:11:40 PDT
Created attachment 318515 [details]
PFL
Comment 39 Build Bot 2017-08-18 10:14:44 PDT
Attachment 318515 [details] did not pass style-queue:


ERROR: Source/WebCore/workers/service/ServiceWorkerContainer.h:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
ERROR: Source/WebCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
ERROR: Source/WebKit/CMakeLists.txt:532:  Alphabetical sorting problem. "WebProcess/Storage/WebSWServerConnection.cpp" should be before "WebProcess/Storage/WebServiceWorkerProvider.cpp".  [list/order] [5]
Total errors found: 4 in 33 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 40 Brady Eidson 2017-08-18 10:21:02 PDT
Created attachment 318517 [details]
PFL
Comment 41 WebKit Commit Bot 2017-08-18 11:52:50 PDT
Comment on attachment 318517 [details]
PFL

Clearing flags on attachment: 318517

Committed r220924: <http://trac.webkit.org/changeset/220924>
Comment 42 Radar WebKit Bug Importer 2017-08-22 18:45:34 PDT
<rdar://problem/34026883>