Skip to content

Commit

Permalink
Legacy EME WebKitMediaKeys createSessions unknown error
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=262874
rdar://116689080

Reviewed by Jer Noble.

If a media resource protected with the legacy EME API loads very quickly, the media player
in GPU process may try to generate a key request before the HTMLMediaElement in the web
process has signaled it is OK to continue after a key request. Have HTMLMediaElement
call player->setShouldContinueAfterKeyNeeded as soon as the media player is allocaged so
it will know before media data loading begins.

No new test added because the problem is extremely timing dependent and I was never able
to reproduce in a layout test.

* Source/WebCore/html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::createMediaPlayer): Call updateShouldContinueAfterNeedKey.

* Source/WebCore/platform/graphics/MediaPlayer.cpp:
(WebCore::MediaPlayer::loadWithNextMediaEngine): Call shouldWaitForLoadingOfResource.
(WebCore::MediaPlayer::setShouldContinueAfterKeyNeeded):

* Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
* Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
(WebCore::MediaPlayerPrivateAVFoundationObjC::MediaPlayerPrivateAVFoundationObjC): Add logging.
(WebCore::MediaPlayerPrivateAVFoundationObjC::shouldWaitForLoadingOfResource): Check
m_shouldContinueAfterKeyNeeded instead of calling up to the player.
(WebCore::MediaPlayerPrivateAVFoundationObjC::didCancelLoadingRequest): Add logging.
(WebCore::MediaPlayerPrivateAVFoundationObjC::didStopLoadingRequest): Ditto.
(WebCore::MediaPlayerPrivateAVFoundationObjC::setShouldContinueAfterKeyNeeded): Check
m_shouldContinueAfterKeyNeeded instead of calling up to the player.
(WebCore::MediaPlayerPrivateAVFoundationObjC::keyAdded): Add logging.

Canonical link: https://commits.webkit.org/275647@main
  • Loading branch information
eric-carlson committed Mar 4, 2024
1 parent 01e08c9 commit cac2e64
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 3 deletions.
3 changes: 3 additions & 0 deletions Source/WebCore/html/HTMLMediaElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7536,6 +7536,9 @@ void HTMLMediaElement::createMediaPlayer() WTF_IGNORES_THREAD_SAFETY_ANALYSIS
player->setPageIsVisible(!m_elementIsHidden, page ? page->sceneIdentifier() : ""_s);
player->setVisibleInViewport(isVisibleInViewport());
schedulePlaybackControlsManagerUpdate();
#if ENABLE(LEGACY_ENCRYPTED_MEDIA) && ENABLE(ENCRYPTED_MEDIA)
updateShouldContinueAfterNeedKey();
#endif

#if ENABLE(WEB_AUDIO)
if (m_audioSourceNode) {
Expand Down
6 changes: 5 additions & 1 deletion Source/WebCore/platform/graphics/MediaPlayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,10 @@ void MediaPlayer::loadWithNextMediaEngine(const MediaPlayerFactory* current)
m_private->startVideoFrameMetadataGathering();
if (m_processIdentity)
m_private->setResourceOwner(m_processIdentity);
#if ENABLE(LEGACY_ENCRYPTED_MEDIA) && ENABLE(ENCRYPTED_MEDIA)
if (m_shouldContinueAfterKeyNeeded)
m_private->setShouldContinueAfterKeyNeeded(m_shouldContinueAfterKeyNeeded);
#endif
m_private->prepareForPlayback(m_privateBrowsing, m_preload, m_preservesPitch, m_shouldPrepareToRender);
}
}
Expand Down Expand Up @@ -764,7 +768,7 @@ void MediaPlayer::attemptToDecryptWithInstance(CDMInstance& instance)
void MediaPlayer::setShouldContinueAfterKeyNeeded(bool should)
{
m_shouldContinueAfterKeyNeeded = should;
m_private->setShouldContinueAfterKeyNeeded(should);
m_private->setShouldContinueAfterKeyNeeded(m_shouldContinueAfterKeyNeeded);
}
#endif

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ class MediaPlayerPrivateAVFoundationObjC final : public MediaPlayerPrivateAVFoun

#if ENABLE(LEGACY_ENCRYPTED_MEDIA)
RetainPtr<AVAssetResourceLoadingRequest> takeRequestForKeyURI(const String&);
void setShouldContinueAfterKeyNeeded(bool) final;
#endif

void playerItemStatusDidChange(int);
Expand Down Expand Up @@ -435,6 +436,7 @@ class MediaPlayerPrivateAVFoundationObjC final : public MediaPlayerPrivateAVFoun

