Title: [231271] trunk/Source/WebKit
Revision
231271
Author
[email protected]
Date
2018-05-02 15:22:21 -0700 (Wed, 02 May 2018)

Log Message

Cleanup NetworkResourceLoader::didReceiveResponse()
https://bugs.webkit.org/show_bug.cgi?id=185209

Reviewed by Chris Dumez.

Use early returns to make the control flow easier to read and reason about. Disregarding a
From-Origin violation, NetworkResourceLoader::didReceiveResponse() only returns NetworkLoadClient::ShouldContinueDidReceiveResponse::No
when the load is for a main resource and hence it must wait for the embedding client to allow
the load before continuing with it. With regards to a From-Origin violation, the network
process schedules to fail the load in a subsequent turn of the event loop before returning
NetworkLoadClient::ShouldContinueDidReceiveResponse::No. It return NetworkLoadClient::ShouldContinueDidReceiveResponse::No
solely to tell the NetworkLoadClient to defer assuming the load is allowed (because we will
fail it on the next turn of the event loop).

Additionally, remove all logging about the return value as we no longer have a need for
such logging.

* NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::didReceiveResponse):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (231270 => 231271)


--- trunk/Source/WebKit/ChangeLog	2018-05-02 22:18:11 UTC (rev 231270)
+++ trunk/Source/WebKit/ChangeLog	2018-05-02 22:22:21 UTC (rev 231271)
@@ -1,3 +1,25 @@
+2018-05-02  Daniel Bates  <[email protected]>
+
+        Cleanup NetworkResourceLoader::didReceiveResponse()
+        https://bugs.webkit.org/show_bug.cgi?id=185209
+
+        Reviewed by Chris Dumez.
+
+        Use early returns to make the control flow easier to read and reason about. Disregarding a
+        From-Origin violation, NetworkResourceLoader::didReceiveResponse() only returns NetworkLoadClient::ShouldContinueDidReceiveResponse::No
+        when the load is for a main resource and hence it must wait for the embedding client to allow
+        the load before continuing with it. With regards to a From-Origin violation, the network
+        process schedules to fail the load in a subsequent turn of the event loop before returning
+        NetworkLoadClient::ShouldContinueDidReceiveResponse::No. It return NetworkLoadClient::ShouldContinueDidReceiveResponse::No
+        solely to tell the NetworkLoadClient to defer assuming the load is allowed (because we will
+        fail it on the next turn of the event loop).
+
+        Additionally, remove all logging about the return value as we no longer have a need for
+        such logging.
+
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::didReceiveResponse):
+
 2018-05-02  Alex Christensen  <[email protected]>
 
         Add WKWebsiteDataStorePrivate._proxyConfiguration SPI

Modified: trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp (231270 => 231271)


--- trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp	2018-05-02 22:18:11 UTC (rev 231270)
+++ trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp	2018-05-02 22:22:21 UTC (rev 231271)
@@ -413,43 +413,36 @@
         } else
             m_cacheEntryForValidation = nullptr;
     }
-    bool shouldSendDidReceiveResponse = !m_cacheEntryForValidation;
+    if (m_cacheEntryForValidation)
+        return ShouldContinueDidReceiveResponse::Yes;
 
-    bool shouldWaitContinueDidReceiveResponse = isMainResource();
-    if (shouldSendDidReceiveResponse) {
-
-        ResourceError error;
-        if (m_parameters.shouldEnableFromOriginResponseHeader && shouldCancelCrossOriginLoad(m_response, m_parameters.frameAncestorOrigins))
-            error = fromOriginResourceError(m_response.url());
-
-        if (error.isNull() && m_networkLoadChecker)
-            error = m_networkLoadChecker->validateResponse(m_response);
-
-        if (!error.isNull()) {
-            RunLoop::main().dispatch([protectedThis = makeRef(*this), error = WTFMove(error)] {
-                if (protectedThis->m_networkLoad)
-                    protectedThis->didFailLoading(error);
-            });
-            return ShouldContinueDidReceiveResponse::No;
-        }
-
-        auto response = sanitizeResponseIfPossible(ResourceResponse { m_response }, ResourceResponse::SanitizationType::CrossOriginSafe);
-        if (isSynchronous())
-            m_synchronousLoadData->response = WTFMove(response);
-        else
-            send(Messages::WebResourceLoader::DidReceiveResponse { response, shouldWaitContinueDidReceiveResponse });
+    ResourceError error;
+    if (m_parameters.shouldEnableFromOriginResponseHeader && shouldCancelCrossOriginLoad(m_response, m_parameters.frameAncestorOrigins))
+        error = fromOriginResourceError(m_response.url());
+    if (error.isNull() && m_networkLoadChecker)
+        error = m_networkLoadChecker->validateResponse(m_response);
+    if (!error.isNull()) {
+        // FIXME: We need to make a main resource load look successful to prevent leaking its existence. See <https://bugs.webkit.org/show_bug.cgi?id=185120>.
+        RunLoop::main().dispatch([protectedThis = makeRef(*this), error = WTFMove(error)] {
+            if (protectedThis->m_networkLoad)
+                protectedThis->didFailLoading(error);
+        });
+        // FIXME: We know that we are not going to continue this load. ShouldContinueDidReceiveResponse::No should only be returned when
+        // the network process is waiting to receive message NetworkResourceLoader::ContinueDidReceiveResponse to continue a load.
+        return ShouldContinueDidReceiveResponse::No;
     }
 
-    // For main resources, the web process is responsible for sending back a NetworkResourceLoader::ContinueDidReceiveResponse message.
-    bool shouldContinueDidReceiveResponse = !shouldWaitContinueDidReceiveResponse || m_cacheEntryForValidation;
-
-    if (shouldContinueDidReceiveResponse) {
-        RELEASE_LOG_IF_ALLOWED("didReceiveResponse: Should not wait for message from WebContent process before continuing resource load (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ")", m_parameters.webPageID, m_parameters.webFrameID, m_parameters.identifier);
+    auto response = sanitizeResponseIfPossible(ResourceResponse { m_response }, ResourceResponse::SanitizationType::CrossOriginSafe);
+    if (isSynchronous()) {
+        m_synchronousLoadData->response = WTFMove(response);
         return ShouldContinueDidReceiveResponse::Yes;
     }
 
-    RELEASE_LOG_IF_ALLOWED("didReceiveResponse: Should wait for message from WebContent process before continuing resource load (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ")", m_parameters.webPageID, m_parameters.webFrameID, m_parameters.identifier);
-    return ShouldContinueDidReceiveResponse::No;
+    // We wait to receive message NetworkResourceLoader::ContinueDidReceiveResponse before continuing a load for
+    // a main resource because the embedding client must decide whether to allow the load.
+    bool willWaitForContinueDidReceiveResponse = isMainResource();
+    send(Messages::WebResourceLoader::DidReceiveResponse { response, willWaitForContinueDidReceiveResponse });
+    return willWaitForContinueDidReceiveResponse ? ShouldContinueDidReceiveResponse::No : ShouldContinueDidReceiveResponse::Yes;
 }
 
 void NetworkResourceLoader::didReceiveBuffer(Ref<SharedBuffer>&& buffer, int reportedEncodedDataLength)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to