Safari Tech Preview can't use GitHub login at forums.swift.org
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Jan 2018 20:22:43 +0000 (20:22 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Jan 2018 20:22:43 +0000 (20:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181908
<rdar://problem/36715111>

Patch by Youenn Fablet <youenn@apple.com> on 2018-01-22
Reviewed by Chris Dumez.

Source/WebCore:

Test: http/wpt/service-workers/navigation-redirect.https.html

For subresource loads, redirections will not change who is in charge of continuing the load (service worker or network process).
For navigation loads, we need to match the registration for every redirection since this is using the Manual redirect mode.
This allows starting the load with a service worker and finishing the load with another service worker, which will become the controller.

Implement this by wrapping the registration matching of an URL within DocumentLoader::matchRegistration.
Use that method in DocumentLoader::redirectReceived.

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::matchRegistration):
(WebCore::doRegistrationsMatch):
(WebCore::DocumentLoader::redirectReceived):
(WebCore::DocumentLoader::startLoadingMainResource):
* loader/DocumentLoader.h:

LayoutTests:

* http/wpt/service-workers/navigation-redirect-worker.js: Added.
(async):
* http/wpt/service-workers/navigation-redirect.https-expected.txt: Added.
* http/wpt/service-workers/navigation-redirect.https.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@227348 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/wpt/service-workers/navigation-redirect-worker.js [new file with mode: 0644]
LayoutTests/http/wpt/service-workers/navigation-redirect.https-expected.txt [new file with mode: 0644]
LayoutTests/http/wpt/service-workers/navigation-redirect.https.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/loader/DocumentLoader.h

index 8decc33..48a705e 100644 (file)
@@ -1,3 +1,16 @@
+2018-01-22  Youenn Fablet  <youenn@apple.com>
+
+        Safari Tech Preview can't use GitHub login at forums.swift.org
+        https://bugs.webkit.org/show_bug.cgi?id=181908
+        <rdar://problem/36715111>
+
+        Reviewed by Chris Dumez.
+
+        * http/wpt/service-workers/navigation-redirect-worker.js: Added.
+        (async):
+        * http/wpt/service-workers/navigation-redirect.https-expected.txt: Added.
+        * http/wpt/service-workers/navigation-redirect.https.html: Added.
+
 2018-01-22  Antti Koivisto  <antti@apple.com>
 
         REGRESSION (Safari 11): Buttons inside a fieldset legend cannot be clicked on in Safari 11
diff --git a/LayoutTests/http/wpt/service-workers/navigation-redirect-worker.js b/LayoutTests/http/wpt/service-workers/navigation-redirect-worker.js
new file mode 100644 (file)
index 0000000..3ee2362
--- /dev/null
@@ -0,0 +1,11 @@
+addEventListener("fetch", async (e) => {
+    if (e.request.url.indexOf("navigation-redirect-frame1.html") !== -1) {
+        //e.respondWith(new Response(self.registration.scope));
+        e.respondWith(Response.redirect("resources/navigation-redirect-frame2.html"));
+        return;
+    }
+    if (e.request.url.indexOf("navigation-redirect-frame2.html") !== -1) {
+        e.respondWith(new Response(self.registration.scope));
+        return;
+    }
+});
diff --git a/LayoutTests/http/wpt/service-workers/navigation-redirect.https-expected.txt b/LayoutTests/http/wpt/service-workers/navigation-redirect.https-expected.txt
new file mode 100644 (file)
index 0000000..d706de2
--- /dev/null
@@ -0,0 +1,7 @@
+
+PASS Setup worker 1 
+PASS Frame redirecting to an unregistered scope 
+PASS Setup worker 2 
+PASS Frame redirecting to a registered scope 
+PASS Clean-up 
+
diff --git a/LayoutTests/http/wpt/service-workers/navigation-redirect.https.html b/LayoutTests/http/wpt/service-workers/navigation-redirect.https.html
new file mode 100644 (file)
index 0000000..cfd4c14
--- /dev/null
@@ -0,0 +1,66 @@
+<html>
+<head>
+<title>Service Worker Fetch Event</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+</head>
+<body>
+<script>
+var scope1 = "resources/navigation-redirect-frame1.html";
+var scope2 = "resources/navigation-redirect-frame2.html";
+var registration1;
+var registration2;
+
+function withFrame(url)
+{
+    return new Promise((resolve) => {
+        let frame = document.createElement('iframe');
+        frame.src = url;
+        frame.onload = function() { resolve(frame); };
+        document.body.appendChild(frame);
+    });
+}
+
+async function registerServiceWorker(scope)
+{
+    var registration = await navigator.serviceWorker.register("navigation-redirect-worker.js", { scope : scope });
+    var activeWorker = registration.active;
+    if (activeWorker)
+        return;
+    activeWorker = registration.installing;
+    return new Promise(resolve => {
+        activeWorker.addEventListener('statechange', () => {
+            if (activeWorker.state === "activated")
+                resolve(registration);
+        });
+    });
+}
+
+promise_test(async (test) => {
+    registration1 = await registerServiceWorker(scope1);
+}, "Setup worker 1");
+
+promise_test(async (test) => {
+    var frame = await withFrame(scope1);
+    assert_equals(frame.contentWindow.navigator.serviceWorker.controller, null);
+    frame.remove();
+}, "Frame redirecting to an unregistered scope");
+
+promise_test(async (test) => {
+    registration2 = await registerServiceWorker(scope2);
+}, "Setup worker 2");
+
+promise_test(async (test) => {
+    var frame = await withFrame(scope1);
+    assert_true(frame.contentWindow.navigator.serviceWorker.controller !== null);
+    frame.remove();
+}, "Frame redirecting to a registered scope");
+
+promise_test(async (test) => {
+    registration1.unregister();
+    registration2.unregister();
+}, "Clean-up");
+
+</script>
+</body>
+</html>
index 823e38f..1e39b3f 100644 (file)
@@ -1,3 +1,27 @@
+2018-01-22  Youenn Fablet  <youenn@apple.com>
+
+        Safari Tech Preview can't use GitHub login at forums.swift.org
+        https://bugs.webkit.org/show_bug.cgi?id=181908
+        <rdar://problem/36715111>
+
+        Reviewed by Chris Dumez.
+
+        Test: http/wpt/service-workers/navigation-redirect.https.html
+
+        For subresource loads, redirections will not change who is in charge of continuing the load (service worker or network process).
+        For navigation loads, we need to match the registration for every redirection since this is using the Manual redirect mode.
+        This allows starting the load with a service worker and finishing the load with another service worker, which will become the controller.
+
+        Implement this by wrapping the registration matching of an URL within DocumentLoader::matchRegistration.
+        Use that method in DocumentLoader::redirectReceived.
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::matchRegistration):
+        (WebCore::doRegistrationsMatch):
+        (WebCore::DocumentLoader::redirectReceived):
+        (WebCore::DocumentLoader::startLoadingMainResource):
+        * loader/DocumentLoader.h:
+
 2018-01-22  Antti Koivisto  <antti@apple.com>
 
         REGRESSION (Safari 11): Buttons inside a fieldset legend cannot be clicked on in Safari 11