#if ENABLE(LEGACY_ENCRYPTED_MEDIA)
WeakPtr<CDMSessionAVFoundationObjC> m_session;
bool m_shouldContinueAfterKeyNeeded { false };
#endif

#if ENABLE(ENCRYPTED_MEDIA)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ static WallTime toSystemClockTime(NSDate *date)
, m_loaderDelegate(adoptNS([[WebCoreAVFLoaderDelegate alloc] initWithPlayer:*this]))
, m_cachedItemStatus(MediaPlayerAVPlayerItemStatusDoesNotExist)
{
ALWAYS_LOG(LOGIDENTIFIER);
m_muted = player->muted();
}

Expand Down Expand Up @@ -2149,7 +2150,7 @@ static void fulfillRequestWithKeyData(AVAssetResourceLoadingRequest *request, Ar
auto initData = SharedBuffer::create(Vector<uint8_t> { static_cast<uint8_t*>(initDataBuffer->data()), byteLength });
player->keyNeeded(initData);
#if ENABLE(ENCRYPTED_MEDIA)
if (!player->shouldContinueAfterKeyNeeded())
if (!m_shouldContinueAfterKeyNeeded)
return true;
#endif
#endif
Expand All @@ -2166,6 +2167,7 @@ static void fulfillRequestWithKeyData(AVAssetResourceLoadingRequest *request, Ar
player->initializationDataEncountered("skd"_s, m_keyID->tryCreateArrayBuffer());
setWaitingForKey(true);
#endif

m_keyURIToRequestMap.set(keyURI, avRequest);

return true;
Expand All @@ -2185,7 +2187,7 @@ static void fulfillRequestWithKeyData(AVAssetResourceLoadingRequest *request, Ar

player->keyNeeded(initData);

if (!player->shouldContinueAfterKeyNeeded())
if (!m_shouldContinueAfterKeyNeeded)
return false;

m_keyURIToRequestMap.set(keyID, avRequest);
Expand All @@ -2204,6 +2206,8 @@ static void fulfillRequestWithKeyData(AVAssetResourceLoadingRequest *request, Ar
void MediaPlayerPrivateAVFoundationObjC::didCancelLoadingRequest(AVAssetResourceLoadingRequest* avRequest)
{
ASSERT(isMainThread());

ALWAYS_LOG(LOGIDENTIFIER);
if (RefPtr resourceLoader = m_resourceLoaderMap.get((__bridge CFTypeRef)avRequest)) {
m_targetQueue->dispatch([resourceLoader = WTFMove(resourceLoader)] { resourceLoader->stopLoading();
});
Expand All @@ -2213,6 +2217,8 @@ static void fulfillRequestWithKeyData(AVAssetResourceLoadingRequest *request, Ar
void MediaPlayerPrivateAVFoundationObjC::didStopLoadingRequest(AVAssetResourceLoadingRequest *avRequest)
{
ASSERT(isMainThread());

ALWAYS_LOG(LOGIDENTIFIER);
if (RefPtr resourceLoader = m_resourceLoaderMap.take((__bridge CFTypeRef)avRequest))
m_targetQueue->dispatch([resourceLoader = WTFMove(resourceLoader)] { });
}
Expand Down Expand Up @@ -2892,6 +2898,12 @@ void determineChangedTracksFromNewTracksAndOldItems(MediaSelectionGroupAVFObjC*

#if ENABLE(LEGACY_ENCRYPTED_MEDIA)

void MediaPlayerPrivateAVFoundationObjC::setShouldContinueAfterKeyNeeded(bool shouldContinue)
{
ALWAYS_LOG(LOGIDENTIFIER, shouldContinue);
m_shouldContinueAfterKeyNeeded = shouldContinue;
}

RetainPtr<AVAssetResourceLoadingRequest> MediaPlayerPrivateAVFoundationObjC::takeRequestForKeyURI(const String& keyURI)
{
return m_keyURIToRequestMap.take(keyURI);
Expand All @@ -2905,6 +2917,7 @@ void determineChangedTracksFromNewTracksAndOldItems(MediaSelectionGroupAVFObjC*

Vector<String> fulfilledKeyIds;

ALWAYS_LOG(LOGIDENTIFIER);
for (auto& pair : m_keyURIToRequestMap) {
const String& keyId = pair.key;
const RetainPtr<AVAssetResourceLoadingRequest>& request = pair.value;
Expand Down

0 comments on commit cac2e64

Please sign in to comment.