Changeset 279864 in webkit

Timestamp:

Jul 12, 2021, 9:39:26 PM (4 years ago)

Author:

Fujii Hironori

Message:

[curl] Can't finish loading some websites after r271946 started throttling low priority resources
​https://bugs.webkit.org/show_bug.cgi?id=227863

Reviewed by Don Olmstead.

NetworkLoadScheduler started throttling low priority resource
loading since r271946. It uses curl_easy_pause to pause the
resource loads. CURLMOPT_MAX_HOST_CONNECTIONS is set to 6 to limit
the number of connections per host. In the certain condition,
curl_multi_fdset can constantly return -1 if 6 handles of the same
host are paused.

This patch changes CurlRequest::start not to actually start the
job if it is suspended.

• platform/network/curl/CurlRequest.cpp:

(WebCore::CurlRequest::start):
(WebCore::CurlRequest::suspend):
(WebCore::CurlRequest::resume):
(WebCore::CurlRequest::setupTransfer):
(WebCore::CurlRequest::setRequestPaused):

• platform/network/curl/CurlRequest.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/network/curl/CurlRequest.cpp (modified) (7 diffs)
• platform/network/curl/CurlRequest.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-07-12  Fujii Hironori  <Hironori.Fujii@sony.com>
+
+        [curl] Can't finish loading some websites after r271946 started throttling low priority resources
+        https://bugs.webkit.org/show_bug.cgi?id=227863
+
+        Reviewed by Don Olmstead.
+
+        NetworkLoadScheduler started throttling low priority resource
+        loading since r271946. It uses curl_easy_pause to pause the
+        resource loads. CURLMOPT_MAX_HOST_CONNECTIONS is set to 6 to limit
+        the number of connections per host. In the certain condition,
+        curl_multi_fdset can constantly return -1 if 6 handles of the same
+        host are paused.
+
+        This patch changes CurlRequest::start not to actually start the
+        job if it is suspended.
+
+        * platform/network/curl/CurlRequest.cpp:
+        (WebCore::CurlRequest::start):
+        (WebCore::CurlRequest::suspend):
+        (WebCore::CurlRequest::resume):
+        (WebCore::CurlRequest::setupTransfer):
+        (WebCore::CurlRequest::setRequestPaused):
+        * platform/network/curl/CurlRequest.h:
+
 2021-07-12  Tim Horton  <timothy_horton@apple.com>
 

trunk/Source/WebCore/platform/network/curl/CurlRequest.cpp

@@ -47 +47 @@
     , m_messageQueue(WTFMove(messageQueue))
     , m_request(request.isolatedCopy())
-    , m_shouldSuspend(shouldSuspend == ShouldSuspend::Yes)
+    , m_startState(shouldSuspend == ShouldSuspend::Yes ? StartState::StartSuspended : StartState::WaitingForStart)
     , m_enableMultipart(enableMultipart == EnableMultipart::Yes)
     , m_formDataStream(m_request.httpBody())
@@ -111 +111 @@
     ASSERT(isMainThread());
 
+    switch (m_startState) {
+    case StartState::DidStart:
+        ASSERT(false);
+        [[fallthrough]];
+    case StartState::StartSuspended:
+        return;
+    }
+
+    m_startState = StartState::DidStart;
+
     if (m_request.url().isLocalFile())
         invokeDidReceiveResponseForFile(m_request.url());
@@ -142 +152 @@
             didCancelTransfer();
         });
-    } else
+    } else if (m_startState == StartState::DidStart)
         scheduler.cancel(this);
 
@@ -164 +174 @@
     ASSERT(isMainThread());
 
-    setRequestPaused(true);
+    switch (m_startState) {
+    case StartState::StartSuspended:
+        ASSERT(false);
+        [[fallthrough]];
+    case StartState::WaitingForStart:
+        m_startState = StartState::StartSuspended;
+        break;
+    case StartState::DidStart:
+        setRequestPaused(true);
+        break;
+    }
 }
 
@@ -171 +191 @@
     ASSERT(isMainThread());
 
-    setRequestPaused(false);
+    switch (m_startState) {
+    case StartState::WaitingForStart:
+        ASSERT(false);
+        [[fallthrough]];
+    case StartState::StartSuspended:
+        m_startState = StartState::WaitingForStart;
+        start();
+        break;
+    case StartState::DidStart:
+        setRequestPaused(false);
+        break;
+    }
 }
 
@@ -239 +270 @@
     m_curlHandle->setTimeout(timeoutInterval());
 
-    if (m_shouldSuspend)
-        setRequestPaused(true);
-
     m_performStartTime = MonotonicTime::now();
 
@@ -644 +672 @@
 
         auto savedState = shouldBePaused();
-        m_shouldSuspend = m_isPausedOfRequest = paused;
+        m_isPausedOfRequest = paused;
         if (shouldBePaused() == savedState)
             return;

trunk/Source/WebCore/platform/network/curl/CurlRequest.h

@@ -176 +176 @@
     unsigned long m_authType { CURLAUTH_ANY };
     bool m_shouldDisableServerTrustEvaluation { false };
-    bool m_shouldSuspend { false };
     bool m_enableMultipart { false };
 
+    enum class StartState : uint8_t { StartSuspended, WaitingForStart, DidStart };
+    StartState m_startState;
+    
     std::unique_ptr<CurlHandle> m_curlHandle;
     CurlFormDataStream m_formDataStream;