Summary: | Parent service worker controller should be used for child iframe as per https://w3c.github.io/ServiceWorker/#control-and-use-window-client | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||||
Component: | Service Workers | Assignee: | youenn fablet <youennf> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | cdumez, commit-queue, danhite, darin, dbates, ews-watchlist, japhet, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=207579 | ||||||||||||
Attachments: |
|
Description
youenn fablet
2020-02-10 13:59:15 PST
Created attachment 390292 [details]
Patch
Created attachment 390360 [details]
Patch
Created attachment 390375 [details]
Patch
Comment on attachment 390375 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=390375&action=review > Source/WebCore/loader/DocumentLoader.cpp:1060 > +// https://w3c.github.io/ServiceWorker/#control-and-use-window-client The terminology used in the specification is so different from the terminology we use here. It made hard for me to understand. For example, the specification doesn’t talk about a "controller" at all. > Source/WebCore/loader/DocumentLoader.cpp:1061 > +static inline bool isInheritingControllerFromParent(const Document& document, const Document& parent) Name should probably be: shouldApplyActiveServiceWorkerFromParent > Source/WebCore/loader/DocumentLoader.cpp:1067 > + if (document.url().protocolIsInHTTPFamily()) > + return false; > + if (document.securityOrigin().isUnique()) > + return false; > + return parent.securityOrigin().canAccess(document.securityOrigin()); I think this might be more readable as !a && !b && c rather than with cascading if statements. > Source/WebCore/loader/DocumentLoader.cpp:1098 > + } else if (auto* parent = m_frame->document()->parentDocument()) { m_frame->document()-> is repeated over 10 times in this function! There’s got to be some refactoring we can do to cut down on that distracting repetition safely (some day). > LayoutTests/http/tests/workers/service/serviceworkerclients-claim.https-expected.txt:3 > +CONSOLE MESSAGE: Origin null is not allowed by Access-Control-Allow-Origin. > +CONSOLE MESSAGE: Fetch API cannot load https://127.0.0.1:8443/pinkelephant due to access control checks. > +CONSOLE MESSAGE: line 1: Unhandled Promise Rejection: TypeError: Origin null is not allowed by Access-Control-Allow-Origin. I don’t understand these changes to the results. Nothing in the change log makes it clear either. (In reply to Darin Adler from comment #4) > Comment on attachment 390375 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=390375&action=review > > > Source/WebCore/loader/DocumentLoader.cpp:1060 > > +// https://w3c.github.io/ServiceWorker/#control-and-use-window-client > > The terminology used in the specification is so different from the > terminology we use here. It made hard for me to understand. For example, the > specification doesn’t talk about a "controller" at all. > > > Source/WebCore/loader/DocumentLoader.cpp:1061 > > +static inline bool isInheritingControllerFromParent(const Document& document, const Document& parent) > > Name should probably be: > > shouldApplyActiveServiceWorkerFromParent OK, will take shouldUseActiveServiceWorkerFromParent > > Source/WebCore/loader/DocumentLoader.cpp:1067 > > + if (document.url().protocolIsInHTTPFamily()) > > + return false; > > + if (document.securityOrigin().isUnique()) > > + return false; > > + return parent.securityOrigin().canAccess(document.securityOrigin()); > > I think this might be more readable as !a && !b && c rather than with > cascading if statements. OK > > Source/WebCore/loader/DocumentLoader.cpp:1098 > > + } else if (auto* parent = m_frame->document()->parentDocument()) { > > m_frame->document()-> is repeated over 10 times in this function! There’s > got to be some refactoring we can do to cut down on that distracting > repetition safely (some day). Will do in a follow up. > > LayoutTests/http/tests/workers/service/serviceworkerclients-claim.https-expected.txt:3 > > +CONSOLE MESSAGE: Origin null is not allowed by Access-Control-Allow-Origin. > > +CONSOLE MESSAGE: Fetch API cannot load https://127.0.0.1:8443/pinkelephant due to access control checks. > > +CONSOLE MESSAGE: line 1: Unhandled Promise Rejection: TypeError: Origin null is not allowed by Access-Control-Allow-Origin. > > I don’t understand these changes to the results. Nothing in the change log > makes it clear either. The iframe is doing a fetch. Since it is no longer controlled, it goes to the network where it fails due to CORS. Will beef up the ChangeLog. Created attachment 390412 [details]
Patch for landing
Comment on attachment 390412 [details] Patch for landing Clearing flags on attachment: 390412 Committed r256381: <https://trac.webkit.org/changeset/256381> All reviewed patches have been landed. Closing bug. *** Bug 207378 has been marked as a duplicate of this bug. *** |