Changeset 233842 in webkit

Timestamp:

Jul 15, 2018 10:22:36 PM (6 years ago)

Author:

Carlos Garcia Campos

Message:

[SOUP] http/tests/misc/bubble-drag-events.html crashes
​https://bugs.webkit.org/show_bug.cgi?id=182352

Reviewed by Youenn Fablet.

PingLoad is not refcounted and deletes itself when the load finishes. The problem is that the network data
task can also finish the load, causing the PingLoad to be deleted early. We tried to fix it in r233032 in the
network data task soup implementation, but it caused regressions.

• NetworkProcess/PingLoad.cpp:

(WebKit::PingLoad::didReceiveChallenge): Create a weak ref for the ping load before calling the operation completion
handler and return early after the completion handler if it was deleted.
(WebKit::PingLoad::didReceiveResponseNetworkSession): Ditto.

• NetworkProcess/PingLoad.h:
• NetworkProcess/soup/NetworkDataTaskSoup.cpp:

(WebKit::NetworkDataTaskSoup::continueAuthenticate): Revert r233032.

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/PingLoad.cpp (modified) (2 diffs)
• NetworkProcess/PingLoad.h (modified) (2 diffs)
• NetworkProcess/soup/NetworkDataTaskSoup.cpp (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2018-07-15  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [SOUP] http/tests/misc/bubble-drag-events.html crashes
+        https://bugs.webkit.org/show_bug.cgi?id=182352
+
+        Reviewed by Youenn Fablet.
+
+        PingLoad is not refcounted and deletes itself when the load finishes. The problem is that the network data
+        task can also finish the load, causing the PingLoad to be deleted early. We tried to fix it in r233032 in the
+        network data task soup implementation, but it caused regressions.
+
+        * NetworkProcess/PingLoad.cpp:
+        (WebKit::PingLoad::didReceiveChallenge): Create a weak ref for the ping load before calling the operation completion
+        handler and return early after the completion handler if it was deleted.
+        (WebKit::PingLoad::didReceiveResponseNetworkSession): Ditto.
+        * NetworkProcess/PingLoad.h:
+        * NetworkProcess/soup/NetworkDataTaskSoup.cpp:
+        (WebKit::NetworkDataTaskSoup::continueAuthenticate): Revert r233032.
+
 2018-07-13  Timothy Hatcher  <timothy@apple.com>
 

trunk/Source/WebKit/NetworkProcess/PingLoad.cpp

@@ -115 +115 @@
 {
     RELEASE_LOG_IF_ALLOWED("didReceiveChallenge");
+    auto weakThis = makeWeakPtr(*this);
     completionHandler(AuthenticationChallengeDisposition::Cancel, { });
+    if (!weakThis)
+        return;
     didFinish(ResourceError { String(), 0, currentURL(), "Failed HTTP authentication"_s, ResourceError::Type::AccessControl });
 }
@@ -122 +125 @@
 {
     RELEASE_LOG_IF_ALLOWED("didReceiveResponseNetworkSession - httpStatusCode: %d", response.httpStatusCode());
+    auto weakThis = makeWeakPtr(*this);
     completionHandler(PolicyAction::Ignore);
+    if (!weakThis)
+        return;
     didFinish({ }, response);
 }

trunk/Source/WebKit/NetworkProcess/PingLoad.h

@@ -32 +32 @@
 #include <wtf/CompletionHandler.h>
 #include <wtf/UniqueRef.h>
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
@@ -44 +45 @@
 class NetworkLoadChecker;
 
-class PingLoad final : private NetworkDataTaskClient {
+class PingLoad final : public CanMakeWeakPtr<PingLoad>, private NetworkDataTaskClient {
 public:
     PingLoad(NetworkResourceLoadParameters&&, CompletionHandler<void(const WebCore::ResourceError&, const WebCore::ResourceResponse&)>&&);

trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp

@@ -528 +528 @@
 
         if (disposition == AuthenticationChallengeDisposition::Cancel) {
-            invalidateAndCancel();
+            cancel();
+            didFail(cancelledError(m_soupRequest.get()));
             return;
         }