Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perform media networking operations off the main thread #719

Merged
merged 0 commits into from May 28, 2022

Conversation

jyavenard
Copy link
Member

@jyavenard jyavenard commented May 18, 2022

be35d3c

Perform media networking operations off the main thread
https://bugs.webkit.org/show_bug.cgi?id=235353
rdar://84517825

Reviewed by Eric Carlson.

We run the media's networking operation and IPC service into a new dedicated WorkQueue.
Not all operation on a MediaResource can be made thread-agnostic, and the creation and shutdown of the MediaResource must still occur on the main thread due to the current architecture which forces more task dispatches than ideal.

However, all other operations will now run on a WorkQueue when the GPU Process is enabled. For WK1 we continue to use the main thread.
We use the IPC::Connection::WorkQueueMessageReceiver characteristics to proxy the network operations from the content process main thread to the GPU Process MediaResourceLoader's WorkQueue.

Some changes were required in data structures such as ResourceResponseBase, where we must use Strings over AtomStrings. A WorkQueue while guaranteeing that all tasks runs sequentially, will use different threads making the use of AtomStrings impossible.

RemoteMediaResourceManager now holds a strong reference to the RemoteMediaResource, to clear that reference and to make the call more explicit we rename PlatformMediaResource::stop into PlatformMediaResource::shutdown()
A call to shutdown() is equivalent to the previous call to stop() followed by setClient(nullptr).

Side-by fixes: various thread-safety fixes in WebCoreNSURLSession, access to some members weren't thread-safe.

To make the code flow easier to follow, we use the new assertIsCurrent(WorkQueue&) extensively. Most methods will now assert that it runs on the thread it's supposed to.

