Bug 175851 – Introduce ServerWorkerRegistration task queues

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Brady Eidson]

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.

[Brady Eidson]

Created attachment 318799 [details]
Patch

[Build Bot]

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

[Build Bot]

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

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

[Brady Eidson]

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

[Brady Eidson]

Created attachment 318816 [details]
Patch

[Brady Eidson]

Comment on attachment 318816 [details]
Patch

Attempting to get more logging from the failure on EWS

[Build Bot]

Comment on attachment 318816 [details]
Patch

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

New failing tests:
http/tests/workers/service/basic-register.html

[Build Bot]

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

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

[Build Bot]

Comment on attachment 318816 [details]
Patch

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

New failing tests:
http/tests/workers/service/basic-register.html

[Build Bot]

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

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

[Brady Eidson]

This is super frustrating, as I can't reproduce the failure on my El Cap, Sierra, or High Sierra machines with a local dev environment. Will have to try to work on the bot directly.

[Brady Eidson]

Created attachment 318918 [details]
With some logging even EWS should give me.

[Build Bot]

Comment on attachment 318918 [details]
With some logging even EWS should give me.

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

New failing tests:
http/tests/workers/service/basic-register.html

[Build Bot]

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

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

[Brady Eidson]

(In reply to Brady Eidson from comment #12)
> Created attachment 318918 [details]
> With some logging even EWS should give me.

And that definitely did not work. It's not even revealing the printf() from the call to serviceWorker.register which is...  suspicious.

[Brady Eidson]

Created attachment 318932 [details]
Trying again, flushing stdout this time

[Brady Eidson]

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

[Build Bot]

Comment on attachment 318932 [details]
Trying again, flushing stdout this time

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

New failing tests:
http/tests/workers/service/basic-register.html

[Build Bot]

Created attachment 318941 [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 318932 [details]
Trying again, flushing stdout this time

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

New failing tests:
http/tests/workers/service/basic-register.html

[Build Bot]

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

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

[Brady Eidson]

This is too weird.

The WK2 bots do release builds. So locally, with a release build, I can reproduce.

But stepping aside from the run-webkit-tests script, I tried invoking WebKitTestRunner directly 4 different ways:

Works - DYLD_FRAMEWORK_PATH=~/build/Debug/ ~/build/Debug/WebKitTestRunner https://localhost:8443/workers/service/basic-register.html

Works - DYLD_FRAMEWORK_PATH=~/build/Debug/ ~/build/Debug/WebKitTestRunner http://localhost:8000/workers/service/basic-register.html

Works - DYLD_FRAMEWORK_PATH=~/build/Release/ ~/build/Release/WebKitTestRunner https://localhost:8443/workers/service/basic-register.html

FAILS - DYLD_FRAMEWORK_PATH=~/build/Release/ ~/build/Release/WebKitTestRunner http://localhost:8000/workers/service/basic-register.html

So while Debug builds work with both http and https input URLs, Release builds work but only with https.

Even more bizarre, with printf debugging, I've verified the serviceWorker property is available in all 4 of these cases, and there is an attempt to schedule the registration job. But for some reason... it never gets to the Storage process.

Now, navigator.serviceWorker *is* supposed to be exposed only on HTTPS contexts but apparently our IDL parser doesn't correctly handle "SecureContext" quite yet.

So I could fix that, make it HTTPs only, and call it a day. But... something weird is going on.

[Brady Eidson]

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!

[Brady Eidson]

Same patch after https://bugs.webkit.org/show_bug.cgi?id=175952 lands should be fine.

[Brady Eidson]

Created attachment 319041 [details]
For EWS (With test!)

[Brady Eidson]

Created attachment 319043 [details]
For EWS (With test!)

[Brady Eidson]

Created attachment 319055 [details]
Patch

[Andy Estes]

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?

[Brady Eidson]

Created attachment 319088 [details]
Patch

[Brady Eidson]

Let EWS chew on it then will cq+

[WebKit Commit Bot]

Comment on attachment 319088 [details]
Patch

Clearing flags on attachment: 319088

Committed r221198: <http://trac.webkit.org/changeset/221198>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34086138>