Bug 207506

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

<rdar://problem/59363308>

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. ***