* Source/WebCore/Modules/fetch/FetchResponse.cpp:
(WebCore::FetchResponse::create):
* Source/WebCore/Modules/websockets/WebSocketHandshake.cpp:
(WebCore::WebSocketHandshake::readServerHandshake):
(WebCore::WebSocketHandshake::readStatusLine):
* Source/WebCore/Modules/websockets/WebSocketHandshake.h:
* Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp:
(WebCore::InspectorNetworkAgent::interceptWithResponse):
(WebCore::InspectorNetworkAgent::interceptRequestWithResponse):
* Source/WebCore/loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::setResponseMIMEType):
* Source/WebCore/loader/DocumentLoader.h:
* Source/WebCore/loader/MediaResourceLoader.cpp:
(WebCore::MediaResource::MediaResource):
(WebCore::MediaResource::~MediaResource):
(WebCore::MediaResource::shutdown):
(WebCore::MediaResource::responseReceived):
(WebCore::MediaResource::shouldCacheResponse):
(WebCore::MediaResource::redirectReceived):
(WebCore::MediaResource::dataSent):
(WebCore::MediaResource::dataReceived):
(WebCore::MediaResource::notifyFinished):
(WebCore::MediaResource::ensureShutdown):
(WebCore::MediaResource::stop): Deleted.
* Source/WebCore/loader/MediaResourceLoader.h:
* Source/WebCore/loader/cache/CachedRawResourceClient.h:
* Source/WebCore/platform/graphics/PlatformMediaResourceLoader.h:
(WebCore::PlatformMediaResourceLoader::targetQueue):
(WebCore::PlatformMediaResource::shutdown):
(WebCore::PlatformMediaResource::didPassAccessControlCheck const):
(WebCore::PlatformMediaResource::setClient):
(WebCore::PlatformMediaResource::client const):
(WebCore::PlatformMediaResource::stop): Deleted.
(WebCore::PlatformMediaResource::client): Deleted.
* Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
* Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
(WebCore::MediaPlayerPrivateAVFoundationObjC::clearMediaCache):
(WebCore::MediaPlayerPrivateAVFoundationObjC::~MediaPlayerPrivateAVFoundationObjC):
(WebCore::MediaPlayerPrivateAVFoundationObjC::createAVAssetForURL):
(WebCore::MediaPlayerPrivateAVFoundationObjC::shouldWaitForLoadingOfResource):
(WebCore::MediaPlayerPrivateAVFoundationObjC::didCancelLoadingRequest):
(WebCore::MediaPlayerPrivateAVFoundationObjC::didStopLoadingRequest):
(WebCore::MediaPlayerPrivateAVFoundationObjC::didPassCORSAccessCheck const):
(WebCore::MediaPlayerPrivateAVFoundationObjC::wouldTaintOrigin const):
* Source/WebCore/platform/graphics/avfoundation/objc/WebCoreAVFResourceLoader.h:
* Source/WebCore/platform/graphics/avfoundation/objc/WebCoreAVFResourceLoader.mm:
(WebCore::CachedResourceMediaLoader::responseReceived):
(WebCore::PlatformResourceMediaLoader::create):
(WebCore::PlatformResourceMediaLoader::PlatformResourceMediaLoader):
(WebCore::PlatformResourceMediaLoader::stop):
(WebCore::PlatformResourceMediaLoader::responseReceived):
(WebCore::PlatformResourceMediaLoader::loadFailed):
(WebCore::PlatformResourceMediaLoader::loadFinished):
(WebCore::PlatformResourceMediaLoader::dataReceived):
(WebCore::DataURLResourceMediaLoader::DataURLResourceMediaLoader):
(WebCore::WebCoreAVFResourceLoader::create):
(WebCore::WebCoreAVFResourceLoader::WebCoreAVFResourceLoader):
(WebCore::WebCoreAVFResourceLoader::~WebCoreAVFResourceLoader):
(WebCore::WebCoreAVFResourceLoader::startLoading):
(WebCore::WebCoreAVFResourceLoader::stopLoading):
(WebCore::WebCoreAVFResourceLoader::responseReceived):
(WebCore::WebCoreAVFResourceLoader::loadFailed):
(WebCore::WebCoreAVFResourceLoader::loadFinished):
(WebCore::WebCoreAVFResourceLoader::newDataStoredInSharedBuffer):
(WebCore::WebCoreAVFResourceLoader::invalidate): Deleted.
* Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
(stopLoaderIfNeeded):
(webKitWebSrcUnLock):
(webKitSrcWouldTaintOrigin):
* Source/WebCore/platform/network/ResourceResponseBase.cpp:
(WebCore::ResourceResponseBase::crossThreadData const):
(WebCore::ResourceResponseBase::fromCrossThreadData):
(WebCore::ResourceResponseBase::mimeType const):
(WebCore::ResourceResponseBase::setMimeType):
(WebCore::ResourceResponseBase::textEncodingName const):
(WebCore::ResourceResponseBase::setTextEncodingName):
(WebCore::ResourceResponseBase::httpStatusText const):
(WebCore::ResourceResponseBase::setHTTPStatusText):
(WebCore::ResourceResponseBase::httpVersion const):
(WebCore::ResourceResponseBase::setHTTPVersion):
* Source/WebCore/platform/network/ResourceResponseBase.h:
(WebCore::ResourceResponseBase::decode):
* Source/WebCore/platform/network/cf/ResourceResponseCFNet.cpp:
(WebCore::ResourceResponse::cfURLResponse const):
(WebCore::ResourceResponse::platformLazyInit):
* Source/WebCore/platform/network/cocoa/RangeResponseGenerator.h:
(WebCore::RangeResponseGenerator::create):
* Source/WebCore/platform/network/cocoa/RangeResponseGenerator.mm:
(WebCore::RangeResponseGenerator::Data::Data):
(WebCore::RangeResponseGenerator::Data::~Data):
(WebCore::RangeResponseGenerator::Data::shutdownResource):
(WebCore::RangeResponseGenerator::RangeResponseGenerator):
(WebCore::synthesizedResponseForRange):
(WebCore::RangeResponseGenerator::removeTask):
(WebCore::RangeResponseGenerator::giveResponseToTaskIfBytesInRangeReceived):
(WebCore::RangeResponseGenerator::expectedContentLengthFromData):
(WebCore::RangeResponseGenerator::giveResponseToTasksWithFinishedRanges):
(WebCore::RangeResponseGenerator::willHandleRequest):
(WebCore::RangeResponseGenerator::willSynthesizeRangeResponses):
(WebCore::RangeResponseGenerator::~RangeResponseGenerator): Deleted.
* Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.h:
* Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.mm:
(-[WebCoreNSURLSessionTaskTransactionMetrics _initWithMetrics:onTarget:]):
(-[WebCoreNSURLSessionTaskTransactionMetrics dealloc]):
(-[WebCoreNSURLSessionTaskMetrics _initWithMetrics:onTarget:]):
(-[WebCoreNSURLSessionTaskMetrics dealloc]):
(-[WebCoreNSURLSessionTaskMetrics transactionMetrics]):
(-[WebCoreNSURLSession initWithResourceLoader:delegate:delegateQueue:]):
(-[WebCoreNSURLSession dealloc]):
(-[WebCoreNSURLSession taskCompleted:]):
(-[WebCoreNSURLSession addDelegateOperation:]):
(-[WebCoreNSURLSession task:didReceiveCORSAccessCheckResult:]):
(-[WebCoreNSURLSession task:didReceiveResponseFromOrigin:]):
(-[WebCoreNSURLSession rangeResponseGenerator]):
(-[WebCoreNSURLSession didPassCORSAccessChecks]):
(-[WebCoreNSURLSession finishTasksAndInvalidate]):
(-[WebCoreNSURLSession dataTaskWithRequest:]):
(-[WebCoreNSURLSession sendH2Ping:pongHandler:]):
(WebCore::WebCoreNSURLSessionDataTaskClient::WebCoreNSURLSessionDataTaskClient):
(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 initWithSession:identifier:request:targetQueue:]):
(-[WebCoreNSURLSessionDataTask _cancel:]):
(-[WebCoreNSURLSessionDataTask error]):
(-[WebCoreNSURLSessionDataTask session]):
(-[WebCoreNSURLSessionDataTask setSession:]):
(-[WebCoreNSURLSessionDataTask resource]):
(-[WebCoreNSURLSessionDataTask setResource:]):
(-[WebCoreNSURLSessionDataTask cancel]):
(-[WebCoreNSURLSessionDataTask suspend]):
(-[WebCoreNSURLSessionDataTask resume]):
(-[WebCoreNSURLSessionDataTask dealloc]):
(-[WebCoreNSURLSessionDataTask resource:sentBytes:totalBytesToBeSent:]):
(-[WebCoreNSURLSessionDataTask resource:receivedResponse:completionHandler:]):
(-[WebCoreNSURLSessionDataTask resource:shouldCacheResponse:]):
(-[WebCoreNSURLSessionDataTask resource:receivedData:]):
(-[WebCoreNSURLSessionDataTask resource:receivedRedirect:request:completionHandler:]):
(-[WebCoreNSURLSessionDataTask _resource:loadFinishedWithError:metrics:]):
(-[WebCoreNSURLSessionDataTask resource:accessControlCheckFailedWithError:]):
(-[WebCoreNSURLSessionDataTask resource:loadFailedWithError:]):
(-[WebCoreNSURLSessionDataTask resourceFinished:metrics:]):
(-[WebCoreNSURLSessionTaskTransactionMetrics _initWithMetrics:]): Deleted.
(-[WebCoreNSURLSessionTaskMetrics _initWithMetrics:]): Deleted.
(-[WebCoreNSURLSessionDataTask initWithSession:identifier:request:]): Deleted.
(-[WebCoreNSURLSessionDataTask _restart]): Deleted.
(-[WebCoreNSURLSessionDataTask _cancel]): Deleted.
* Source/WebCore/platform/network/curl/CurlCacheEntry.cpp:
(WebCore::CurlCacheEntry::setResponseFromCachedHeaders):
* Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:
(WebCore::ResourceHandle::handleDataURL):
* Source/WebCore/platform/network/curl/ResourceResponseCurl.cpp:
(WebCore::ResourceResponse::ResourceResponse):
(WebCore::ResourceResponse::setStatusLine):
* Source/WebCore/platform/network/soup/ResourceResponseSoup.cpp:
(WebCore::ResourceResponse::ResourceResponse):
* Source/WebKit/GPUProcess/GPUConnectionToWebProcess.cpp:
(WebKit::GPUConnectionToWebProcess::didClose):
(WebKit::GPUConnectionToWebProcess::remoteMediaResourceManager):
* Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp:
(WebKit::RemoteMediaPlayerProxy::requestResource):
* Source/WebKit/GPUProcess/media/RemoteMediaResource.cpp:
(WebKit::RemoteMediaResource::RemoteMediaResource):
(WebKit::RemoteMediaResource::~RemoteMediaResource):
(WebKit::RemoteMediaResource::shutdown):
(WebKit::RemoteMediaResource::didPassAccessControlCheck const):
(WebKit::RemoteMediaResource::responseReceived):
(WebKit::RemoteMediaResource::redirectReceived):
(WebKit::RemoteMediaResource::dataSent):
(WebKit::RemoteMediaResource::dataReceived):
(WebKit::RemoteMediaResource::accessControlCheckFailed):
(WebKit::RemoteMediaResource::loadFailed):
(WebKit::RemoteMediaResource::loadFinished):
(WebKit::RemoteMediaResource::stop): Deleted.
* Source/WebKit/GPUProcess/media/RemoteMediaResource.h:
* Source/WebKit/GPUProcess/media/RemoteMediaResourceLoader.cpp:
(WebKit::RemoteMediaResourceLoader::RemoteMediaResourceLoader):
(WebKit::RemoteMediaResourceLoader::requestResource):
(WebKit::RemoteMediaResourceLoader::sendH2Ping):
* Source/WebKit/GPUProcess/media/RemoteMediaResourceLoader.h:
* Source/WebKit/GPUProcess/media/RemoteMediaResourceManager.cpp:
(WebKit::RemoteMediaResourceManager::~RemoteMediaResourceManager):
(WebKit::RemoteMediaResourceManager::stopListeningForIPC):
(WebKit::RemoteMediaResourceManager::initializeConnection):
(WebKit::RemoteMediaResourceManager::addMediaResource):
(WebKit::RemoteMediaResourceManager::removeMediaResource):
(WebKit::RemoteMediaResourceManager::resourceForIdAndReady):
(WebKit::RemoteMediaResourceManager::responseReceived):
(WebKit::RemoteMediaResourceManager::redirectReceived):
(WebKit::RemoteMediaResourceManager::dataSent):
(WebKit::RemoteMediaResourceManager::dataReceived):
(WebKit::RemoteMediaResourceManager::accessControlCheckFailed):
(WebKit::RemoteMediaResourceManager::loadFailed):
(WebKit::RemoteMediaResourceManager::loadFinished):
* Source/WebKit/GPUProcess/media/RemoteMediaResourceManager.h:
* Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::didReceiveResponse):
* Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp:
(WebKit::ServiceWorkerFetchTask::processResponse):
* Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp:
(WebKit::NetworkDataTaskSoup::didGetFileInfo):
* Source/WebKit/Shared/API/glib/WebKitURIResponse.cpp:
(webkit_uri_response_get_mime_type):
* Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:
(webkitURISchemeRequestReadCallback):
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
(WebKit::MediaPlayerPrivateRemote::~MediaPlayerPrivateRemote):
(WebKit::MediaPlayerPrivateRemote::removeResource):
* Source/WebKit/WebProcess/Network/WebResourceLoader.cpp:
(WebKit::WebResourceLoader::didReceiveData):
* Source/WebKit/WebProcess/Network/WebResourceLoader.h:

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

