Title: [228211] branches/safari-605-branch

Diff

Modified: branches/safari-605-branch/LayoutTests/ChangeLog (228210 => 228211)


--- branches/safari-605-branch/LayoutTests/ChangeLog	2018-02-07 02:45:09 UTC (rev 228210)
+++ branches/safari-605-branch/LayoutTests/ChangeLog	2018-02-07 02:45:12 UTC (rev 228211)
@@ -1,5 +1,21 @@
 2018-02-06  Jason Marcell  <[email protected]>
 
+        Cherry-pick r228198. rdar://problem/37294605
+
+    2018-02-06  Chris Dumez  <[email protected]>
+
+            Layout Test imported/w3c/web-platform-tests/service-workers/service-worker/fetch-waits-for-activate.https.html is a flaky failure on macOS and iOS
+            https://bugs.webkit.org/show_bug.cgi?id=181392
+            <rdar://problem/36384136>
+
+            Reviewed by Youenn Fablet.
+
+            Unskip test that is no longer flaky.
+
+            * platform/mac-wk2/TestExpectations:
+
+2018-02-06  Jason Marcell  <[email protected]>
+
         Cherry-pick r228195. rdar://problem/37292905
 
     2018-02-06  Andy Estes  <[email protected]>

Modified: branches/safari-605-branch/Source/WebKit/ChangeLog (228210 => 228211)


--- branches/safari-605-branch/Source/WebKit/ChangeLog	2018-02-07 02:45:09 UTC (rev 228210)
+++ branches/safari-605-branch/Source/WebKit/ChangeLog	2018-02-07 02:45:12 UTC (rev 228211)
@@ -1,5 +1,32 @@
 2018-02-06  Jason Marcell  <[email protected]>
 
+        Cherry-pick r228198. rdar://problem/37294605
+
+    2018-02-06  Chris Dumez  <[email protected]>
+
+            Layout Test imported/w3c/web-platform-tests/service-workers/service-worker/fetch-waits-for-activate.https.html is a flaky failure on macOS and iOS
+            https://bugs.webkit.org/show_bug.cgi?id=181392
+            <rdar://problem/36384136>
+
+            Reviewed by Youenn Fablet.
+
+            All tasks from the StorageProcess to the WebContent process to update registrations
+            and service workers state are posted to the runloop. However, the fetch callbacks
+            do not do so. This means that fetch results might come in out of order with regards
+            to the registration / service worker state updates. The test was flaky because an
+            intercepted load would sometimes finish before the task to update the service worker
+            state to "activated" was processed by the runloop. We address the issue by having
+            the ServiceWorkerClientFetch callbacks schedule tasks to the runloop too.
+
+            * WebProcess/Storage/ServiceWorkerClientFetch.cpp:
+            (WebKit::ServiceWorkerClientFetch::didReceiveResponse):
+            (WebKit::ServiceWorkerClientFetch::didReceiveData):
+            (WebKit::ServiceWorkerClientFetch::didFinish):
+            (WebKit::ServiceWorkerClientFetch::didFail):
+            (WebKit::ServiceWorkerClientFetch::didNotHandle):
+
+2018-02-06  Jason Marcell  <[email protected]>
+
         Cherry-pick r228188. rdar://problem/37293107
 
     2018-02-06  Youenn Fablet  <[email protected]>

Modified: branches/safari-605-branch/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp (228210 => 228211)


