Bug 238664 – Reject Clients.openWindow and Client.navigate promises in some failure cases

• Status: NEW
• Component: Service Workers

[youenn fablet]

Investigate rejecting Clients.openWindow promise in some cases

[youenn fablet]

Created attachment 456352 [details]
WIP

[Radar WebKit Bug Importer]

<rdar://problem/91350577>

[youenn fablet]

Created attachment 456815 [details]
Patch

[youenn fablet]

Comments from https://bugs.webkit.org/show_bug.cgi?id=238503#c14:

I had a look at Chrome code.
Chrome is rejecting the promise in some cases that the spec does not define, like openWindow('file://').
I filed https://bugs.webkit.org/show_bug.cgi?id=238658 to keep track of that.

Chrome has the infrastructure to reject the promise based on the openWindow result gathered from the main process: https://source.chromium.org/chromium/chromium/src/+/main:content/browser/service_worker/service_worker_client_utils.cc;l=429;drc=7fb345a0da63049b102e1c0bcdc8d7831110e324.
A few things worth noting (from code inspection so this needs actual validation):
- It seems the default implementation of the callback (no tab is created) will trigger rejection of the promise. This is inline with this patch.
- If openWindow triggers creation of a tab that quickly got closed, promise is resolved with null. I am not sure we are aligned here, since probably we would reject if WebFrameProxy got deallocated.
- navigate and openWindow use the same code path so I would expect similar resolve/reject behaviour.
- navigate and openWindow do reject for some bad URLs, which is not described in the spec. This seems to indicate that errors in HTML navigate could sometimes lead to rejection

[youenn fablet]

Created attachment 457034 [details]
Patch

[youenn fablet]

Comment on attachment 457034 [details]
Patch

Marking as r?
This patch makes it more consistent in navigate algorithm when UIProcess stops the load differently (in case of 'view-source//' and in case of 'file:///'). We align with Chrome behavior here.
It also rejects in case the service worker openWindow delegate is not implemented, which I think mirrors what Chrome is doing.
This area still needs some interop and spec work since Firefox for instance is not rejecting on 'file:///'but does on 'view-source://'.