Attachment 347108 Details for Bug 188565 – PATCH

[patch] PATCH

188565.diff (text/plain), 7.85 KB, created by Basuke Suzuki on 2018-08-14 14:11:57 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Basuke Suzuki

Created: 2018-08-14 14:11:57 PDT

Size: 7.85 KB

patch

obsolete

>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index ee74960aa01..9c4dfa93490 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-08-14  Basuke Suzuki  <Basuke.Suzuki@sony.com>
>+
>+        [Curl] Implement platform default timeout interval.
>+        https://bugs.webkit.org/show_bug.cgi?id=188565
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Caught by this test:
>+        * http/tests/xmlhttprequest/on-network-timeout-error-during-preflight.html
>+
>+        * platform/wincairo/TestExpectations:
>+
> 2018-08-14  Antoine Quint  <graouts@apple.com>
> 
>         [Web Animations] Crash under AnimationTimeline::cancelOrRemoveDeclarativeAnimation()
>diff --git a/LayoutTests/platform/wincairo/TestExpectations b/LayoutTests/platform/wincairo/TestExpectations
>index ee44d004fee..5ecd9090070 100644
>--- a/LayoutTests/platform/wincairo/TestExpectations
>+++ b/LayoutTests/platform/wincairo/TestExpectations
>@@ -930,6 +930,7 @@ http/tests/workers/worker-redirect.html [ Failure ]
> 
> http/tests/xmlhttprequest/resetting-timeout-to-zero.html [ Slow ]
> http/tests/xmlhttprequest/timeout-greater-than-default-network-timeout.html [ Slow ]
>+http/tests/xmlhttprequest/on-network-timeout-error-during-preflight.html [ Slow ]
> 
> http/tests/xmlhttprequest/access-control-and-redirects.html [ Failure ]
> http/tests/xmlhttprequest/access-control-basic-whitelist-request-headers.html [ Failure ]
>@@ -967,7 +968,6 @@ http/tests/xmlhttprequest/upload-progress-events.html [ Failure ]
> http/tests/xmlhttprequest/workers/methods-async.html [ Failure ]
> http/tests/xmlhttprequest/workers/methods.html [ Failure ]
> 
>-http/tests/xmlhttprequest/on-network-timeout-error-during-preflight.html [ Timeout ]
> http/tests/xmlhttprequest/upload-onabort-progressevent-attributes.html [ Timeout ]
> http/tests/xmlhttprequest/upload-onload-progressevent-attributes.html [ Timeout ]
> http/tests/xmlhttprequest/upload-onloadend-event-after-abort.html [ Timeout ]
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index af596853ede..ca5d80622d5 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2018-08-14  Basuke Suzuki  <Basuke.Suzuki@sony.com>
>+
>+        [Curl] Implement platform default timeout interval.
>+        https://bugs.webkit.org/show_bug.cgi?id=188565
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Curl port didn't implement platform default timeout interval. It treated
>+        zero value for timeout interval as no timeout.
>+
>+        Add platform dependent timeout interval into CurlContext and use that if
>+        timeout and default timeout aren't supplied.
>+
>+        Caught by this test:
>+         * http/tests/xmlhttprequest/on-network-timeout-error-during-preflight.html
>+
>+        * platform/network/curl/CurlContext.cpp:
>+        (WebCore::CurlHandle::setTimeout):
>+        * platform/network/curl/CurlContext.h:
>+        (WebCore::CurlContext::defaultTimeoutInterval const):
>+        * platform/network/curl/CurlRequest.cpp:
>+        (WebCore::CurlRequest::setupTransfer):
>+        (WebCore::CurlRequest::timeoutInterval const):
>+        (WebCore::CurlRequest::didCompleteTransfer):
>+        * platform/network/curl/CurlRequest.h:
>+
> 2018-08-14  Ali Juma  <ajuma@chromium.org>
> 
>         Follow-up: [IntersectionObserver] Implement rootMargin parsing
>diff --git a/Source/WebCore/platform/network/curl/CurlContext.cpp b/Source/WebCore/platform/network/curl/CurlContext.cpp
>index 2c28b62368a..16b760f2923 100644
>--- a/Source/WebCore/platform/network/curl/CurlContext.cpp
>+++ b/Source/WebCore/platform/network/curl/CurlContext.cpp
>@@ -604,7 +604,14 @@ void CurlHandle::setConnectTimeout(Seconds timeout)
> 
> void CurlHandle::setTimeout(Seconds timeout)
> {
>-    curl_easy_setopt(m_handle, CURLOPT_TIMEOUT_MS, safeTimeValue(timeout.milliseconds()));
>+    // Originally CURLOPT_TIMEOUT_MS was used here, but that is not the
>+    // idle timeout, but entire duration time limit. It's not safe to specify
>+    // such a time limit for communications, such as downloading.
>+    // CURLOPT_LOW_SPEED_LIMIT is used instead. It enables the spped watcher
>+    // and if the speed is below specified limit and last for specified duration,
>+    // it invokes timeout error.
>+    curl_easy_setopt(m_handle, CURLOPT_LOW_SPEED_LIMIT, 1L);
>+    curl_easy_setopt(m_handle, CURLOPT_LOW_SPEED_TIME, safeTimeValue(timeout.seconds()));
> }
> 
> void CurlHandle::setHeaderCallbackFunction(curl_write_callback callbackFunc, void* userData)
>diff --git a/Source/WebCore/platform/network/curl/CurlContext.h b/Source/WebCore/platform/network/curl/CurlContext.h
>index 51be6fdc691..5c126af1101 100644
>--- a/Source/WebCore/platform/network/curl/CurlContext.h
>+++ b/Source/WebCore/platform/network/curl/CurlContext.h
>@@ -118,6 +118,7 @@ public:
>     // Timeout
>     Seconds dnsCacheTimeout() const { return m_dnsCacheTimeout; }
>     Seconds connectTimeout() const { return m_connectTimeout; }
>+    Seconds defaultTimeoutInterval() const { return m_defaultTimeoutInterval; }
> 
> #ifndef NDEBUG
>     FILE* getLogFile() const { return m_logFile; }
>@@ -135,6 +136,7 @@ private:
> 
>     Seconds m_dnsCacheTimeout { Seconds::fromMinutes(5) };
>     Seconds m_connectTimeout { 30.0 };
>+    Seconds m_defaultTimeoutInterval { 60.0 };
> 
> #ifndef NDEBUG
>     FILE* m_logFile { nullptr };
>diff --git a/Source/WebCore/platform/network/curl/CurlRequest.cpp b/Source/WebCore/platform/network/curl/CurlRequest.cpp
>index 7e4907620bd..133cb2830c6 100644
>--- a/Source/WebCore/platform/network/curl/CurlRequest.cpp
>+++ b/Source/WebCore/platform/network/curl/CurlRequest.cpp
>@@ -190,7 +190,7 @@ CURL* CurlRequest::setupTransfer()
>     m_curlHandle->setHeaderCallbackFunction(didReceiveHeaderCallback, this);
>     m_curlHandle->setWriteCallbackFunction(didReceiveDataCallback, this);
> 
>-    m_curlHandle->setTimeout(Seconds(m_request.timeoutInterval()));
>+    m_curlHandle->setTimeout(timeoutInterval());
> 
>     if (m_shouldSuspend)
>         setRequestPaused(true);
>@@ -198,6 +198,20 @@ CURL* CurlRequest::setupTransfer()
>     return m_curlHandle->handle();
> }
> 
>+Seconds CurlRequest::timeoutInterval() const
>+{
>+    // Request specific timeout interval.
>+    if (m_request.timeoutInterval())
>+        return Seconds { m_request.timeoutInterval() };
>+
>+    // Default timeout interval set by application.
>+    if (m_request.defaultTimeoutInterval())
>+        return Seconds { m_request.defaultTimeoutInterval() };
>+
>+    // Use platform default timeout interval.
>+    return CurlContext::singleton().defaultTimeoutInterval();
>+}
>+
> // This is called to obtain HTTP POST or PUT data.
> // Iterate through FormData elements and upload files.
> // Carefully respect the given buffer size and fill the rest of the data at the next calls.
>@@ -396,7 +410,7 @@ void CurlRequest::didCompleteTransfer(CURLcode result)
>             client.curlDidComplete(request);
>         });
>     } else {
>-        auto type = (result == CURLE_OPERATION_TIMEDOUT && m_request.timeoutInterval() > 0.0) ? ResourceError::Type::Timeout : ResourceError::Type::General;
>+        auto type = (result == CURLE_OPERATION_TIMEDOUT && timeoutInterval()) ? ResourceError::Type::Timeout : ResourceError::Type::General;
>         auto resourceError = ResourceError::httpError(result, m_request.url(), type);
>         if (auto sslErrors = m_curlHandle->sslErrors())
>             resourceError.setSslErrors(sslErrors);
>diff --git a/Source/WebCore/platform/network/curl/CurlRequest.h b/Source/WebCore/platform/network/curl/CurlRequest.h
>index 8ef83a7fb49..e4cc4a234fd 100644
>--- a/Source/WebCore/platform/network/curl/CurlRequest.h
>+++ b/Source/WebCore/platform/network/curl/CurlRequest.h
>@@ -77,6 +77,7 @@ public:
>     bool isCompleted() const { return !m_curlHandle; }
>     bool isCancelled() const { return m_cancelled; }
>     bool isCompletedOrCancelled() const { return isCompleted() || isCancelled(); }
>+    Seconds timeoutInterval() const;
> 
>     const String& user() const { return m_user; }
>     const String& password() const { return m_password; }

Actions: View | Formatted Diff | Diff

Attachments on bug 188565: 347108 | 347111 | 347112 | 347117