[webkit-reviews] review granted: [Bug 207506] Parent service worker controller should be used for child iframe as per https://w3c.github.io/ServiceWorker/#control-and-use-window-client : [Attachment 390375] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Feb 11 12:47:22 PST 2020
Darin Adler <darin at apple.com> has granted youenn fablet <youennf at gmail.com>'s
request for review:
Bug 207506: Parent service worker controller should be used for child iframe as
per https://w3c.github.io/ServiceWorker/#control-and-use-window-client
https://bugs.webkit.org/show_bug.cgi?id=207506
Attachment 390375: Patch
https://bugs.webkit.org/attachment.cgi?id=390375&action=review
--- Comment #4 from Darin Adler <darin at apple.com> ---
Comment on attachment 390375
--> https://bugs.webkit.org/attachment.cgi?id=390375
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-expecte
d.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.
More information about the webkit-reviews
mailing list