@jyavenard jyavenard self-assigned this May 18, 2022
@jyavenard jyavenard marked this pull request as draft May 18, 2022 10:29
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label May 18, 2022
@grorg
Copy link
Contributor

grorg commented May 18, 2022

Could you give the PR a better title? 😀

@jyavenard jyavenard removed the merging-blocked Applied to prevent a change from being merged label May 18, 2022
@jyavenard jyavenard changed the title Need a short description (OOPS!). fix various builds May 18, 2022
@jyavenard
Copy link
Member Author

Could you give the PR a better title? 😀

I've made it a draft :) shouldn't be visible !

@jyavenard jyavenard changed the title fix various builds Run media networking operation off the main thread May 18, 2022
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label May 18, 2022
@jyavenard jyavenard added Media Bugs related to the HTML 5 Media elements. Other and removed merging-blocked Applied to prevent a change from being merged Media Bugs related to the HTML 5 Media elements. Other labels May 18, 2022
@jyavenard jyavenard added Media Bugs related to the HTML 5 Media elements. Other labels May 18, 2022
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label May 18, 2022
Source/WebCore/dom/ContainerNode.cpp Outdated Show resolved Hide resolved
Source/WebCore/dom/ContainerNode.cpp Outdated Show resolved Hide resolved
Source/WebCore/dom/Node.cpp Outdated Show resolved Hide resolved
@jyavenard jyavenard removed merging-blocked Applied to prevent a change from being merged Media Bugs related to the HTML 5 Media elements. Other labels May 19, 2022
@jyavenard jyavenard changed the title Run media networking operation off the main thread Properly set NSURLSession to NSURLSessionTaskStateRunning when we will use the rangeResponseGenerator. May 19, 2022
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label May 19, 2022
@jyavenard jyavenard removed the merging-blocked Applied to prevent a change from being merged label May 20, 2022
@jyavenard jyavenard changed the title Properly set NSURLSession to NSURLSessionTaskStateRunning when we will use the rangeResponseGenerator. Make RemoteMediaResourceManager keep a strong reference to the MediaResource and enforce shutting down the mediaresource before deleting it (to break any potential cycle) May 20, 2022
@jyavenard jyavenard added Media Bugs related to the HTML 5 Media elements. Other labels May 24, 2022
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label May 24, 2022
@jyavenard jyavenard removed merging-blocked Applied to prevent a change from being merged Media Bugs related to the HTML 5 Media elements. Other labels May 26, 2022
@jyavenard jyavenard added Media Bugs related to the HTML 5 Media elements. Other labels May 26, 2022
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label May 26, 2022
@jyavenard jyavenard removed merging-blocked Applied to prevent a change from being merged Media Bugs related to the HTML 5 Media elements. Other labels May 27, 2022
@jyavenard jyavenard added Media Bugs related to the HTML 5 Media elements. Other labels May 27, 2022
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label May 27, 2022
@jyavenard
Copy link
Member Author

compositing/video/video-border-radius.html is failing 90% of the time, even without those changes with WK1

@jyavenard jyavenard removed merging-blocked Applied to prevent a change from being merged Media Bugs related to the HTML 5 Media elements. Other labels May 28, 2022
@jyavenard jyavenard added the merge-queue Applied to send a pull request to merge-queue label May 28, 2022
@webkit-early-warning-system webkit-early-warning-system merged commit be35d3c into WebKit:main May 28, 2022
@webkit-early-warning-system
Copy link
Collaborator

Committed r294999 (251097@main): https://commits.webkit.org/251097@main

Reviewed commits have been landed. Closing PR #719 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system removed the merge-queue Applied to send a pull request to merge-queue label May 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants