Bug 175851 - Introduce ServerWorkerRegistration task queues
Summary: Introduce ServerWorkerRegistration task queues
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: Brady Eidson
URL:
Keywords: InRadar
Depends on: 175952
Blocks:
  Show dependency treegraph
 
Reported: 2017-08-22 14:21 PDT by Brady Eidson
Modified: 2017-08-25 11:59 PDT (History)
7 users (show)

See Also:


Attachments
Patch (68.33 KB, patch)
2017-08-22 14:26 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (1.58 MB, application/zip)
2017-08-22 15:42 PDT, Build Bot
no flags Details
Patch (69.24 KB, patch)
2017-08-22 15:57 PDT, Brady Eidson
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (2.27 MB, application/zip)
2017-08-22 17:09 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (10.12 MB, application/zip)
2017-08-22 17:32 PDT, Build Bot
no flags Details
With some logging even EWS should give me. (63.31 KB, patch)
2017-08-23 14:22 PDT, Brady Eidson
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (1.83 MB, application/zip)
2017-08-23 15:16 PDT, Build Bot
no flags Details
Trying again, flushing stdout this time (63.33 KB, patch)
2017-08-23 15:41 PDT, Brady Eidson
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-elcapitan-wk2 (1.47 MB, application/zip)
2017-08-23 17:00 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (10.10 MB, application/zip)
2017-08-23 17:27 PDT, Build Bot
no flags Details
For EWS (With test!) (66.58 KB, patch)
2017-08-24 17:11 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
For EWS (With test!) (64.08 KB, patch)
2017-08-24 17:14 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (73.75 KB, patch)
2017-08-24 19:09 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (73.69 KB, patch)
2017-08-25 10:35 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-22 14:21:49 PDT
Introduced ServerWorkerRegistration task queues

This adds a job queue per-ServiceWorkerRegistration in the storage process.

All it's used for ATM is still failing out the "register()" call's promise.
Comment 1 Brady Eidson 2017-08-22 14:26:18 PDT Comment hidden (obsolete)
Comment 2 Build Bot 2017-08-22 15:42:38 PDT Comment hidden (obsolete)
Comment 3 Build Bot 2017-08-22 15:42:40 PDT Comment hidden (obsolete)
Comment 4 Brady Eidson 2017-08-22 15:45:47 PDT
(In reply to Build Bot from comment #2)
> Comment on attachment 318799 [details]
> Patch
> 
> Attachment 318799 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-queues.webkit.org/results/4363865
> 
> New failing tests:
> http/tests/workers/service/basic-register.html

Well, considering this passes 100% locally, this will be fun to figure out.
Comment 5 Brady Eidson 2017-08-22 15:57:58 PDT Comment hidden (obsolete)
Comment 6 Brady Eidson 2017-08-22 15:58:18 PDT Comment hidden (obsolete)
Comment 7 Build Bot 2017-08-22 17:09:24 PDT Comment hidden (obsolete)
Comment 8 Build Bot 2017-08-22 17:09:25 PDT Comment hidden (obsolete)
Comment 9 Build Bot 2017-08-22 17:32:40 PDT Comment hidden (obsolete)
Comment 10 Build Bot 2017-08-22 17:32:43 PDT Comment hidden (obsolete)
Comment 11 Brady Eidson 2017-08-23 10:30:36 PDT Comment hidden (obsolete)
Comment 12 Brady Eidson 2017-08-23 14:22:58 PDT Comment hidden (obsolete)
Comment 13 Build Bot 2017-08-23 15:16:03 PDT Comment hidden (obsolete)
Comment 14 Build Bot 2017-08-23 15:16:05 PDT Comment hidden (obsolete)
Comment 15 Brady Eidson 2017-08-23 15:28:31 PDT Comment hidden (obsolete)
Comment 16 Brady Eidson 2017-08-23 15:41:42 PDT Comment hidden (obsolete)
Comment 17 Brady Eidson 2017-08-23 16:50:05 PDT
Okay so I can reproduce locally with a release build (which is what the EWS bot is using)

I can *not* reproduce locally with a debug build.

Now exploring what's different between those two...
Comment 18 Build Bot 2017-08-23 17:00:03 PDT Comment hidden (obsolete)
Comment 19 Build Bot 2017-08-23 17:00:05 PDT Comment hidden (obsolete)
Comment 20 Build Bot 2017-08-23 17:27:33 PDT Comment hidden (obsolete)
Comment 21 Build Bot 2017-08-23 17:27:35 PDT Comment hidden (obsolete)
Comment 22 Brady Eidson 2017-08-24 07:29:58 PDT Comment hidden (obsolete)
Comment 23 Brady Eidson 2017-08-24 13:31:02 PDT
We figured this out. 

Encoders and Decoder didn't match (one in WebCore one in WebKit)

Why this works in Debug builds is a huge mystery!
Comment 24 Brady Eidson 2017-08-24 15:17:23 PDT Comment hidden (obsolete)
Comment 25 Brady Eidson 2017-08-24 17:11:16 PDT Comment hidden (obsolete)
Comment 26 Brady Eidson 2017-08-24 17:14:04 PDT Comment hidden (obsolete)
Comment 27 Brady Eidson 2017-08-24 19:09:19 PDT
Created attachment 319055 [details]
Patch
Comment 28 Andy Estes 2017-08-25 09:27:59 PDT
Comment on attachment 319055 [details]
Patch

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

> Source/WebCore/workers/service/ServiceWorkerJobData.h:41
> +    ServiceWorkerJobData(uint64_t connectionIdentifier);
> +    ServiceWorkerJobData(const ServiceWorkerJobData&);

Should these be explicit?

> Source/WebCore/workers/service/server/SWServer.cpp:108
> +    {
> +        Locker<Lock> locker(m_taskThreadLock);
> +    }

Not sure why this necessary, but you told me in person that this was needed in IndexedDB and might be needed here too. If this is necessary then a comment would be helpful.

> Source/WebCore/workers/service/server/SWServerRegistration.h:41
> +    SWServerRegistration(SWServer&);

Explicit?
Comment 29 Brady Eidson 2017-08-25 10:35:57 PDT
Created attachment 319088 [details]
Patch
Comment 30 Brady Eidson 2017-08-25 10:36:43 PDT
Let EWS chew on it then will cq+
Comment 31 WebKit Commit Bot 2017-08-25 11:57:48 PDT
Comment on attachment 319088 [details]
Patch

Clearing flags on attachment: 319088

Committed r221198: <http://trac.webkit.org/changeset/221198>
Comment 32 WebKit Commit Bot 2017-08-25 11:57:50 PDT
All reviewed patches have been landed.  Closing bug.
Comment 33 Radar WebKit Bug Importer 2017-08-25 11:59:47 PDT
<rdar://problem/34086138>