[Curl] CurlRequest must protect its client from disposal while it's o…

…n duty.

https://bugs.webkit.org/show_bug.cgi?id=181875

Patch by Basuke Suzuki <Basuke.Suzuki@sony.com> on 2018-01-23
Reviewed by Alex Christensen.

No new tests. It's covered by existing tests.

* platform/network/curl/CurlDownload.h:
* platform/network/curl/CurlRequest.cpp:
(WebCore::CurlRequest::callClient):
(WebCore::CurlRequest::didReceiveData):
(WebCore::CurlRequest::didReceiveDataFromMultipart):
(WebCore::CurlRequest::didCompleteTransfer):
(WebCore::CurlRequest::invokeDidReceiveResponse):
* platform/network/curl/CurlRequest.h:
* platform/network/curl/CurlRequestClient.h:
* platform/network/curl/ResourceHandleCurlDelegate.h:

Canonical link: https://commits.webkit.org/197844@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@227449 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,23 @@
+2018-01-23  Basuke Suzuki  <Basuke.Suzuki@sony.com>
+
+        [Curl] CurlRequest must protect its client from disposal while it's on duty.
+        https://bugs.webkit.org/show_bug.cgi?id=181875
+
+        Reviewed by Alex Christensen.
+
+        No new tests. It's covered by existing tests.
+
+        * platform/network/curl/CurlDownload.h:
+        * platform/network/curl/CurlRequest.cpp:
+        (WebCore::CurlRequest::callClient):
+        (WebCore::CurlRequest::didReceiveData):
+        (WebCore::CurlRequest::didReceiveDataFromMultipart):
+        (WebCore::CurlRequest::didCompleteTransfer):
+        (WebCore::CurlRequest::invokeDidReceiveResponse):
+        * platform/network/curl/CurlRequest.h:
+        * platform/network/curl/CurlRequestClient.h:
+        * platform/network/curl/ResourceHandleCurlDelegate.h:
+
 2018-01-23  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r227437.

Source/WebCore/platform/network/curl/CurlDownload.h

@@ -52,6 +52,9 @@ class CurlDownload : public ThreadSafeRefCounted<CurlDownload>, public CurlReque
     CurlDownload() = default;
     ~CurlDownload();
 
+    void ref() override { ThreadSafeRefCounted<CurlDownload>::ref(); }
+    void deref() override { ThreadSafeRefCounted<CurlDownload>::deref(); }
+
     void init(CurlDownloadListener&, const URL&);
     void init(CurlDownloadListener&, ResourceHandle*, const ResourceRequest&, const ResourceResponse&);
 

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

@@ -142,15 +142,19 @@ void CurlRequest::resume()
 }
 
 /* `this` is protected inside this method. */
-void CurlRequest::callClient(WTF::Function<void(CurlRequestClient*)> task)
+void CurlRequest::callClient(WTF::Function<void(CurlRequestClient&)> task)
 {
     if (isMainThread()) {
-        if (CurlRequestClient* client = m_client)
-            task(client);
+        if (CurlRequestClient* client = m_client) {
+            RefPtr<CurlRequestClient> protectedClient(client);
+            task(*client);
+        }
     } else {
         callOnMainThread([protectedThis = makeRef(*this), task = WTFMove(task)]() mutable {
-            if (CurlRequestClient* client = protectedThis->m_client)
-                task(client);
+            if (CurlRequestClient* client = protectedThis->m_client) {
+                RefPtr<CurlRequestClient> protectedClient(client);
+                task(*client);
+            }
         });
     }
 }
@@ -356,9 +360,8 @@ size_t CurlRequest::didReceiveData(Ref<SharedBuffer>&& buffer)
         if (m_multipartHandle)
             m_multipartHandle->didReceiveData(buffer);
         else {
-            callClient([this, buffer = WTFMove(buffer)](CurlRequestClient* client) mutable {
-                if (client)
-                    client->curlDidReceiveBuffer(WTFMove(buffer));
+            callClient([buffer = WTFMove(buffer)](CurlRequestClient& client) mutable {
+                client.curlDidReceiveBuffer(WTFMove(buffer));
             });
         }
     }
@@ -389,9 +392,8 @@ void CurlRequest::didReceiveDataFromMultipart(Ref<SharedBuffer>&& buffer)
     auto receiveBytes = buffer->size();
 
     if (receiveBytes) {
-        callClient([this, buffer = WTFMove(buffer)](CurlRequestClient* client) mutable {
-            if (client)
-                client->curlDidReceiveBuffer(WTFMove(buffer));
+        callClient([buffer = WTFMove(buffer)](CurlRequestClient& client) mutable {
+            client.curlDidReceiveBuffer(WTFMove(buffer));
         });
     }
 }
@@ -418,9 +420,8 @@ void CurlRequest::didCompleteTransfer(CURLcode result)
                 m_networkLoadMetrics = *metrics;
 
             finalizeTransfer();
-            callClient([this](CurlRequestClient* client) {
-                if (client)
-                    client->curlDidComplete();
+            callClient([](CurlRequestClient& client) {
+                client.curlDidComplete();
             });
         }
     } else {
@@ -430,9 +431,8 @@ void CurlRequest::didCompleteTransfer(CURLcode result)
             resourceError.setSslErrors(m_sslVerifier.sslErrors());
 
         finalizeTransfer();
-        callClient([this, error = resourceError.isolatedCopy()](CurlRequestClient* client) {
-            if (client)
-                client->curlDidFailWithError(error);
+        callClient([error = resourceError.isolatedCopy()](CurlRequestClient& client) {
+            client.curlDidFailWithError(error);
         });
     }
 }
@@ -531,9 +531,8 @@ void CurlRequest::invokeDidReceiveResponse(const CurlResponse& response, Action
     m_didNotifyResponse = true;
     m_actionAfterInvoke = behaviorAfterInvoke;
 
-    callClient([this, response = response.isolatedCopy()](CurlRequestClient* client) {
-        if (client)
-            client->curlDidReceiveResponse(response);
+    callClient([response = response.isolatedCopy()](CurlRequestClient& client) {
+        client.curlDidReceiveResponse(response);
     });
 }
 

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

@@ -102,7 +102,7 @@ class CurlRequest : public ThreadSafeRefCounted<CurlRequest>, public CurlRequest
 
     void startWithJobManager();
 
-    void callClient(WTF::Function<void(CurlRequestClient*)>);
+    void callClient(WTF::Function<void(CurlRequestClient&)>);
 
     // Transfer processing of Request body, Response header/body
     // Called by worker thread in case of async, main thread in case of sync.

Source/WebCore/platform/network/curl/CurlRequestClient.h

@@ -35,6 +35,9 @@ class SharedBuffer;
 
 class CurlRequestClient {
 public:
+    virtual void ref() = 0;
+    virtual void deref() = 0;
+
     virtual void curlDidReceiveResponse(const CurlResponse&) = 0;
     virtual void curlDidReceiveBuffer(Ref<SharedBuffer>&&) = 0;
     virtual void curlDidComplete() = 0;

Source/WebCore/platform/network/curl/ResourceHandleCurlDelegate.h

@@ -43,6 +43,9 @@ class ResourceHandleCurlDelegate final : public ThreadSafeRefCounted<ResourceHan
     ResourceHandleCurlDelegate(ResourceHandle*);
     ~ResourceHandleCurlDelegate();
 
+    void ref() override { ThreadSafeRefCounted<ResourceHandleCurlDelegate>::ref(); }
+    void deref() override { ThreadSafeRefCounted<ResourceHandleCurlDelegate>::deref(); }
+
     bool hasHandle() const;
     void releaseHandle();