--- branches/safari-605-branch/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp	2018-02-07 02:45:09 UTC (rev 228210)
+++ branches/safari-605-branch/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp	2018-02-07 02:45:12 UTC (rev 228211)
@@ -100,53 +100,61 @@
 
 void ServiceWorkerClientFetch::didReceiveResponse(ResourceResponse&& response)
 {
-    auto protectedThis = makeRef(*this);
+    callOnMainThread([this, protectedThis = makeRef(*this), response = WTFMove(response)]() mutable {
+        if (!m_loader)
+            return;
 
-    if (auto error = validateResponse(response)) {
-        m_loader->didFail(error.value());
-        if (auto callback = WTFMove(m_callback))
-            callback(Result::Succeeded);
-        return;
-    }
-    response.setSource(ResourceResponse::Source::ServiceWorker);
+        if (auto error = validateResponse(response)) {
+            m_loader->didFail(error.value());
+            if (auto callback = WTFMove(m_callback))
+                callback(Result::Succeeded);
+            return;
+        }
+        response.setSource(ResourceResponse::Source::ServiceWorker);
 
-    if (response.isRedirection() && response.httpHeaderFields().contains(HTTPHeaderName::Location)) {
-        m_redirectionStatus = RedirectionStatus::Receiving;
-        m_loader->willSendRequest(m_loader->request().redirectedRequest(response, m_shouldClearReferrerOnHTTPSToHTTPRedirect), response, [protectedThis = makeRef(*this), this](ResourceRequest&& request) {
-            if (request.isNull() || !m_callback)
-                return;
+        if (response.isRedirection() && response.httpHeaderFields().contains(HTTPHeaderName::Location)) {
+            m_redirectionStatus = RedirectionStatus::Receiving;
+            m_loader->willSendRequest(m_loader->request().redirectedRequest(response, m_shouldClearReferrerOnHTTPSToHTTPRedirect), response, [protectedThis = makeRef(*this), this](ResourceRequest&& request) {
+                if (request.isNull() || !m_callback)
+                    return;
 
-            ASSERT(request == m_loader->request());
-            if (m_redirectionStatus == RedirectionStatus::Received) {
-                start();
-                return;
+                ASSERT(request == m_loader->request());
+                if (m_redirectionStatus == RedirectionStatus::Received) {
+                    start();
+                    return;
+                }
+                m_redirectionStatus = RedirectionStatus::Following;
+            });
+            return;
+        }
+
+        // In case of main resource and mime type is the default one, we set it to text/html to pass more service worker WPT tests.
+        // FIXME: We should refine our MIME type sniffing strategy for synthetic responses.
+        if (m_loader->originalRequest().requester() == ResourceRequest::Requester::Main) {
+            if (response.mimeType() == defaultMIMEType()) {
+                response.setMimeType(ASCIILiteral("text/html"));
+                response.setTextEncodingName(ASCIILiteral("UTF-8"));
             }
-            m_redirectionStatus = RedirectionStatus::Following;
-        });
-        return;
-    }
-
-    // In case of main resource and mime type is the default one, we set it to text/html to pass more service worker WPT tests.
-    // FIXME: We should refine our MIME type sniffing strategy for synthetic responses.
-    if (m_loader->originalRequest().requester() == ResourceRequest::Requester::Main) {
-        if (response.mimeType() == defaultMIMEType()) {
-            response.setMimeType(ASCIILiteral("text/html"));
-            response.setTextEncodingName(ASCIILiteral("UTF-8"));
         }
-    }
 
-    // As per https://fetch.spec.whatwg.org/#main-fetch step 9, copy request's url list in response's url list if empty.
-    if (response.url().isNull())
-        response.setURL(m_loader->request().url());
+        // As per https://fetch.spec.whatwg.org/#main-fetch step 9, copy request's url list in response's url list if empty.
+        if (response.url().isNull())
+            response.setURL(m_loader->request().url());
 
-    m_loader->didReceiveResponse(response);
-    if (auto callback = WTFMove(m_callback))
-        callback(Result::Succeeded);
+        m_loader->didReceiveResponse(response);
+        if (auto callback = WTFMove(m_callback))
+            callback(Result::Succeeded);
+    });
 }
 
 void ServiceWorkerClientFetch::didReceiveData(const IPC::DataReference& data, int64_t encodedDataLength)
 {
-    m_loader->didReceiveData(reinterpret_cast<const char*>(data.data()), data.size(), encodedDataLength, DataPayloadBytes);
+    callOnMainThread([this, protectedThis = makeRef(*this), data = "" encodedDataLength] {
+        if (!m_loader)
+            return;
+
+        m_loader->didReceiveData(reinterpret_cast<const char*>(data.data()), data.size(), encodedDataLength, DataPayloadBytes);
+    });
 }
 
 void ServiceWorkerClientFetch::didReceiveFormData(const IPC::FormDataReference&)
@@ -156,44 +164,57 @@
 
 void ServiceWorkerClientFetch::didFinish()
 {
-    switch (m_redirectionStatus) {
-    case RedirectionStatus::None:
-        break;
-    case RedirectionStatus::Receiving:
-        m_redirectionStatus = RedirectionStatus::Received;
-        return;
-    case RedirectionStatus::Following:
-        start();
-        return;
-    case RedirectionStatus::Received:
-        ASSERT_NOT_REACHED();
-        m_redirectionStatus = RedirectionStatus::None;
-    }
+    callOnMainThread([this, protectedThis = makeRef(*this)] {
+        if (!m_loader)
+            return;
 
-    ASSERT(!m_callback);
+        switch (m_redirectionStatus) {
+        case RedirectionStatus::None:
+            break;
+        case RedirectionStatus::Receiving:
+            m_redirectionStatus = RedirectionStatus::Received;
+            return;
+        case RedirectionStatus::Following:
+            start();
+            return;
+        case RedirectionStatus::Received:
+            ASSERT_NOT_REACHED();
+            m_redirectionStatus = RedirectionStatus::None;
+        }
 
-    auto protectedThis = makeRef(*this);
-    m_loader->didFinishLoading(NetworkLoadMetrics { });
-    m_serviceWorkerProvider.fetchFinished(m_identifier);
+        ASSERT(!m_callback);
+
+        m_loader->didFinishLoading(NetworkLoadMetrics { });
+        m_serviceWorkerProvider.fetchFinished(m_identifier);
+    });
 }
 
 void ServiceWorkerClientFetch::didFail()
 {
-    auto protectedThis = makeRef(*this);
-    m_loader->didFail({ ResourceError::Type::General });
+    callOnMainThread([this, protectedThis = makeRef(*this)] {
+        if (!m_loader)
+            return;
 
-    if (auto callback = WTFMove(m_callback))
-        callback(Result::Succeeded);
+        m_loader->didFail({ ResourceError::Type::General });
 
-    m_serviceWorkerProvider.fetchFinished(m_identifier);
+        if (auto callback = WTFMove(m_callback))
+            callback(Result::Succeeded);
+
+        m_serviceWorkerProvider.fetchFinished(m_identifier);
+    });
 }
 
 void ServiceWorkerClientFetch::didNotHandle()
 {
-    if (auto callback = WTFMove(m_callback))
-        callback(Result::Unhandled);
+    callOnMainThread([this, protectedThis = makeRef(*this)] {
+        if (!m_loader)
+            return;
 
-    m_serviceWorkerProvider.fetchFinished(m_identifier);
+        if (auto callback = WTFMove(m_callback))
+            callback(Result::Unhandled);
+
+        m_serviceWorkerProvider.fetchFinished(m_identifier);
+    });
 }
 
 void ServiceWorkerClientFetch::cancel()
@@ -200,6 +221,7 @@
 {
     if (auto callback = WTFMove(m_callback))
         callback(Result::Cancelled);
+    m_loader = nullptr;
 }
 
 } // namespace WebKit

Modified: branches/safari-605-branch/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.h (228210 => 228211)


--- branches/safari-605-branch/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.h	2018-02-07 02:45:09 UTC (rev 228210)
+++ branches/safari-605-branch/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.h	2018-02-07 02:45:12 UTC (rev 228211)
@@ -67,7 +67,7 @@
     void didNotHandle();
 
     WebServiceWorkerProvider& m_serviceWorkerProvider;
-    Ref<WebCore::ResourceLoader> m_loader;
+    RefPtr<WebCore::ResourceLoader> m_loader;
     uint64_t m_identifier { 0 };
     Ref<WebSWClientConnection> m_connection;
     Callback m_callback;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to