[Curl] Bug fix for synchronous transfer
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Sep 2017 23:33:56 +0000 (23:33 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Sep 2017 23:33:56 +0000 (23:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=176552

Patch by Basuke Suzuki <Basuke.Suzuki@sony.com> on 2017-09-13
Reviewed by Alex Christensen.

ResourceHandleInternal::m_delegate is null when transfer is synchronous. It should be set ResourceHandleCurlDelegate.
Also the callback functions called when transfer is completed is wrong.

* platform/network/curl/ResourceHandleCurl.cpp:
(WebCore::ResourceHandle::platformLoadResourceSynchronously):
* platform/network/curl/ResourceHandleCurlDelegate.cpp:
(WebCore::ResourceHandleCurlDelegate::dispatchSynchronousJob):
(WebCore::ResourceHandleCurlDelegate::notifyFinish):
(WebCore::ResourceHandleCurlDelegate::notifyFail):
(WebCore::ResourceHandleCurlDelegate::didReceiveHeader):
(WebCore::ResourceHandleCurlDelegate::didReceiveData):
(WebCore::ResourceHandleCurlDelegate::willSendData):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@221999 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp
Source/WebCore/platform/network/curl/ResourceHandleCurlDelegate.cpp

index e883c09..59d6ac1 100644 (file)
@@ -1,3 +1,23 @@
+2017-09-13  Basuke Suzuki  <Basuke.Suzuki@sony.com>
+
+        [Curl] Bug fix for synchronous transfer
+        https://bugs.webkit.org/show_bug.cgi?id=176552
+
+        Reviewed by Alex Christensen.
+
+        ResourceHandleInternal::m_delegate is null when transfer is synchronous. It should be set ResourceHandleCurlDelegate.
+        Also the callback functions called when transfer is completed is wrong.
+
+        * platform/network/curl/ResourceHandleCurl.cpp:
+        (WebCore::ResourceHandle::platformLoadResourceSynchronously):
+        * platform/network/curl/ResourceHandleCurlDelegate.cpp:
+        (WebCore::ResourceHandleCurlDelegate::dispatchSynchronousJob):
+        (WebCore::ResourceHandleCurlDelegate::notifyFinish):
+        (WebCore::ResourceHandleCurlDelegate::notifyFail):
+        (WebCore::ResourceHandleCurlDelegate::didReceiveHeader):
+        (WebCore::ResourceHandleCurlDelegate::didReceiveData):
+        (WebCore::ResourceHandleCurlDelegate::willSendData):
+
 2017-09-13  Matt Lewis  <jlewis3@apple.com>
 
         Unreviewed, rolling out r221976.
index 3e1b3b0..db9d53a 100644 (file)
@@ -230,8 +230,6 @@ void ResourceHandle::receivedChallengeRejection(const AuthenticationChallenge&)
     ASSERT_NOT_REACHED();
 }
 
-// sync loader
-
 void ResourceHandle::platformLoadResourceSynchronously(NetworkingContext* context, const ResourceRequest& request, StoredCredentials, ResourceError& error, ResourceResponse& response, Vector<char>& data)
 {
     ASSERT(isMainThread());
@@ -239,6 +237,7 @@ void ResourceHandle::platformLoadResourceSynchronously(NetworkingContext* contex
     SynchronousLoaderClient client;
     RefPtr<ResourceHandle> handle = adoptRef(new ResourceHandle(context, request, &client, false, false));
 
+    handle->d->m_delegate = adoptRef(new ResourceHandleCurlDelegate(handle.get()));
     handle->d->m_delegate->dispatchSynchronousJob();
 
     error = client.error();
index cd5a7db..2b01d6a 100644 (file)
@@ -171,18 +171,10 @@ void ResourceHandleCurlDelegate::dispatchSynchronousJob()
     m_curlHandle.getTimes(pretransferTime, dnsLookupTime, connectTime, appConnectTime);
     setWebTimings(pretransferTime, dnsLookupTime, connectTime, appConnectTime);
 
-    if (m_handle->client()) {
-        if (ret != CURLE_OK) {
-            String domain = CurlContext::errorDomain;
-            int errorCode = m_curlHandle.errorCode();
-            URL failingURL = m_curlHandle.getEffectiveURL();
-            String errorDescription = m_curlHandle.errorDescription();
-            unsigned sslErrors = m_curlHandle.getSslErrors();
-
-            m_handle->client()->didFail(m_handle, ResourceError(domain, errorCode, failingURL, errorDescription, sslErrors));
-        } else
-            m_handle->client()->didReceiveResponse(m_handle, ResourceResponse(response()));
-    }
+    if (ret != CURLE_OK)
+        notifyFail();
+    else
+        notifyFinish();
 }
 
 void ResourceHandleCurlDelegate::retain()
@@ -291,11 +283,15 @@ void ResourceHandleCurlDelegate::notifyFinish()
 
     m_curlHandle.getTimes(pretransferTime, dnsLookupTime, connectTime, appConnectTime);
 
-    callOnMainThread([protectedThis = makeRef(*this), pretransferTime, dnsLookupTime, connectTime, appConnectTime] {
-        if (!protectedThis->m_handle)
-            return;
-        protectedThis->didFinish(pretransferTime, dnsLookupTime, connectTime, appConnectTime);
-    });
+    if (isMainThread())
+        didFinish(pretransferTime, dnsLookupTime, connectTime, appConnectTime);
+    else {
+        callOnMainThread([protectedThis = makeRef(*this), pretransferTime, dnsLookupTime, connectTime, appConnectTime] {
+            if (!protectedThis->m_handle)
+                return;
+            protectedThis->didFinish(pretransferTime, dnsLookupTime, connectTime, appConnectTime);
+        });
+    }
 }
 
 void ResourceHandleCurlDelegate::notifyFail()
@@ -306,11 +302,15 @@ void ResourceHandleCurlDelegate::notifyFail()
     String errorDescription = m_curlHandle.errorDescription();
     unsigned sslErrors = m_curlHandle.getSslErrors();
 
-    callOnMainThread([protectedThis = makeRef(*this), domain = domain.isolatedCopy(), errorCode, failingURL = failingURL.isolatedCopy(), errorDescription = errorDescription.isolatedCopy(), sslErrors] {
-        if (!protectedThis->m_handle)
-            return;
-        protectedThis->didFail(domain, errorCode, failingURL, errorDescription, sslErrors);
-    });
+    if (isMainThread())
+        didFail(domain, errorCode, failingURL, errorDescription, sslErrors);
+    else {
+        callOnMainThread([protectedThis = makeRef(*this), domain = domain.isolatedCopy(), errorCode, failingURL = failingURL.isolatedCopy(), errorDescription = errorDescription.isolatedCopy(), sslErrors] {
+            if (!protectedThis->m_handle)
+                return;
+            protectedThis->didFail(domain, errorCode, failingURL, errorDescription, sslErrors);
+        });
+    }
 }
 
 ResourceResponse& ResourceHandleCurlDelegate::response()
