Bug 238664 - Reject Clients.openWindow and Client.navigate promises in some failure cases
Summary: Reject Clients.openWindow and Client.navigate promises in some failure cases
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Service Workers (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on: 238503
Blocks:
  Show dependency treegraph
 
Reported: 2022-04-01 05:23 PDT by youenn fablet
Modified: 2022-04-11 01:00 PDT (History)
4 users (show)

See Also:


Attachments
WIP (4.87 KB, patch)
2022-04-01 05:30 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (22.68 KB, patch)
2022-04-06 06:43 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (9.17 KB, patch)
2022-04-08 02:26 PDT, youenn fablet
youennf: review?
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2022-04-01 05:23:18 PDT
Investigate rejecting Clients.openWindow promise in some cases
Comment 1 youenn fablet 2022-04-01 05:30:43 PDT
Created attachment 456352 [details]
WIP
Comment 2 Radar WebKit Bug Importer 2022-04-06 06:42:28 PDT
<rdar://problem/91350577>
Comment 3 youenn fablet 2022-04-06 06:43:00 PDT
Created attachment 456815 [details]
Patch
Comment 4 youenn fablet 2022-04-06 09:09:37 PDT
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
Comment 5 youenn fablet 2022-04-08 02:26:38 PDT
Created attachment 457034 [details]
Patch
Comment 6 youenn fablet 2022-04-11 01:00:15 PDT
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://'.