Attachment 340232 Details for Bug 185568 – PATCH

[patch] PATCH

185568.diff (text/plain), 11.03 KB, created by Basuke Suzuki on 2018-05-11 16:56:08 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Basuke Suzuki

Created: 2018-05-11 16:56:08 PDT

Size: 11.03 KB

patch

obsolete

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 20d7b2350c5..34e779c0c26 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2018-05-11  Basuke Suzuki  <Basuke.Suzuki@sony.com>
>+
>+        [Curl] Move cookie header handling into background thread.
>+        https://bugs.webkit.org/show_bug.cgi?id=185568
>+
>+        For async (normal) request, current implementation add header for cookie and
>+        store back the cookie data on main thread. Move them into background thread
>+        to get better responsiveness.
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        No new tests. Covered by existing tests.
>+
>+        * platform/network/curl/CurlRequest.cpp:
>+        (WebCore::CurlRequest::setFetchCookie):
>+        (WebCore::CurlRequest::start):
>+        (WebCore::CurlRequest::setupTransfer):
>+        (WebCore::CurlRequest::appendCookieHeader):
>+        (WebCore::CurlRequest::handleCookieHeaders):
>+        (WebCore::CurlRequest::invokeDidReceiveResponse):
>+        * platform/network/curl/CurlRequest.h:
>+        * platform/network/curl/CurlResourceHandleDelegate.cpp:
>+        (WebCore::CurlResourceHandleDelegate::curlDidReceiveResponse):
>+        (WebCore::handleCookieHeaders): Deleted.
>+        * platform/network/curl/ResourceHandleCurl.cpp:
>+        (WebCore::ResourceHandle::createCurlRequest):
>+
> 2018-05-11  Chris Dumez  <cdumez@apple.com>
> 
>         REGRESSION (async policy delegate): Revoking an object URL immediately after triggering download breaks file download
>diff --git a/Source/WebCore/platform/network/curl/CurlRequest.cpp b/Source/WebCore/platform/network/curl/CurlRequest.cpp
>index ad1b9a713e4..43cabc12fba 100644
>--- a/Source/WebCore/platform/network/curl/CurlRequest.cpp
>+++ b/Source/WebCore/platform/network/curl/CurlRequest.cpp
>@@ -28,10 +28,13 @@
> 
> #if USE(CURL)
> 
>+#include "CookiesStrategy.h"
> #include "CurlRequestClient.h"
> #include "CurlRequestScheduler.h"
> #include "MIMETypeRegistry.h"
>+#include "NetworkStorageSession.h"
> #include "ResourceError.h"
>+#include "SameSiteInfo.h"
> #include "SharedBuffer.h"
> #include <wtf/Language.h>
> #include <wtf/MainThread.h>
>@@ -56,6 +59,13 @@ void CurlRequest::setUserPass(const String& user, const String& password)
>     m_password = password.isolatedCopy();
> }
> 
>+void CurlRequest::setFetchCookie(bool flag)
>+{
>+    ASSERT(!m_curlHandle);
>+
>+    m_fetchCookie = flag;
>+}
>+
> void CurlRequest::start(bool isSyncRequest)
> {
>     // The pausing of transfer does not work with protocols, like file://.
>@@ -68,6 +78,7 @@ void CurlRequest::start(bool isSyncRequest)
>     // file : invokeDidReceiveResponseForFile => (MainThread)curlDidReceiveResponse => completeDidReceiveResponse => didReceiveData
> 
>     ASSERT(isMainThread());
>+    ASSERT(!m_curlHandle);
> 
>     m_isSyncRequest = isSyncRequest;
> 
>@@ -166,6 +177,8 @@ CURL* CurlRequest::setupTransfer()
> 
>     auto httpHeaderFields = m_request.httpHeaderFields();
>     appendAcceptLanguageHeader(httpHeaderFields);
>+    if (m_fetchCookie)
>+        appendCookieHeader(httpHeaderFields);
> 
>     m_curlHandle = std::make_unique<CurlHandle>();
> 
>@@ -473,6 +486,17 @@ void CurlRequest::appendAcceptLanguageHeader(HTTPHeaderMap& header)
>         header.add(HTTPHeaderName::AcceptLanguage, language);
> }
> 
>+void CurlRequest::appendCookieHeader(HTTPHeaderMap& header)
>+{
>+    auto& storageSession = NetworkStorageSession::defaultStorageSession();
>+    auto& cookieJar = storageSession.cookieStorage();
>+
>+    auto includeSecureCookies = m_request.url().protocolIs("https") ? IncludeSecureCookies::Yes : IncludeSecureCookies::No;
>+    m_appendedCookieField = cookieJar.cookieRequestHeaderFieldValue(storageSession, m_request.firstPartyForCookies(), SameSiteInfo::create(m_request), m_request.url(), std::nullopt, std::nullopt, includeSecureCookies).first;
>+    if (!m_appendedCookieField.isEmpty())
>+        header.add(HTTPHeaderName::Cookie, m_appendedCookieField);
>+}
>+
> void CurlRequest::setupPUT(ResourceRequest& request)
> {
>     m_curlHandle->enableHttpPutRequest();
>@@ -546,6 +570,20 @@ void CurlRequest::invokeDidReceiveResponseForFile(URL& url)
>     }
> }
> 
>+void CurlRequest::handleCookieHeaders(const CurlResponse& response)
>+{
>+    static const auto setCookieHeader = "set-cookie: ";
>+
>+    const auto& storageSession = NetworkStorageSession::defaultStorageSession();
>+    const auto& cookieJar = storageSession.cookieStorage();
>+    for (const auto& header : response.headers) {
>+        if (header.startsWithIgnoringASCIICase(setCookieHeader)) {
>+            const auto contents = header.right(header.length() - strlen(setCookieHeader));
>+            cookieJar.setCookiesFromHTTPResponse(storageSession, response.url, contents);
>+        }
>+    }
>+}
>+
> void CurlRequest::invokeDidReceiveResponse(const CurlResponse& response, Action behaviorAfterInvoke)
> {
>     ASSERT(!m_didNotifyResponse || m_multipartHandle);
>@@ -553,6 +591,8 @@ void CurlRequest::invokeDidReceiveResponse(const CurlResponse& response, Action
>     m_didNotifyResponse = true;
>     m_actionAfterInvoke = behaviorAfterInvoke;
> 
>+    handleCookieHeaders(response);
>+
>     callClient([response = response.isolatedCopy()](CurlRequest& request, CurlRequestClient& client) {
>         client.curlDidReceiveResponse(request, response);
>     });
>diff --git a/Source/WebCore/platform/network/curl/CurlRequest.h b/Source/WebCore/platform/network/curl/CurlRequest.h
>index 92c451e1ee4..fbfa9013f6a 100644
>--- a/Source/WebCore/platform/network/curl/CurlRequest.h
>+++ b/Source/WebCore/platform/network/curl/CurlRequest.h
>@@ -65,6 +65,7 @@ public:
> 
>     void invalidateClient() { m_client = nullptr;  }
>     WEBCORE_EXPORT void setUserPass(const String&, const String&);
>+    void setFetchCookie(bool);
> 
>     void start(bool isSyncRequest = false);
>     void cancel();
>@@ -122,6 +123,7 @@ private:
> 
>     // For setup 
>     void appendAcceptLanguageHeader(HTTPHeaderMap&);
>+    void appendCookieHeader(HTTPHeaderMap&);
>     void setupPOST(ResourceRequest&);
>     void setupPUT(ResourceRequest&);
>     void setupSendData(bool forPutMethod);
>@@ -129,6 +131,7 @@ private:
>     // Processing for DidReceiveResponse
>     bool needToInvokeDidReceiveResponse() const { return m_didReceiveResponse && (!m_didNotifyResponse || !m_didReturnFromNotify); }
>     bool needToInvokeDidCancelTransfer() const { return m_didNotifyResponse && !m_didReturnFromNotify && m_actionAfterInvoke == Action::FinishTransfer; }
>+    void handleCookieHeaders(const CurlResponse&);
>     void invokeDidReceiveResponseForFile(URL&);
>     void invokeDidReceiveResponse(const CurlResponse&, Action);
>     void setRequestPaused(bool);
>@@ -147,7 +150,6 @@ private:
>     static size_t didReceiveHeaderCallback(char*, size_t, size_t, void*);
>     static size_t didReceiveDataCallback(char*, size_t, size_t, void*);
> 
>-
>     std::atomic<CurlRequestClient*> m_client { };
>     bool m_isSyncRequest { false };
>     bool m_cancelled { false };
>@@ -158,6 +160,8 @@ private:
>     String m_password;
>     bool m_shouldSuspend { false };
>     bool m_enableMultipart { false };
>+    bool m_fetchCookie { false };
>+    String m_appendedCookieField;
> 
>     std::unique_ptr<CurlHandle> m_curlHandle;
>     CurlFormDataStream m_formDataStream;
>diff --git a/Source/WebCore/platform/network/curl/CurlResourceHandleDelegate.cpp b/Source/WebCore/platform/network/curl/CurlResourceHandleDelegate.cpp
>index 416b96872da..a5ceaedbf3b 100644
>--- a/Source/WebCore/platform/network/curl/CurlResourceHandleDelegate.cpp
>+++ b/Source/WebCore/platform/network/curl/CurlResourceHandleDelegate.cpp
>@@ -32,7 +32,6 @@
> 
> #include "CurlCacheManager.h"
> #include "CurlRequest.h"
>-#include "NetworkStorageSession.h"
> #include "ResourceHandle.h"
> #include "ResourceHandleClient.h"
> #include "ResourceHandleInternal.h"
>@@ -85,20 +84,6 @@ void CurlResourceHandleDelegate::curlDidSendData(CurlRequest& request, unsigned
>     client()->didSendData(&m_handle, bytesSent, totalBytesToBeSent);
> }
> 
>-static void handleCookieHeaders(const CurlResponse& response)
>-{
>-    static const auto setCookieHeader = "set-cookie: ";
>-
>-    const auto& storageSession = NetworkStorageSession::defaultStorageSession();
>-    const auto& cookieJar = storageSession.cookieStorage();
>-    for (const auto& header : response.headers) {
>-        if (header.startsWithIgnoringASCIICase(setCookieHeader)) {
>-            const auto contents = header.right(header.length() - strlen(setCookieHeader));
>-            cookieJar.setCookiesFromHTTPResponse(storageSession, response.url, contents);
>-        }
>-    }
>-}
>-
> void CurlResourceHandleDelegate::curlDidReceiveResponse(CurlRequest& request, const CurlResponse& receivedResponse)
> {
>     ASSERT(isMainThread());
>@@ -110,8 +95,6 @@ void CurlResourceHandleDelegate::curlDidReceiveResponse(CurlRequest& request, co
>     m_response = ResourceResponse(receivedResponse);
>     m_response.setDeprecatedNetworkLoadMetrics(request.getNetworkLoadMetrics());
> 
>-    handleCookieHeaders(receivedResponse);
>-
>     if (m_response.shouldRedirect()) {
>         m_handle.willSendRequest();
>         return;
>diff --git a/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp b/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp
>index cc4d3cc1d02..ae576c5b175 100644
>--- a/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp
>+++ b/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp
>@@ -33,7 +33,6 @@
> #if USE(CURL)
> 
> #include "CookieJarCurl.h"
>-#include "CookiesStrategy.h"
> #include "CredentialStorage.h"
> #include "CurlCacheManager.h"
> #include "CurlContext.h"
>@@ -41,9 +40,7 @@
> #include "FileSystem.h"
> #include "HTTPParsers.h"
> #include "Logging.h"
>-#include "NetworkStorageSession.h"
> #include "ResourceHandleInternal.h"
>-#include "SameSiteInfo.h"
> #include "SharedBuffer.h"
> #include "SynchronousLoaderClient.h"
> #include "TextEncoding.h"
>@@ -140,20 +137,14 @@ Ref<CurlRequest> ResourceHandle::createCurlRequest(ResourceRequest& request, Req
> {
>     ASSERT(isMainThread());
> 
>+    CurlRequest::ShouldSuspend shouldSuspend = d->m_defersLoading ? CurlRequest::ShouldSuspend::Yes : CurlRequest::ShouldSuspend::No;
>+    auto curlRequest = CurlRequest::create(request, *delegate(), shouldSuspend, CurlRequest::EnableMultipart::Yes);
>+
>     if (status == RequestStatus::NewRequest) {
>         addCacheValidationHeaders(request);
>-
>-        auto& storageSession = NetworkStorageSession::defaultStorageSession();
>-        auto& cookieJar = storageSession.cookieStorage();
>-        auto includeSecureCookies = request.url().protocolIs("https") ? IncludeSecureCookies::Yes : IncludeSecureCookies::No;
>-        String cookieHeaderField = cookieJar.cookieRequestHeaderFieldValue(storageSession, request.firstPartyForCookies(), SameSiteInfo::create(request), request.url(), std::nullopt, std::nullopt, includeSecureCookies).first;
>-        if (!cookieHeaderField.isEmpty())
>-            request.addHTTPHeaderField(HTTPHeaderName::Cookie, cookieHeaderField);
>+        curlRequest->setFetchCookie(true);
>     }
> 
>-    CurlRequest::ShouldSuspend shouldSuspend = d->m_defersLoading ? CurlRequest::ShouldSuspend::Yes : CurlRequest::ShouldSuspend::No;
>-    auto curlRequest = CurlRequest::create(request, *delegate(), shouldSuspend, CurlRequest::EnableMultipart::Yes);
>-    
>     return curlRequest;
> }
>

Flags:

ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 185568: 340232 | 340237 | 340241 | 340244