@@ -868,17 +868,25 @@ size_t ResourceHandleCurlDelegate::didReceiveHeader(String&& header)
         long long contentLength = 0;
         m_curlHandle.getContentLenghtDownload(contentLength);
 
-        callOnMainThread([protectedThis = makeRef(*this), httpCode, contentLength] {
-            if (!protectedThis->m_handle)
-                return;
-            protectedThis->didReceiveAllHeaders(httpCode, contentLength);
-        });
+        if (isMainThread())
+            didReceiveAllHeaders(httpCode, contentLength);
+        else {
+            callOnMainThread([protectedThis = makeRef(*this), httpCode, contentLength] {
+                if (!protectedThis->m_handle)
+                    return;
+                protectedThis->didReceiveAllHeaders(httpCode, contentLength);
+            });
+        }
     } else {
-        callOnMainThread([protectedThis = makeRef(*this), header = header.isolatedCopy() ] {
-            if (!protectedThis->m_handle)
-                return;
-            protectedThis->didReceiveHeaderLine(header);
-        });
+        if (isMainThread())
+            didReceiveHeaderLine(header);
+        else {
+            callOnMainThread([protectedThis = makeRef(*this), header = header.isolatedCopy() ] {
+                if (!protectedThis->m_handle)
+                    return;
+                protectedThis->didReceiveHeaderLine(header);
+            });
+        }
     }
 
     return header.length();
@@ -904,11 +912,15 @@ size_t ResourceHandleCurlDelegate::didReceiveData(ThreadSafeDataBuffer data)
     if (!data.size())
         return 0;
 
-    callOnMainThread([protectedThis = makeRef(*this), data] {
-        if (!protectedThis->m_handle)
-            return;
-        protectedThis->didReceiveContentData(data);
-    });
+    if (isMainThread())
+        didReceiveContentData(data);
+    else {
+        callOnMainThread([protectedThis = makeRef(*this), data] {
+            if (!protectedThis->m_handle)
+                return;
+            protectedThis->didReceiveContentData(data);
+        });
+    }
 
     return data.size();
 }
@@ -935,15 +947,19 @@ size_t ResourceHandleCurlDelegate::willSendData(char* buffer, size_t blockSize,
 
         m_sendBytes = 0;
 
-        callOnMainThread([protectedThis = makeRef(*this), buffer, blockSize, numberOfBlocks] {
-            if (!protectedThis->m_handle)
-                return;
-            protectedThis->prepareSendData(buffer, blockSize, numberOfBlocks);
-        });
-
-        m_workerThreadConditionVariable.wait(lock, [this] {
-            return m_sendBytes;
-        });
+        if (isMainThread())
+            prepareSendData(buffer, blockSize, numberOfBlocks);
+        else {
+            callOnMainThread([protectedThis = makeRef(*this), buffer, blockSize, numberOfBlocks] {
+                if (!protectedThis->m_handle)
+                    return;
+                protectedThis->prepareSendData(buffer, blockSize, numberOfBlocks);
+            });
+
+            m_workerThreadConditionVariable.wait(lock, [this] {
+                return m_sendBytes;
+            });
+        }
     }
 
     return m_sendBytes;