- Revision
- 197413
- Author
- achristen...@apple.com
- Date
- 2016-03-01 14:13:00 -0800 (Tue, 01 Mar 2016)
Log Message
Correctly keep track of NetworkDataTasks with and without credentials when using NetworkSession
https://bugs.webkit.org/show_bug.cgi?id=154876
Reviewed by Brady Eidson.
I was seeing an assertion failure from ASSERT(!m_session.m_dataTaskMap.contains(taskIdentifier()))
in the NetworkDataTask constructor sometimes. This is because a task identifier is not enough information
to uniquely find a NetworkDataTask in a NetworkSession since r196034 because there are two NSURLSessions
in a NetworkSession, one with credentials and one without. The assertion would fire in a case like if we
made the first NetworkDataTask with credentials (taskIdentifier is 1) and the first NetworkDataTask
without credentials before the first NetworkDataTask with credentials was finished. In that case, the
taskIdentifier would also be 1, which would conflict with the other taskIdentifier. That taskIdentifier
would uniquely identify the task in the correct NSURLSession, though, so the solution is to keep a map
for each NSURLSession in the NetworkSession.
* NetworkProcess/NetworkDataTask.h:
* NetworkProcess/NetworkSession.h:
* NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
(WebKit::NetworkDataTask::NetworkDataTask):
(WebKit::NetworkDataTask::~NetworkDataTask):
(WebKit::NetworkDataTask::suspend):
(WebKit::serverTrustCredential):
* NetworkProcess/cocoa/NetworkSessionCocoa.mm:
(-[WKNetworkSessionDelegate URLSession:task:didSendBodyData:totalBytesSent:totalBytesExpectedToSend:]):
(-[WKNetworkSessionDelegate URLSession:task:willPerformHTTPRedirection:newRequest:completionHandler:]):
(-[WKNetworkSessionDelegate URLSession:task:didReceiveChallenge:completionHandler:]):
(-[WKNetworkSessionDelegate URLSession:task:didCompleteWithError:]):
(-[WKNetworkSessionDelegate URLSession:dataTask:didReceiveResponse:completionHandler:]):
(-[WKNetworkSessionDelegate URLSession:dataTask:didReceiveData:]):
(-[WKNetworkSessionDelegate URLSession:downloadTask:didWriteData:totalBytesWritten:totalBytesExpectedToWrite:]):
(-[WKNetworkSessionDelegate URLSession:dataTask:didBecomeDownloadTask:]):
(WebKit::NetworkSession::clearCredentials):
(WebKit::NetworkSession::dataTaskForIdentifier):
(WebKit::NetworkSession::addDownloadID):
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (197412 => 197413)
--- trunk/Source/WebKit2/ChangeLog 2016-03-01 21:45:16 UTC (rev 197412)
+++ trunk/Source/WebKit2/ChangeLog 2016-03-01 22:13:00 UTC (rev 197413)
@@ -1,3 +1,40 @@
+2016-03-01 Alex Christensen <achristen...@webkit.org>
+
+ Correctly keep track of NetworkDataTasks with and without credentials when using NetworkSession
+ https://bugs.webkit.org/show_bug.cgi?id=154876
+
+ Reviewed by Brady Eidson.
+
+ I was seeing an assertion failure from ASSERT(!m_session.m_dataTaskMap.contains(taskIdentifier()))
+ in the NetworkDataTask constructor sometimes. This is because a task identifier is not enough information
+ to uniquely find a NetworkDataTask in a NetworkSession since r196034 because there are two NSURLSessions
+ in a NetworkSession, one with credentials and one without. The assertion would fire in a case like if we
+ made the first NetworkDataTask with credentials (taskIdentifier is 1) and the first NetworkDataTask
+ without credentials before the first NetworkDataTask with credentials was finished. In that case, the
+ taskIdentifier would also be 1, which would conflict with the other taskIdentifier. That taskIdentifier
+ would uniquely identify the task in the correct NSURLSession, though, so the solution is to keep a map
+ for each NSURLSession in the NetworkSession.
+
+ * NetworkProcess/NetworkDataTask.h:
+ * NetworkProcess/NetworkSession.h:
+ * NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
+ (WebKit::NetworkDataTask::NetworkDataTask):
+ (WebKit::NetworkDataTask::~NetworkDataTask):
+ (WebKit::NetworkDataTask::suspend):
+ (WebKit::serverTrustCredential):
+ * NetworkProcess/cocoa/NetworkSessionCocoa.mm:
+ (-[WKNetworkSessionDelegate URLSession:task:didSendBodyData:totalBytesSent:totalBytesExpectedToSend:]):
+ (-[WKNetworkSessionDelegate URLSession:task:willPerformHTTPRedirection:newRequest:completionHandler:]):
+ (-[WKNetworkSessionDelegate URLSession:task:didReceiveChallenge:completionHandler:]):
+ (-[WKNetworkSessionDelegate URLSession:task:didCompleteWithError:]):
+ (-[WKNetworkSessionDelegate URLSession:dataTask:didReceiveResponse:completionHandler:]):
+ (-[WKNetworkSessionDelegate URLSession:dataTask:didReceiveData:]):
+ (-[WKNetworkSessionDelegate URLSession:downloadTask:didWriteData:totalBytesWritten:totalBytesExpectedToWrite:]):
+ (-[WKNetworkSessionDelegate URLSession:dataTask:didBecomeDownloadTask:]):
+ (WebKit::NetworkSession::clearCredentials):
+ (WebKit::NetworkSession::dataTaskForIdentifier):
+ (WebKit::NetworkSession::addDownloadID):
+
2016-03-01 Andreas Kling <akl...@apple.com>
REGRESSION (r154616): Accelerated drawing is off during the initial load
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkDataTask.h (197412 => 197413)
--- trunk/Source/WebKit2/NetworkProcess/NetworkDataTask.h 2016-03-01 21:45:16 UTC (rev 197412)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkDataTask.h 2016-03-01 22:13:00 UTC (rev 197413)
@@ -93,7 +93,6 @@
void resume();
typedef uint64_t TaskIdentifier;
- TaskIdentifier taskIdentifier();
~NetworkDataTask();
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkSession.h (197412 => 197413)
--- trunk/Source/WebKit2/NetworkProcess/NetworkSession.h 2016-03-01 21:45:16 UTC (rev 197412)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkSession.h 2016-03-01 22:13:00 UTC (rev 197413)
@@ -57,14 +57,15 @@
static NetworkSession& defaultSession();
void clearCredentials();
- NetworkDataTask* dataTaskForIdentifier(NetworkDataTask::TaskIdentifier);
+ NetworkDataTask* dataTaskForIdentifier(NetworkDataTask::TaskIdentifier, WebCore::StoredCredentials);
void addDownloadID(NetworkDataTask::TaskIdentifier, DownloadID);
DownloadID downloadID(NetworkDataTask::TaskIdentifier);
DownloadID takeDownloadID(NetworkDataTask::TaskIdentifier);
private:
- HashMap<NetworkDataTask::TaskIdentifier, NetworkDataTask*> m_dataTaskMap;
+ HashMap<NetworkDataTask::TaskIdentifier, NetworkDataTask*> m_dataTaskMapWithCredentials;
+ HashMap<NetworkDataTask::TaskIdentifier, NetworkDataTask*> m_dataTaskMapWithoutCredentials;
HashMap<NetworkDataTask::TaskIdentifier, DownloadID> m_downloadMap;
#if PLATFORM(COCOA)
RetainPtr<NSURLSession> m_sessionWithCredentialStorage;
Modified: trunk/Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm (197412 => 197413)
--- trunk/Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm 2016-03-01 21:45:16 UTC (rev 197412)
+++ trunk/Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm 2016-03-01 22:13:00 UTC (rev 197413)
@@ -76,22 +76,28 @@
nsRequest = mutableRequest;
}
- if (storedCredentials == WebCore::AllowStoredCredentials)
+ if (storedCredentials == WebCore::AllowStoredCredentials) {
m_task = [m_session.m_sessionWithCredentialStorage dataTaskWithRequest:nsRequest];
- else
+ ASSERT(!m_session.m_dataTaskMapWithCredentials.contains([m_task taskIdentifier]));
+ m_session.m_dataTaskMapWithCredentials.add([m_task taskIdentifier], this);
+ } else {
m_task = [m_session.m_sessionWithoutCredentialStorage dataTaskWithRequest:nsRequest];
-
- ASSERT(!m_session.m_dataTaskMap.contains(taskIdentifier()));
- m_session.m_dataTaskMap.add(taskIdentifier(), this);
+ ASSERT(!m_session.m_dataTaskMapWithoutCredentials.contains([m_task taskIdentifier]));
+ m_session.m_dataTaskMapWithoutCredentials.add([m_task taskIdentifier], this);
+ }
}
NetworkDataTask::~NetworkDataTask()
{
+ ASSERT(isMainThread());
if (m_task) {
- ASSERT(m_session.m_dataTaskMap.contains(taskIdentifier()));
- ASSERT(m_session.m_dataTaskMap.get(taskIdentifier()) == this);
- ASSERT(isMainThread());
- m_session.m_dataTaskMap.remove(taskIdentifier());
+ if (m_storedCredentials == WebCore::StoredCredentials::AllowStoredCredentials) {
+ ASSERT(m_session.m_dataTaskMapWithCredentials.get([m_task taskIdentifier]) == this);
+ m_session.m_dataTaskMapWithCredentials.remove([m_task taskIdentifier]);
+ } else {
+ ASSERT(m_session.m_dataTaskMapWithoutCredentials.get([m_task taskIdentifier]) == this);
+ m_session.m_dataTaskMapWithoutCredentials.remove([m_task taskIdentifier]);
+ }
}
}
@@ -260,11 +266,6 @@
[m_task suspend];
}
-auto NetworkDataTask::taskIdentifier() -> TaskIdentifier
-{
- return [m_task taskIdentifier];
-}
-
WebCore::Credential serverTrustCredential(const WebCore::AuthenticationChallenge& challenge)
{
return WebCore::Credential([NSURLCredential credentialForTrust:challenge.nsURLAuthenticationChallenge().protectionSpace.serverTrust]);
Modified: trunk/Source/WebKit2/NetworkProcess/cocoa/NetworkSessionCocoa.mm (197412 => 197413)
--- trunk/Source/WebKit2/NetworkProcess/cocoa/NetworkSessionCocoa.mm 2016-03-01 21:45:16 UTC (rev 197412)
+++ trunk/Source/WebKit2/NetworkProcess/cocoa/NetworkSessionCocoa.mm 2016-03-01 22:13:00 UTC (rev 197413)
@@ -98,13 +98,15 @@
- (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didSendBodyData:(int64_t)bytesSent totalBytesSent:(int64_t)totalBytesSent totalBytesExpectedToSend:(int64_t)totalBytesExpectedToSend
{
- if (auto* networkDataTask = _session->dataTaskForIdentifier(task.taskIdentifier))
+ auto storedCredentials = session.configuration.URLCredentialStorage ? WebCore::StoredCredentials::AllowStoredCredentials : WebCore::StoredCredentials::DoNotAllowStoredCredentials;
+ if (auto* networkDataTask = _session->dataTaskForIdentifier(task.taskIdentifier, storedCredentials))
networkDataTask->didSendData(totalBytesSent, totalBytesExpectedToSend);
}
- (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task willPerformHTTPRedirection:(NSHTTPURLResponse *)response newRequest:(NSURLRequest *)request completionHandler:(void (^)(NSURLRequest *))completionHandler
{
- if (auto* networkDataTask = _session->dataTaskForIdentifier(task.taskIdentifier)) {
+ auto storedCredentials = session.configuration.URLCredentialStorage ? WebCore::StoredCredentials::AllowStoredCredentials : WebCore::StoredCredentials::DoNotAllowStoredCredentials;
+ if (auto* networkDataTask = _session->dataTaskForIdentifier(task.taskIdentifier, storedCredentials)) {
auto completionHandlerCopy = Block_copy(completionHandler);
networkDataTask->willPerformHTTPRedirection(response, request, [completionHandlerCopy](const WebCore::ResourceRequest& request) {
completionHandlerCopy(request.nsURLRequest(WebCore::UpdateHTTPBody));
@@ -115,7 +117,8 @@
- (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didReceiveChallenge:(NSURLAuthenticationChallenge *)challenge completionHandler:(void (^)(NSURLSessionAuthChallengeDisposition disposition, NSURLCredential *credential))completionHandler
{
- if (auto* networkDataTask = _session->dataTaskForIdentifier(task.taskIdentifier)) {
+ auto storedCredentials = session.configuration.URLCredentialStorage ? WebCore::StoredCredentials::AllowStoredCredentials : WebCore::StoredCredentials::DoNotAllowStoredCredentials;
+ if (auto* networkDataTask = _session->dataTaskForIdentifier(task.taskIdentifier, storedCredentials)) {
auto completionHandlerCopy = Block_copy(completionHandler);
auto challengeCompletionHandler = [completionHandlerCopy](WebKit::AuthenticationChallengeDisposition disposition, const WebCore::Credential& credential)
{
@@ -132,7 +135,8 @@
- (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didCompleteWithError:(NSError *)error
{
- if (auto* networkDataTask = _session->dataTaskForIdentifier(task.taskIdentifier))
+ auto storedCredentials = session.configuration.URLCredentialStorage ? WebCore::StoredCredentials::AllowStoredCredentials : WebCore::StoredCredentials::DoNotAllowStoredCredentials;
+ if (auto* networkDataTask = _session->dataTaskForIdentifier(task.taskIdentifier, storedCredentials))
networkDataTask->didCompleteWithError(error);
else if (error) {
auto downloadID = _session->takeDownloadID(task.taskIdentifier);
@@ -145,7 +149,8 @@
- (void)URLSession:(NSURLSession *)session dataTask:(NSURLSessionDataTask *)dataTask didReceiveResponse:(NSURLResponse *)response completionHandler:(void (^)(NSURLSessionResponseDisposition disposition))completionHandler
{
- if (auto* networkDataTask = _session->dataTaskForIdentifier(dataTask.taskIdentifier)) {
+ auto storedCredentials = session.configuration.URLCredentialStorage ? WebCore::StoredCredentials::AllowStoredCredentials : WebCore::StoredCredentials::DoNotAllowStoredCredentials;
+ if (auto* networkDataTask = _session->dataTaskForIdentifier(dataTask.taskIdentifier, storedCredentials)) {
ASSERT(isMainThread());
WebCore::ResourceResponse resourceResponse(response);
copyTimingData([dataTask _timingData], resourceResponse.resourceLoadTiming());
@@ -159,7 +164,8 @@
- (void)URLSession:(NSURLSession *)session dataTask:(NSURLSessionDataTask *)dataTask didReceiveData:(NSData *)data
{
- if (auto* networkDataTask = _session->dataTaskForIdentifier(dataTask.taskIdentifier))
+ auto storedCredentials = session.configuration.URLCredentialStorage ? WebCore::StoredCredentials::AllowStoredCredentials : WebCore::StoredCredentials::DoNotAllowStoredCredentials;
+ if (auto* networkDataTask = _session->dataTaskForIdentifier(dataTask.taskIdentifier, storedCredentials))
networkDataTask->didReceiveData(WebCore::SharedBuffer::wrapNSData(data));
}
@@ -172,7 +178,8 @@
- (void)URLSession:(NSURLSession *)session downloadTask:(NSURLSessionDownloadTask *)downloadTask didWriteData:(int64_t)bytesWritten totalBytesWritten:(int64_t)totalBytesWritten totalBytesExpectedToWrite:(int64_t)totalBytesExpectedToWrite
{
- ASSERT_WITH_MESSAGE(!_session->dataTaskForIdentifier([downloadTask taskIdentifier]), "The NetworkDataTask should be destroyed immediately after didBecomeDownloadTask returns");
+ auto storedCredentials = session.configuration.URLCredentialStorage ? WebCore::StoredCredentials::AllowStoredCredentials : WebCore::StoredCredentials::DoNotAllowStoredCredentials;
+ ASSERT_WITH_MESSAGE(!_session->dataTaskForIdentifier([downloadTask taskIdentifier], storedCredentials), "The NetworkDataTask should be destroyed immediately after didBecomeDownloadTask returns");
auto downloadID = _session->downloadID([downloadTask taskIdentifier]);
if (auto* download = WebKit::NetworkProcess::singleton().downloadManager().download(downloadID))
@@ -186,7 +193,8 @@
- (void)URLSession:(NSURLSession *)session dataTask:(NSURLSessionDataTask *)dataTask didBecomeDownloadTask:(NSURLSessionDownloadTask *)downloadTask
{
- if (auto* networkDataTask = _session->dataTaskForIdentifier([dataTask taskIdentifier])) {
+ auto storedCredentials = session.configuration.URLCredentialStorage ? WebCore::StoredCredentials::AllowStoredCredentials : WebCore::StoredCredentials::DoNotAllowStoredCredentials;
+ if (auto* networkDataTask = _session->dataTaskForIdentifier([dataTask taskIdentifier], storedCredentials)) {
auto downloadID = networkDataTask->pendingDownloadID();
auto& downloadManager = WebKit::NetworkProcess::singleton().downloadManager();
auto download = std::make_unique<WebKit::Download>(downloadManager, downloadID);
@@ -266,15 +274,18 @@
void NetworkSession::clearCredentials()
{
- ASSERT(m_dataTaskMap.isEmpty());
+ ASSERT(m_dataTaskMapWithCredentials.isEmpty());
+ ASSERT(m_dataTaskMapWithoutCredentials.isEmpty());
ASSERT(m_downloadMap.isEmpty());
m_sessionWithCredentialStorage = [NSURLSession sessionWithConfiguration:m_sessionWithCredentialStorage.get().configuration delegate:static_cast<id>(m_sessionDelegate.get()) delegateQueue:[NSOperationQueue mainQueue]];
}
-NetworkDataTask* NetworkSession::dataTaskForIdentifier(NetworkDataTask::TaskIdentifier taskIdentifier)
+NetworkDataTask* NetworkSession::dataTaskForIdentifier(NetworkDataTask::TaskIdentifier taskIdentifier, WebCore::StoredCredentials storedCredentials)
{
ASSERT(isMainThread());
- return m_dataTaskMap.get(taskIdentifier);
+ if (storedCredentials == WebCore::StoredCredentials::AllowStoredCredentials)
+ return m_dataTaskMapWithCredentials.get(taskIdentifier);
+ return m_dataTaskMapWithoutCredentials.get(taskIdentifier);
}
void NetworkSession::addDownloadID(NetworkDataTask::TaskIdentifier taskIdentifier, DownloadID downloadID)
