Bug 180444 – ServiceWorkers API should reject promises when calling objects inside detached frames

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

ServiceWorkers API should reject promises when called on objects inside detached frames. This is covered by:
- imported/w3c/web-platform-tests/service-workers/service-worker/detached-context.https.html

[Chris Dumez]

Created attachment 328503 [details]
Patch

[EWS Watchlist]

Comment on attachment 328503 [details]
Patch

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

New failing tests:
fast/dom/navigator-detached-no-crash.html

[EWS Watchlist]

Created attachment 328511 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

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

[EWS Watchlist]

Comment on attachment 328503 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/service-workers/service-worker/ready.https.html

[EWS Watchlist]

Created attachment 328516 [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.6

[EWS Watchlist]

Comment on attachment 328503 [details]
Patch

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

New failing tests:
fast/dom/navigator-detached-no-crash.html
imported/w3c/web-platform-tests/service-workers/service-worker/ready.https.html
http/tests/media/media-stream/disconnected-frame.html

[EWS Watchlist]

Created attachment 328520 [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

[EWS Watchlist]

Comment on attachment 328503 [details]
Patch

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

New failing tests:
fast/dom/navigator-detached-no-crash.html

[EWS Watchlist]

Created attachment 328522 [details]
Archive of layout-test-results from ews113 for mac-elcapitan

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

[Chris Dumez]

Created attachment 328547 [details]
Patch

[Chris Dumez]

Created attachment 328550 [details]
Patch

[Chris Dumez]

Created attachment 328551 [details]
Patch

[youenn fablet]

Comment on attachment 328551 [details]
Patch

LGTM.
Win bot build error might be related to the patch though.

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

> Source/WebCore/bindings/js/JSDOMPromiseDeferred.h:269
>      JSC::JSPromiseDeferred* promiseDeferred = JSC::JSPromiseDeferred::create(&state, &globalObject);

Let's have callerGlobalObject return a JSDOMGlobalObject& so that we can write the promise creation as a oneliner.

> Source/WebCore/bindings/js/JSDOMWindowBase.h:104
> +WEBCORE_EXPORT JSC::JSGlobalObject& callerGlobalObject(JSC::ExecState&);

Can we move it to JSDOMGlobalObject.h instead?

> Source/WebCore/workers/service/ServiceWorkerContainer.cpp:114
> +    if (!context || !context->sessionID().isValid() || m_isStopped) {

I think m_isStopped is set to true sooner than context is made null.
So probably if (m_isStopped || !context->sessionID().isValid()) is sufficient.
Ditto below.

> LayoutTests/fast/dom/navigator-detached-no-crash-expected.txt:1
> +CONSOLE MESSAGE: Unhandled Promise Rejection: TypeError: Not enough arguments

Since we control this test, we might want to catch the promise rejection in the test itself.
Either when calling the promise function or just by adding an 'unhandledrejection' event listener.
These error messages are sometimes making the tests flaky.
Also that would remove the need to rebase expected.txt files.

[Chris Dumez]

Created attachment 328579 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 328579 [details]
Patch

Clearing flags on attachment: 328579

Committed r225577: <https://trac.webkit.org/changeset/225577>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35895925>

[Radar WebKit Bug Importer]

<rdar://problem/35895926>