Summary: | Reject Clients.openWindow and Client.navigate promises in some failure cases | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||
Component: | Service Workers | Assignee: | youenn fablet <youennf> | ||||||||
Status: | NEW --- | ||||||||||
Severity: | Normal | CC: | beidson, cdumez, ggaren, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 238503 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
youenn fablet
2022-04-01 05:23:18 PDT
Created attachment 456352 [details]
WIP
Created attachment 456815 [details]
Patch
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 Created attachment 457034 [details]
Patch
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://'.
|