Modified: branches/safari-605-branch/Source/WebCore/ChangeLog (228910 => 228911)
--- branches/safari-605-branch/Source/WebCore/ChangeLog 2018-02-22 00:37:29 UTC (rev 228910)
+++ branches/safari-605-branch/Source/WebCore/ChangeLog 2018-02-22 00:37:32 UTC (rev 228911)
@@ -1,5 +1,50 @@
2018-02-21 Jason Marcell <[email protected]>
+ Cherry-pick r228903. rdar://problem/37765339
+
+ 2018-02-21 Chris Dumez <[email protected]>
+
+ Regression(r228708): Crash under WebCore::MediaResource::responseReceived(WebCore::CachedResource&, WebCore::ResourceResponse const&)
+ https://bugs.webkit.org/show_bug.cgi?id=183018
+ <rdar://problem/37754154>
+
+ Reviewed by Eric Carlson.
+
+ The fix at r228708 was trying to address the fact that avplayer sometimes
+ deallocates WebCoreNSURLSessionDataTask objects on a non-main thread, which
+ was not safe because its _resource data member needs to be deallocated on
+ the main thread.
+
+ The issue is that r228708 caused _resource to outlive its WebCoreNSURLSessionDataTask.
+ This is an issue because _resource has a client data member (of type WebCoreNSURLSessionDataTaskClient)
+ which has a raw pointer to the WebCoreNSURLSessionDataTask. This means that the main thread could
+ call methods like responseReceived() on the resource, which would call responseReceived() on the
+ client, which would try to call [WebCoreNSURLSessionDataTask receivedResponse:] with an invalid
+ m_task pointer.
+
+ To address the issue, I introduced a clearTask() method on WebCoreNSURLSessionDataTaskClient, which
+ gets called from a non-main thread to clear the client's m_task pointer when the task is destroyed
+ on a non-main thread. So that this is safe, every time the client tries to use m_task, we now
+ acquire a lock for thread-safety and do a null-check on m_task.
+
+ No new tests, no known reproduction case.
+
+ * platform/graphics/PlatformMediaResourceLoader.h:
+ (WebCore::PlatformMediaResource::client):
+ * platform/network/cocoa/WebCoreNSURLSession.mm:
+ (WebCore::WebCoreNSURLSessionDataTaskClient::clearTask):
+ (WebCore::WebCoreNSURLSessionDataTaskClient::dataSent):
+ (WebCore::WebCoreNSURLSessionDataTaskClient::responseReceived):
+ (WebCore::WebCoreNSURLSessionDataTaskClient::shouldCacheResponse):
+ (WebCore::WebCoreNSURLSessionDataTaskClient::dataReceived):
+ (WebCore::WebCoreNSURLSessionDataTaskClient::redirectReceived):
+ (WebCore::WebCoreNSURLSessionDataTaskClient::accessControlCheckFailed):
+ (WebCore::WebCoreNSURLSessionDataTaskClient::loadFailed):
+ (WebCore::WebCoreNSURLSessionDataTaskClient::loadFinished):
+ (-[WebCoreNSURLSessionDataTask dealloc]):
+
+2018-02-21 Jason Marcell <[email protected]>
+
Cherry-pick r228851. rdar://problem/37734494
2018-02-20 Chris Dumez <[email protected]>
Modified: branches/safari-605-branch/Source/WebCore/platform/graphics/PlatformMediaResourceLoader.h (228910 => 228911)
--- branches/safari-605-branch/Source/WebCore/platform/graphics/PlatformMediaResourceLoader.h 2018-02-22 00:37:29 UTC (rev 228910)
+++ branches/safari-605-branch/Source/WebCore/platform/graphics/PlatformMediaResourceLoader.h 2018-02-22 00:37:32 UTC (rev 228911)
@@ -80,6 +80,7 @@
virtual bool didPassAccessControlCheck() const { return false; }
void setClient(std::unique_ptr<PlatformMediaResourceClient>&& client) { m_client = WTFMove(client); }
+ PlatformMediaResourceClient* client() { return m_client.get(); }
protected:
std::unique_ptr<PlatformMediaResourceClient> m_client;
Modified: branches/safari-605-branch/Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.mm (228910 => 228911)
--- branches/safari-605-branch/Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.mm 2018-02-22 00:37:29 UTC (rev 228910)
+++ branches/safari-605-branch/Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.mm 2018-02-22 00:37:32 UTC (rev 228911)
@@ -354,6 +354,8 @@
{
}
+ void clearTask();
+
void responseReceived(PlatformMediaResource&, const ResourceResponse&) override;
void redirectReceived(PlatformMediaResource&, ResourceRequest&&, const ResourceResponse&, CompletionHandler<void(ResourceRequest&&)>&&) override;
bool shouldCacheResponse(PlatformMediaResource&, const ResourceResponse&) override;
@@ -364,31 +366,58 @@
void loadFinished(PlatformMediaResource&) override;
private:
+ Lock m_taskLock;
WebCoreNSURLSessionDataTask *m_task;
};
+void WebCoreNSURLSessionDataTaskClient::clearTask()
+{
+ LockHolder locker(m_taskLock);
+ m_task = nullptr;
+}
+
void WebCoreNSURLSessionDataTaskClient::dataSent(PlatformMediaResource& resource, unsigned long long bytesSent, unsigned long long totalBytesToBeSent)
{
+ LockHolder locker(m_taskLock);
+ if (!m_task)
+ return;
+
[m_task resource:resource sentBytes:bytesSent totalBytesToBeSent:totalBytesToBeSent];
}
void WebCoreNSURLSessionDataTaskClient::responseReceived(PlatformMediaResource& resource, const ResourceResponse& response)
{
+ LockHolder locker(m_taskLock);
+ if (!m_task)
+ return;
+
[m_task resource:resource receivedResponse:response];
}
bool WebCoreNSURLSessionDataTaskClient::shouldCacheResponse(PlatformMediaResource& resource, const ResourceResponse& response)
{
+ LockHolder locker(m_taskLock);
+ if (!m_task)
+ return false;
+
return [m_task resource:resource shouldCacheResponse:response];
}
void WebCoreNSURLSessionDataTaskClient::dataReceived(PlatformMediaResource& resource, const char* data, int length)
{
+ LockHolder locker(m_taskLock);
+ if (!m_task)
+ return;
+
[m_task resource:resource receivedData:data length:length];
}
void WebCoreNSURLSessionDataTaskClient::redirectReceived(PlatformMediaResource& resource, ResourceRequest&& request, const ResourceResponse& response, CompletionHandler<void(ResourceRequest&&)>&& completionHandler)
{
+ LockHolder locker(m_taskLock);
+ if (!m_task)
+ return;
+
[m_task resource:resource receivedRedirect:response request:WTFMove(request) completionHandler: [completionHandler = WTFMove(completionHandler)] (auto&& request) {
ASSERT(isMainThread());
completionHandler(WTFMove(request));
@@ -397,16 +426,28 @@
void WebCoreNSURLSessionDataTaskClient::accessControlCheckFailed(PlatformMediaResource& resource, const ResourceError& error)
{
+ LockHolder locker(m_taskLock);
+ if (!m_task)
+ return;
+
[m_task resource:resource accessControlCheckFailedWithError:error];
}
void WebCoreNSURLSessionDataTaskClient::loadFailed(PlatformMediaResource& resource, const ResourceError& error)
{
+ LockHolder locker(m_taskLock);
+ if (!m_task)
+ return;
+
[m_task resource:resource loadFailedWithError:error];
}
void WebCoreNSURLSessionDataTaskClient::loadFinished(PlatformMediaResource& resource)
{
+ LockHolder locker(m_taskLock);
+ if (!m_task)
+ return;
+
[m_task resourceFinished:resource];
}
@@ -539,7 +580,13 @@
[_currentRequest release];
[_error release];
[_taskDescription release];
- callOnMainThread([resource = WTFMove(_resource)] { });
+
+ if (!isMainThread() && _resource) {
+ if (auto* client = _resource->client())
+ static_cast<WebCoreNSURLSessionDataTaskClient*>(client)->clearTask();
+ callOnMainThread([resource = WTFMove(_resource)] { });
+ }
+
[super dealloc];
}
