Changeset 272750 in webkit

Timestamp:

Feb 11, 2021 2:05:29 PM (3 years ago)

Author:

Chris Dumez

Message:

[GPUP] <audio> won't load when URL ends with .php causing some tests to time out
​https://bugs.webkit.org/show_bug.cgi?id=221695

Reviewed by Eric Carlson.

Source/WebCore:

No new tests, covered by unskipped layout tests.

• html/HTMLMediaElement.cpp:

(WebCore::HTMLMediaElement::mediaPlayerEngineUpdated):
(WebCore::HTMLMediaElement::mediaPlayerDidInitializeMediaEngine):
We were calling HTMLMediaElement::audioSourceProvider() in mediaPlayerEngineUpdated() which
happens right after we've constructed the MediaPlayerPrivate but before we've called
load() on the MediaPlayerPrivate. The issue was that calling audioSourceProvider() would
initialize the AudioSourceProvider and end up sending the RemoteMediaPlayerProxy::CreateAudioSourceProvider
IPC to the GPUProcess. RemoteMediaPlayerProxy::createAudioSourceProvider() would return early
because m_player->audioSourceProvider() returns null. The reason m_player->audioSourceProvider()
returns null is because it's MediaPlayerPrivate is still a NullMediaPlayerPrivate, because
MediaPlayer::load() has not been called in the GPUProcess yet. For this reason, I moved the
audioSourceProvider() initialization from mediaPlayerEngineUpdated() to
mediaPlayerDidInitializeMediaEngine(). mediaPlayerDidInitializeMediaEngine() happens right
after we've called MediaPlayerPrivate::load() which will end up calling MediaPlayer::load()
in the GPUProcess.

• platform/graphics/MediaPlayer.cpp:

(WebCore::MediaPlayer::loadWithNextMediaEngine):
Pass an empty ContentType to MediaPlayerPrivate::load() when we did not have a content type
but guessed one based on the extension. This ends up getting passed to the MediaPlayer
in the GPUProcess and it is important it knows it does not have a content type so that
it can guess one from the extension and try the next media engine if it cannot find one.

LayoutTests:

Unskip tests that are no longer timing out.

• gpu-process/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/gpu-process/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/HTMLMediaElement.cpp (modified) (2 diffs)
• Source/WebCore/platform/graphics/MediaPlayer.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-02-11  Chris Dumez  <cdumez@apple.com>
+
+        [GPUP] <audio> won't load when URL ends with .php causing some tests to time out
+        https://bugs.webkit.org/show_bug.cgi?id=221695
+
+        Reviewed by Eric Carlson.
+
+        Unskip tests that are no longer timing out.
+
+        * gpu-process/TestExpectations:
+
 2021-02-11  Aditya Keerthi  <akeerthi@apple.com>
 

trunk/LayoutTests/gpu-process/TestExpectations

@@ -377 +377 @@
 imported/w3c/web-platform-tests/webaudio/the-audio-api/the-destinationnode-interface/destination.html [ Failure ]
 imported/w3c/web-platform-tests/webaudio/the-audio-api/the-mediaelementaudiosourcenode-interface/no-cors.https.html [ Failure ]
-
-# webkit.org/b/221695
-http/tests/security/webaudio-render-remote-audio-allowed-crossorigin-redirect.html [ Timeout ]
-http/tests/security/webaudio-render-remote-audio-allowed-crossorigin.html [ Timeout ]
-http/tests/security/webaudio-render-remote-audio-blocked-no-crossorigin-redirect.html [ Timeout ]
-http/tests/security/webaudio-render-remote-audio-blocked-no-crossorigin.html [ Timeout ]
 
 webgl/1.0.3/conformance/canvas/canvas-test.html [ Failure ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-02-11  Chris Dumez  <cdumez@apple.com>
+
+        [GPUP] <audio> won't load when URL ends with .php causing some tests to time out
+        https://bugs.webkit.org/show_bug.cgi?id=221695
+
+        Reviewed by Eric Carlson.
+
+        No new tests, covered by unskipped layout tests.
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::mediaPlayerEngineUpdated):
+        (WebCore::HTMLMediaElement::mediaPlayerDidInitializeMediaEngine):
+        We were calling HTMLMediaElement::audioSourceProvider() in mediaPlayerEngineUpdated() which
+        happens right after we've constructed the MediaPlayerPrivate but before we've called
+        load() on the MediaPlayerPrivate. The issue was that calling audioSourceProvider() would
+        initialize the AudioSourceProvider and end up sending the RemoteMediaPlayerProxy::CreateAudioSourceProvider
+        IPC to the GPUProcess. RemoteMediaPlayerProxy::createAudioSourceProvider() would return early
+        because m_player->audioSourceProvider() returns null. The reason m_player->audioSourceProvider()
+        returns null is because it's MediaPlayerPrivate is still a NullMediaPlayerPrivate, because
+        MediaPlayer::load() has not been called in the GPUProcess yet. For this reason, I moved the
+        audioSourceProvider() initialization from mediaPlayerEngineUpdated() to
+        mediaPlayerDidInitializeMediaEngine(). mediaPlayerDidInitializeMediaEngine() happens right
+        after we've called MediaPlayerPrivate::load() which will end up calling MediaPlayer::load()
+        in the GPUProcess.
+
+        * platform/graphics/MediaPlayer.cpp:
+        (WebCore::MediaPlayer::loadWithNextMediaEngine):
+        Pass an empty ContentType to MediaPlayerPrivate::load() when we did not have a content type
+        but guessed one based on the extension. This ends up getting passed to the MediaPlayer
+        in the GPUProcess and it is important it knows it does not have a content type so that
+        it can guess one from the extension and try the next media engine if it cannot find one.
+
 2021-02-11  Aditya Keerthi  <akeerthi@apple.com>
 

trunk/Source/WebCore/html/HTMLMediaElement.cpp

@@ -5045 +5045 @@
 #endif
 
-#if ENABLE(WEB_AUDIO)
-    if (m_audioSourceNode) {
-        if (auto* provider = audioSourceProvider())
-            provider->setClient(m_audioSourceNode);
-    }
-#endif
-
     m_havePreparedToPlay = false;
 
@@ -5071 +5064 @@
     ASSERT(isMainThread());
 #if ENABLE(WEB_AUDIO)
-    if (m_audioSourceNode)
+    if (m_audioSourceNode) {
+        if (auto* provider = audioSourceProvider())
+            provider->setClient(m_audioSourceNode);
+
         m_audioSourceNode->processLock().unlock();
+    }
 #endif
 }

trunk/Source/WebCore/platform/graphics/MediaPlayer.cpp

@@ -578 +578 @@
 #if ENABLE(MEDIA_SOURCE)
         if (m_mediaSource)
-            m_private->load(m_url, m_contentType, m_mediaSource.get());
+            m_private->load(m_url, m_contentMIMETypeWasInferredFromExtension ? ContentType() : m_contentType, m_mediaSource.get());
         else
 #endif
@@ -586 +586 @@
         else
 #endif
-        m_private->load(m_url, m_contentType, m_keySystem);
+        m_private->load(m_url, m_contentMIMETypeWasInferredFromExtension ? ContentType() : m_contentType, m_keySystem);
     } else {
         m_private = makeUnique<NullMediaPlayerPrivate>(this);