index f0516ad..189cda9 100644 (file)
@@ -482,10 +482,70 @@ void DocumentLoader::handleSubstituteDataLoadSoon()
         startDataLoadTimer();
 }
 
+#if ENABLE(SERVICE_WORKER)
+void DocumentLoader::matchRegistration(const URL& url, SWClientConnection::RegistrationCallback&& callback)
+{
+    auto shouldTryLoadingThroughServiceWorker = !frameLoader()->isReloadingFromOrigin() && m_frame->page() && RuntimeEnabledFeatures::sharedFeatures().serviceWorkerEnabled() && SchemeRegistry::canServiceWorkersHandleURLScheme(url.protocol().toStringWithoutCopying());
+    if (!shouldTryLoadingThroughServiceWorker) {
+        callback(std::nullopt);
+        return;
+    }
+
+    auto origin = (!m_frame->isMainFrame() && m_frame->document()) ? makeRef(m_frame->document()->topOrigin()) : SecurityOrigin::create(url);
+    auto sessionID = m_frame->page()->sessionID();
+    auto& provider = ServiceWorkerProvider::singleton();
+    if (!provider.mayHaveServiceWorkerRegisteredForOrigin(sessionID, origin)) {
+        callback(std::nullopt);
+        return;
+    }
+
+    auto& connection = ServiceWorkerProvider::singleton().serviceWorkerConnectionForSession(sessionID);
+    connection.matchRegistration(origin, url, WTFMove(callback));
+}
+
+static inline bool areRegistrationsEqual(const std::optional<ServiceWorkerRegistrationData>& a, const std::optional<ServiceWorkerRegistrationData>& b)
+{
+    if (!a)
+        return !b;
+    if (!b)
+        return false;
+    return a->identifier == b->identifier;
+}
+#endif
+
 void DocumentLoader::redirectReceived(CachedResource& resource, ResourceRequest&& request, const ResourceResponse& redirectResponse, CompletionHandler<void(ResourceRequest&&)>&& completionHandler)
 {
     ASSERT_UNUSED(resource, &resource == m_mainResource);
+#if ENABLE(SERVICE_WORKER)
+    willSendRequest(WTFMove(request), redirectResponse, [completionHandler = WTFMove(completionHandler), protectedThis = makeRef(*this), this] (auto&& request) mutable {
+        if (request.isNull() || !m_mainDocumentError.isNull() || !m_frame) {
+            completionHandler({ });
+            return;
+        }
+        auto url = request.url();
+        matchRegistration(url, [request = WTFMove(request), completionHandler = WTFMove(completionHandler), protectedThis = WTFMove(protectedThis), this] (auto&& registrationData) mutable {
+            if (!m_mainDocumentError.isNull() || !m_frame) {
+                completionHandler({ });
+                return;
+            }
+
+            if (areRegistrationsEqual(m_serviceWorkerRegistrationData, registrationData)) {
+                completionHandler(WTFMove(request));
+                return;
+            }
+
+            // Service worker registration changed, we need to cancel the current load to restart a new one.
+            clearMainResource();
+            completionHandler({ });
+
+            m_serviceWorkerRegistrationData = WTFMove(registrationData);
+            loadMainResource(WTFMove(request));
+            return;
+        });
+    });
+#else
     willSendRequest(WTFMove(request), redirectResponse, WTFMove(completionHandler));
+#endif
 }
 
 void DocumentLoader::willSendRequest(ResourceRequest&& newRequest, const ResourceResponse& redirectResponse, CompletionHandler<void(ResourceRequest&&)>&& completionHandler)
