175650 – Bounce ServiceWorker jobs to the Storage Process

RESOLVED FIXED 175650

Bounce ServiceWorker jobs to the Storage Process

https://bugs.webkit.org/show_bug.cgi?id=175650

Summary Bounce ServiceWorker jobs to the Storage Process

Brady Eidson

Reported 2017-08-16 17:08:50 PDT

Bounce SW jobs to the Storage Process

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
Show Obsolete (22) View All Add attachment proposed patch, testcase, etc.

Brady Eidson

Comment 1 2017-08-16 17:09:47 PDT Comment hidden (obsolete)

Created attachment 318308 [details] WIP Attaching incomplete WIP to get from a different machine later

Brady Eidson

Comment 2 2017-08-16 22:32:50 PDT Comment hidden (obsolete)

Created attachment 318327 [details] EWS run

Brady Eidson

Comment 3 2017-08-16 22:35:39 PDT Comment hidden (obsolete)

Created attachment 318328 [details] EWS run

Build Bot

Comment 4 2017-08-16 22:59:01 PDT Comment hidden (obsolete)

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

Comment 5 2017-08-16 23:01:24 PDT Comment hidden (obsolete)

Created attachment 318330 [details] EWS

Build Bot

Comment 6 2017-08-16 23:03:06 PDT Comment hidden (obsolete)

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 7 2017-08-17 00:03:43 PDT Comment hidden (obsolete)

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

Comment 8 2017-08-17 00:03:44 PDT Comment hidden (obsolete)

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 9 2017-08-17 00:29:30 PDT Comment hidden (obsolete)

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

Comment 10 2017-08-17 00:29:31 PDT Comment hidden (obsolete)

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

Comment 11 2017-08-17 09:18:34 PDT Comment hidden (obsolete)

Created attachment 318372 [details] EWS

Brady Eidson

Comment 12 2017-08-17 10:59:47 PDT Comment hidden (obsolete)

Created attachment 318385 [details] EWS

Build Bot

Comment 13 2017-08-17 11:37:48 PDT Comment hidden (obsolete)

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

Comment 14 2017-08-17 11:37:51 PDT Comment hidden (obsolete)

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

Comment 15 2017-08-17 12:34:50 PDT Comment hidden (obsolete)

Created attachment 318400 [details] EWS

Brady Eidson

Comment 16 2017-08-17 12:48:53 PDT Comment hidden (obsolete)

Created attachment 318403 [details] EWS

Build Bot

Comment 17 2017-08-17 14:05:55 PDT Comment hidden (obsolete)

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

Comment 18 2017-08-17 14:05:58 PDT Comment hidden (obsolete)

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 19 2017-08-17 14:09:54 PDT Comment hidden (obsolete)

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

Comment 20 2017-08-17 14:09:56 PDT Comment hidden (obsolete)

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 21 2017-08-17 14:16:26 PDT Comment hidden (obsolete)

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

Comment 22 2017-08-17 14:16:28 PDT Comment hidden (obsolete)

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

Comment 23 2017-08-17 14:26:21 PDT Comment hidden (obsolete)

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

Comment 24 2017-08-17 14:28:42 PDT Comment hidden (obsolete)

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

Comment 25 2017-08-17 14:30:52 PDT Comment hidden (obsolete)

Created attachment 318424 [details] EWS

Brady Eidson

Comment 26 2017-08-17 15:12:22 PDT Comment hidden (obsolete)

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

Brady Eidson

Comment 27 2017-08-17 15:45:29 PDT Comment hidden (obsolete)

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

Brady Eidson

Comment 28 2017-08-17 16:31:20 PDT Comment hidden (obsolete)

Created attachment 318437 [details] EWS

Brady Eidson

Comment 29 2017-08-17 16:42:06 PDT Comment hidden (obsolete)

Created attachment 318439 [details] EWS

Brady Eidson

Comment 30 2017-08-17 17:05:51 PDT Comment hidden (obsolete)

Looks like this one might stick. Finally. I'll go make ChangeLogs

Brady Eidson

Comment 31 2017-08-17 17:13:15 PDT Comment hidden (obsolete)

Created attachment 318445 [details] Patch

Brady Eidson

Comment 32 2017-08-18 07:28:40 PDT Comment hidden (obsolete)

Created attachment 318495 [details] Patch

Brady Eidson

Comment 33 2017-08-18 07:29:54 PDT Comment hidden (obsolete)

(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

Comment 34 2017-08-18 07:30:35 PDT Comment hidden (obsolete)

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

Comment 35 2017-08-18 08:51:23 PDT

Created attachment 318501 [details] Patch

Andy Estes

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

Brady Eidson

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

Brady Eidson

Comment 38 2017-08-18 10:11:40 PDT

Created attachment 318515 [details] PFL

Build Bot

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

Brady Eidson

Comment 40 2017-08-18 10:21:02 PDT

Created attachment 318517 [details] PFL

WebKit Commit Bot

Comment 41 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>

Radar WebKit Bug Importer

Comment 42 2017-08-22 18:45:34 PDT

<rdar://problem/34026883>

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware Unspecified

OS Unspecified

Product WebKit

Component WebKit2

Assignee

Brady Eidson

Reported

2017-08-16 17:08 PDT

Modified

2022-03-01 02:33 PST History

CC List

6 users Show

URL

Keywords InRadar

Depends on

Blocks