Changeset 256857 in webkit

Timestamp:

Feb 18, 2020 3:09:01 PM (4 years ago)

Author:

youenn@apple.com

Message:

NetworkDataTask should not expect its session wrapper to be always live
​https://bugs.webkit.org/show_bug.cgi?id=207903
rdar://problem/59291486

Reviewed by Alex Christensen.

NetworkDataTaskCocoa should take a weak pointer to its session wrapper.
If the session wrapper is still valid, then we can remove the task from the session wrapper map.
We cannot guarantee session wrapper is valid since NetworkDataTask is ref counted.

• NetworkProcess/cocoa/NetworkDataTaskCocoa.h:
• NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:

(WebKit::NetworkDataTaskCocoa::NetworkDataTaskCocoa):
(WebKit::NetworkDataTaskCocoa::~NetworkDataTaskCocoa):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/cocoa/NetworkDataTaskCocoa.h (modified) (1 diff)
• NetworkProcess/cocoa/NetworkDataTaskCocoa.mm (modified) (3 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-02-18  Youenn Fablet  <youenn@apple.com>
+
+        NetworkDataTask should not expect its session wrapper to be always live
+        https://bugs.webkit.org/show_bug.cgi?id=207903
+        rdar://problem/59291486
+
+        Reviewed by Alex Christensen.
+
+        NetworkDataTaskCocoa should take a weak pointer to its session wrapper.
+        If the session wrapper is still valid, then we can remove the task from the session wrapper map.
+        We cannot guarantee session wrapper is valid since NetworkDataTask is ref counted.
+
+        * NetworkProcess/cocoa/NetworkDataTaskCocoa.h:
+        * NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
+        (WebKit::NetworkDataTaskCocoa::NetworkDataTaskCocoa):
+        (WebKit::NetworkDataTaskCocoa::~NetworkDataTaskCocoa):
+
 2020-02-18  Antti Koivisto  <antti@apple.com>
 

trunk/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.h

@@ -91 +91 @@
     bool isAlwaysOnLoggingAllowed() const;
 
-    SessionWrapper& m_sessionWrapper;
+    WeakPtr<SessionWrapper> m_sessionWrapper;
     RefPtr<SandboxExtension> m_sandboxExtension;
     RetainPtr<NSURLSessionDataTask> m_task;

trunk/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm

@@ -231 +231 @@
 NetworkDataTaskCocoa::NetworkDataTaskCocoa(NetworkSession& session, NetworkDataTaskClient& client, const WebCore::ResourceRequest& requestWithCredentials, WebCore::FrameIdentifier frameID, WebCore::PageIdentifier pageID, WebCore::StoredCredentialsPolicy storedCredentialsPolicy, WebCore::ContentSniffingPolicy shouldContentSniff, WebCore::ContentEncodingSniffingPolicy shouldContentEncodingSniff, bool shouldClearReferrerOnHTTPSToHTTPRedirect, PreconnectOnly shouldPreconnectOnly, bool dataTaskIsForMainFrameNavigation, bool dataTaskIsForMainResourceNavigationForAnyFrame, Optional<NetworkActivityTracker> networkActivityTracker)
     : NetworkDataTask(session, client, requestWithCredentials, storedCredentialsPolicy, shouldClearReferrerOnHTTPSToHTTPRedirect, dataTaskIsForMainFrameNavigation)
-    , m_sessionWrapper(static_cast<NetworkSessionCocoa&>(session).sessionWrapperForTask(requestWithCredentials, storedCredentialsPolicy))
+    , m_sessionWrapper(makeWeakPtr(static_cast<NetworkSessionCocoa&>(session).sessionWrapperForTask(requestWithCredentials, storedCredentialsPolicy)))
     , m_frameID(frameID)
     , m_pageID(pageID)
@@ -278 +278 @@
     applySniffingPoliciesAndBindRequestToInferfaceIfNeeded(nsRequest, shouldContentSniff == WebCore::ContentSniffingPolicy::SniffContent && !url.isLocalFile(), shouldContentEncodingSniff == WebCore::ContentEncodingSniffingPolicy::Sniff);
 
-    m_task = [m_sessionWrapper.session dataTaskWithRequest:nsRequest];
+    m_task = [m_sessionWrapper->session dataTaskWithRequest:nsRequest];
 
     BEGIN_SIGNPOST(m_task, "%{public}s pri: %f preconnect: %d", url.string().ascii().data(), toNSURLSessionTaskPriority(request.priority()), shouldPreconnectOnly == PreconnectOnly::Yes);
 
-    RELEASE_ASSERT(!m_sessionWrapper.dataTaskMap.contains([m_task taskIdentifier]));
-    m_sessionWrapper.dataTaskMap.add([m_task taskIdentifier], this);
+    RELEASE_ASSERT(!m_sessionWrapper->dataTaskMap.contains([m_task taskIdentifier]));
+    m_sessionWrapper->dataTaskMap.add([m_task taskIdentifier], this);
     LOG(NetworkSession, "%llu Creating NetworkDataTask with URL %s", [m_task taskIdentifier], nsRequest.URL.absoluteString.UTF8String);
 
@@ -319 +319 @@
 NetworkDataTaskCocoa::~NetworkDataTaskCocoa()
 {
-    if (!m_task || !m_session)
+    if (!m_task || !m_sessionWrapper)
         return;
 
-    RELEASE_ASSERT(m_sessionWrapper.dataTaskMap.get([m_task taskIdentifier]) == this);
-    m_sessionWrapper.dataTaskMap.remove([m_task taskIdentifier]);
+    RELEASE_ASSERT(m_sessionWrapper->dataTaskMap.get([m_task taskIdentifier]) == this);
+    m_sessionWrapper->dataTaskMap.remove([m_task taskIdentifier]);
 }