Changeset 227348 in webkit

Timestamp:

Jan 22, 2018 12:22:43 PM (6 years ago)

Author:

Chris Dumez

Message:

Safari Tech Preview can't use GitHub login at forums.swift.org
​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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/wpt/service-workers/navigation-redirect-worker.js (added)
• LayoutTests/http/wpt/service-workers/navigation-redirect.https-expected.txt (added)
• LayoutTests/http/wpt/service-workers/navigation-redirect.https.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/DocumentLoader.cpp (modified) (2 diffs)
• Source/WebCore/loader/DocumentLoader.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/loader/DocumentLoader.cpp

@@ -483 +483 @@
 }
 
+#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
 }
 
@@ -1587 +1647 @@
 #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
     });
 }

trunk/Source/WebCore/loader/DocumentLoader.h

@@ -330 +330 @@
     Document* document() const;
 
+#if ENABLE(SERVICE_WORKER)
+    void matchRegistration(const URL&, CompletionHandler<void(std::optional<ServiceWorkerRegistrationData>&&)>&&);
+#endif
+
     void loadMainResource(ResourceRequest&&);