Bug 175650 – Bounce ServiceWorker jobs to the Storage Process

• Component: WebKit2
• Status: RESOLVED FIXED

[Brady Eidson]

Bounce SW jobs to the Storage Process

[Brady Eidson]

Created attachment 318308 [details]
WIP

Attaching incomplete WIP to get from a different machine later

[Brady Eidson]

Created attachment 318327 [details]
EWS run

[Brady Eidson]

Created attachment 318328 [details]
EWS run

[Build Bot]

Attachment 318328 [details] did not pass style-queue:


ERROR: Source/WebKit/WebProcess/Storage/WebServiceWorkerProvider.cpp:32:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/workers/service/ServiceWorkerServer.h:40:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/workers/service/ServiceWorkerServer.cpp:34:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
Total errors found: 3 in 31 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brady Eidson]

Created attachment 318330 [details]
EWS

[Build Bot]

Attachment 318330 [details] did not pass style-queue:


ERROR: Source/WebKit/WebProcess/Storage/WebServiceWorkerProvider.cpp:32:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/workers/service/ServiceWorkerServer.h:40:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/workers/service/ServiceWorkerServer.cpp:34:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
Total errors found: 3 in 31 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 318330 [details]
EWS

Attachment 318330 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4328583

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 318333 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 318330 [details]
EWS

Attachment 318330 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4328611

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 318334 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Brady Eidson]

Created attachment 318372 [details]
EWS

[Brady Eidson]

Created attachment 318385 [details]
EWS

[Build Bot]

Comment on attachment 318385 [details]
EWS

Attachment 318385 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4331512

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 318392 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Brady Eidson]

Created attachment 318400 [details]
EWS

[Brady Eidson]

Created attachment 318403 [details]
EWS

[Build Bot]

Comment on attachment 318403 [details]
EWS

Attachment 318403 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4332274

New failing tests:
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-count.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-location-worker.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-count-worker.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-location.html

[Build Bot]

Created attachment 318416 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 318403 [details]
EWS

Attachment 318403 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4332287

New failing tests:
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-count.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-location-worker.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-count-worker.html
imported/w3c/web-platform-tests/background-fetch/interfaces-worker.https.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-location.html

[Build Bot]

Created attachment 318417 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 318403 [details]
EWS

Attachment 318403 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4332255

New failing tests:
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-count.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-location-worker.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-count-worker.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-location.html

[Build Bot]

Created attachment 318419 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Brady Eidson]

Of the 5 reproducible failures:
- imported/w3c/web-platform-tests/background-fetch/interfaces-worker.https.html needs updated results
- The ones in imported/w3c/web-platform-tests/fetch/api/redirect/redirect-count-worker.html are a huge question mark.

[Brady Eidson]

If I run this locally:
`run-webkit-tests imported/w3c/web-platform-tests/fetch/api/redirect`

Two tests that are expected to fail... instead pass!

Including imported/w3c/web-platform-tests/fetch/api/redirect/redirect-location.html which fails here in EWS =/

[Brady Eidson]

Created attachment 318424 [details]
EWS

[Brady Eidson]

Created attachment 318428 [details]
EWS (accidentally rebased some tests I didn't mean to!)

[Brady Eidson]

Created attachment 318429 [details]
Full set of EWS bots, maybe?

[Brady Eidson]

Created attachment 318437 [details]
EWS

[Brady Eidson]

Created attachment 318439 [details]
EWS

[Brady Eidson]

Looks like this one might stick. Finally.

I'll go make ChangeLogs

[Brady Eidson]

Created attachment 318445 [details]
Patch

[Brady Eidson]

Created attachment 318495 [details]
Patch

[Brady Eidson]

(In reply to Brady Eidson from comment #32)
> Created attachment 318495 [details]
> Patch

(Having started on the next patch, quickly learned "ServiceWorkerServer" was too unwieldy, renamed to "SWServer")

[Build Bot]

Attachment 318495 [details] did not pass style-queue:


ERROR: Source/WebCore/workers/service/ServiceWorkerContainer.h:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/CMakeLists.txt:2998:  Alphabetical sorting problem. "workers/service/SWServer.cpp" should be before "workers/service/ServiceWorkerRegistrationParameters.cpp".  [list/order] [5]
ERROR: Source/WebKit/CMakeLists.txt:527:  Alphabetical sorting problem. "WebProcess/Storage/WebSWServerConnection.cpp" should be before "WebProcess/Storage/WebServiceWorkerProvider.cpp".  [list/order] [5]
Total errors found: 3 in 33 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brady Eidson]

Created attachment 318501 [details]
Patch

[Andy Estes]

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

[Brady Eidson]

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

[Brady Eidson]

Created attachment 318515 [details]
PFL

[Build Bot]

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.

[Brady Eidson]

Created attachment 318517 [details]
PFL

[WebKit Commit Bot]

Comment on attachment 318517 [details]
PFL

Clearing flags on attachment: 318517

Committed r220924: <http://trac.webkit.org/changeset/220924>

[Radar WebKit Bug Importer]

<rdar://problem/34026883>