@@ -1586,26 +1646,17 @@ void DocumentLoader::startLoadingMainResource()
 
 #if ENABLE(SERVICE_WORKER)
         // FIXME: Implement local URL interception by getting the service worker of the parent.
-        auto tryLoadingThroughServiceWorker = !frameLoader()->isReloadingFromOrigin() && m_frame->page() && RuntimeEnabledFeatures::sharedFeatures().serviceWorkerEnabled() && SchemeRegistry::canServiceWorkersHandleURLScheme(request.url().protocol().toStringWithoutCopying());
-        if (tryLoadingThroughServiceWorker) {
-            auto origin = (!m_frame->isMainFrame() && m_frame->document()) ? makeRef(m_frame->document()->topOrigin()) : SecurityOrigin::create(request.url());
-            auto sessionID = m_frame->page()->sessionID();
-            auto& provider = ServiceWorkerProvider::singleton();
-            if (provider.mayHaveServiceWorkerRegisteredForOrigin(sessionID, origin)) {
-                auto& connection = ServiceWorkerProvider::singleton().serviceWorkerConnectionForSession(sessionID);
-                auto url = request.url();
-                connection.matchRegistration(origin, url, [request = WTFMove(request), protectedThis = WTFMove(protectedThis), this] (auto&& registrationData) mutable {
-                    if (!m_mainDocumentError.isNull() || !m_frame)
-                        return;
-
-                    m_serviceWorkerRegistrationData = WTFMove(registrationData);
-                    loadMainResource(WTFMove(request));
-                });
+        auto url = request.url();
+        matchRegistration(url, [request = WTFMove(request), protectedThis = WTFMove(protectedThis), this] (auto&& registrationData) mutable {
+            if (!m_mainDocumentError.isNull() || !m_frame)
                 return;
-            }
-        }
-#endif
+
+            m_serviceWorkerRegistrationData = WTFMove(registrationData);
+            loadMainResource(WTFMove(request));
+        });
+#else
         loadMainResource(WTFMove(request));
+#endif
     });
 }
 
index 6bdef8d..fb4db77 100644 (file)
@@ -329,6 +329,10 @@ protected:
 private:
     Document* document() const;
 
+#if ENABLE(SERVICE_WORKER)
+    void matchRegistration(const URL&, CompletionHandler<void(std::optional<ServiceWorkerRegistrationData>&&)>&&);
+#endif
+
     void loadMainResource(ResourceRequest&&);
 
     void setRequest(const ResourceRequest&);