- Revision
- 272414
- Author
- [email protected]
- Date
- 2021-02-05 07:50:53 -0800 (Fri, 05 Feb 2021)
Log Message
[GPUProcess][iOS] Audio is lost after media playback recovers from the GPUProcess crash
https://bugs.webkit.org/show_bug.cgi?id=221305
<rdar://problem/73904864>
Reviewed by Eric Carlson.
Source/WebCore:
The issue was that when the GPUProcess crashed while in the middle of media playback, we would
fail to remove the old/invalid AudioTrack and then append the new/valid AudioTrack upon
recovery. We would end up using the wrong old AudioTrack instead of the new track as a result.
To address the issue, I have refactored the code so that the "reload and resume" logic now
resides in HTMLMediaElement instead of MediaPlayer. This makes sure that the HTMLMediaElement
object's state stays good. In particular, when HTMLMediaElement::prepareToLoad() is called,
it calls forgetResourceSpecificTracks() to drop the old/invalid AudioTracks.
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::mediaPlayerReloadAndResumePlaybackIfNeeded):
* html/HTMLMediaElement.h:
* platform/graphics/MediaPlayer.cpp:
(WebCore::MediaPlayer::reloadAndResumePlaybackIfNeeded):
* platform/graphics/MediaPlayer.h:
(WebCore::MediaPlayerClient::mediaPlayerUserAgent const):
(WebCore::MediaPlayerClient::mediaPlayerReloadAndResumePlaybackIfNeeded):
Source/WebKit:
Fix issue where we would sometimes lose the video after recovering from a GPUProcess
crash. The issue is that RemoteMediaPlayerProxy::setVideoInlineSizeFenced() may get
called *before* RemoteMediaPlayerProxy::mediaPlayerFirstVideoFrameAvailable(). As a
result, we would not have a root layer yet and we would fail to set the video
dimensions and they would remain at 0x0.
* GPUProcess/media/RemoteMediaPlayerProxy.cpp:
* GPUProcess/media/RemoteMediaPlayerProxy.h:
* GPUProcess/media/cocoa/RemoteMediaPlayerProxyCocoa.mm:
(WebKit::setVideoInlineSizeIfPossible):
(WebKit::RemoteMediaPlayerProxy::mediaPlayerFirstVideoFrameAvailable):
(WebKit::RemoteMediaPlayerProxy::setVideoInlineSizeFenced):
Tools:
Unskip API test checks on iOS now that they are passing.
* TestWebKitAPI/Tests/WebKitCocoa/GPUProcess.mm:
(TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (272413 => 272414)
--- trunk/Source/WebCore/ChangeLog 2021-02-05 15:11:44 UTC (rev 272413)
+++ trunk/Source/WebCore/ChangeLog 2021-02-05 15:50:53 UTC (rev 272414)
@@ -1,3 +1,29 @@
+2021-02-05 Chris Dumez <[email protected]>
+
+ [GPUProcess][iOS] Audio is lost after media playback recovers from the GPUProcess crash
+ https://bugs.webkit.org/show_bug.cgi?id=221305
+ <rdar://problem/73904864>
+
+ Reviewed by Eric Carlson.
+
+ The issue was that when the GPUProcess crashed while in the middle of media playback, we would
+ fail to remove the old/invalid AudioTrack and then append the new/valid AudioTrack upon
+ recovery. We would end up using the wrong old AudioTrack instead of the new track as a result.
+
+ To address the issue, I have refactored the code so that the "reload and resume" logic now
+ resides in HTMLMediaElement instead of MediaPlayer. This makes sure that the HTMLMediaElement
+ object's state stays good. In particular, when HTMLMediaElement::prepareToLoad() is called,
+ it calls forgetResourceSpecificTracks() to drop the old/invalid AudioTracks.
+
+ * html/HTMLMediaElement.cpp:
+ (WebCore::HTMLMediaElement::mediaPlayerReloadAndResumePlaybackIfNeeded):
+ * html/HTMLMediaElement.h:
+ * platform/graphics/MediaPlayer.cpp:
+ (WebCore::MediaPlayer::reloadAndResumePlaybackIfNeeded):
+ * platform/graphics/MediaPlayer.h:
+ (WebCore::MediaPlayerClient::mediaPlayerUserAgent const):
+ (WebCore::MediaPlayerClient::mediaPlayerReloadAndResumePlaybackIfNeeded):
+
2021-02-05 Zalan Bujtas <[email protected]>
[LFC][IFC] Incorrect last potential wrap position when inline box is present
Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (272413 => 272414)
--- trunk/Source/WebCore/html/HTMLMediaElement.cpp 2021-02-05 15:11:44 UTC (rev 272413)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp 2021-02-05 15:50:53 UTC (rev 272414)
@@ -1198,6 +1198,31 @@
configureMediaControls();
}
+void HTMLMediaElement::mediaPlayerReloadAndResumePlaybackIfNeeded()
+{
+ auto previousMediaTime = m_cachedTime;
+ bool wasPaused = paused();
+
+ load();
+
+ // FIXME: It would be even better if we could resume in full screen mode, but, for now, exiting full screen makes the video rendering work.
+ if (m_videoFullscreenMode != VideoFullscreenModeNone)
+ exitFullscreen();
+
+ if (previousMediaTime) {
+ m_resourceSelectionTaskQueue.enqueueTask([this, previousMediaTime] {
+ if (m_player)
+ m_player->seekWhenPossible(previousMediaTime);
+ });
+ }
+
+ if (!wasPaused) {
+ m_resourceSelectionTaskQueue.enqueueTask([this] {
+ playInternal();
+ });
+ }
+}
+
void HTMLMediaElement::selectMediaResource()
{
// https://www.w3.org/TR/2016/REC-html51-20161101/semantics-embedded-content.html#resource-selection-algorithm
@@ -6884,16 +6909,6 @@
#endif
-void HTMLMediaElement::mediaPlayerEnterFullscreen()
-{
- enterFullscreen();
-}
-
-void HTMLMediaElement::mediaPlayerExitFullscreen()
-{
- exitFullscreen();
-}
-
bool HTMLMediaElement::mediaPlayerIsFullscreen() const
{
return isFullscreen();
Modified: trunk/Source/WebCore/html/HTMLMediaElement.h (272413 => 272414)
--- trunk/Source/WebCore/html/HTMLMediaElement.h 2021-02-05 15:11:44 UTC (rev 272413)
+++ trunk/Source/WebCore/html/HTMLMediaElement.h 2021-02-05 15:50:53 UTC (rev 272414)
@@ -660,6 +660,7 @@
void mediaPlayerEngineUpdated() final;
void mediaPlayerWillInitializeMediaEngine() final;
void mediaPlayerDidInitializeMediaEngine() final;
+ void mediaPlayerReloadAndResumePlaybackIfNeeded() final;
void scheduleMediaEngineWasUpdated();
void mediaEngineWasUpdated();
@@ -699,8 +700,6 @@
String mediaPlayerReferrer() const override;
String mediaPlayerUserAgent() const override;
- void mediaPlayerEnterFullscreen() override;
- void mediaPlayerExitFullscreen() override;
bool mediaPlayerIsFullscreen() const override;
bool mediaPlayerIsFullscreenPermitted() const override;
bool mediaPlayerIsVideo() const override;
Modified: trunk/Source/WebCore/platform/graphics/MediaPlayer.cpp (272413 => 272414)
--- trunk/Source/WebCore/platform/graphics/MediaPlayer.cpp 2021-02-05 15:11:44 UTC (rev 272413)
+++ trunk/Source/WebCore/platform/graphics/MediaPlayer.cpp 2021-02-05 15:50:53 UTC (rev 272414)
@@ -528,21 +528,7 @@
void MediaPlayer::reloadAndResumePlaybackIfNeeded()
{
- auto previousMediaTime = currentTime();
- bool wasPaused = paused();
-
- // FIXME: It would be even better if we could resume in full screen mode, but, for now, exiting full screen makes the video rendering work.
- if (client().mediaPlayerFullscreenMode() != MediaPlayer::VideoFullscreenModeNone)
- client().mediaPlayerExitFullscreen();
-
- m_currentMediaEngine = nullptr;
- loadWithNextMediaEngine(nullptr);
-
- prepareToPlay();
- if (!wasPaused)
- play();
- if (previousMediaTime)
- seekWhenPossible(previousMediaTime);
+ client().mediaPlayerReloadAndResumePlaybackIfNeeded();
}
void MediaPlayer::loadWithNextMediaEngine(const MediaPlayerFactory* current)
Modified: trunk/Source/WebCore/platform/graphics/MediaPlayer.h (272413 => 272414)
--- trunk/Source/WebCore/platform/graphics/MediaPlayer.h 2021-02-05 15:11:44 UTC (rev 272413)
+++ trunk/Source/WebCore/platform/graphics/MediaPlayer.h 2021-02-05 15:50:53 UTC (rev 272414)
@@ -217,8 +217,6 @@
virtual String mediaPlayerReferrer() const { return String(); }
virtual String mediaPlayerUserAgent() const { return String(); }
- virtual void mediaPlayerEnterFullscreen() { }
- virtual void mediaPlayerExitFullscreen() { }
virtual bool mediaPlayerIsFullscreen() const { return false; }
virtual bool mediaPlayerIsFullscreenPermitted() const { return false; }
virtual bool mediaPlayerIsVideo() const { return false; }
@@ -239,6 +237,8 @@
virtual void mediaPlayerDidRemoveTextTrack(InbandTextTrackPrivate&) { }
virtual void mediaPlayerDidRemoveVideoTrack(VideoTrackPrivate&) { }
+ virtual void mediaPlayerReloadAndResumePlaybackIfNeeded() { }
+
virtual void textTrackRepresentationBoundsChanged(const IntRect&) { }
#if ENABLE(AVF_CAPTIONS)
Modified: trunk/Source/WebKit/ChangeLog (272413 => 272414)
--- trunk/Source/WebKit/ChangeLog 2021-02-05 15:11:44 UTC (rev 272413)
+++ trunk/Source/WebKit/ChangeLog 2021-02-05 15:50:53 UTC (rev 272414)
@@ -1,3 +1,24 @@
+2021-02-05 Chris Dumez <[email protected]>
+
+ [GPUProcess][iOS] Audio is lost after media playback recovers from the GPUProcess crash
+ https://bugs.webkit.org/show_bug.cgi?id=221305
+ <rdar://problem/73904864>
+
+ Reviewed by Eric Carlson.
+
+ Fix issue where we would sometimes lose the video after recovering from a GPUProcess
+ crash. The issue is that RemoteMediaPlayerProxy::setVideoInlineSizeFenced() may get
+ called *before* RemoteMediaPlayerProxy::mediaPlayerFirstVideoFrameAvailable(). As a
+ result, we would not have a root layer yet and we would fail to set the video
+ dimensions and they would remain at 0x0.
+
+ * GPUProcess/media/RemoteMediaPlayerProxy.cpp:
+ * GPUProcess/media/RemoteMediaPlayerProxy.h:
+ * GPUProcess/media/cocoa/RemoteMediaPlayerProxyCocoa.mm:
+ (WebKit::setVideoInlineSizeIfPossible):
+ (WebKit::RemoteMediaPlayerProxy::mediaPlayerFirstVideoFrameAvailable):
+ (WebKit::RemoteMediaPlayerProxy::setVideoInlineSizeFenced):
+
2021-02-05 Carlos Garcia Campos <[email protected]>
REGRESSION(r271879) [SOUP] webrtc/datachannel/gather-candidates-networkprocess-crash.html is crashing in debug mode
Modified: trunk/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp (272413 => 272414)
--- trunk/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp 2021-02-05 15:11:44 UTC (rev 272413)
+++ trunk/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp 2021-02-05 15:50:53 UTC (rev 272414)
@@ -648,16 +648,6 @@
}
#endif
-void RemoteMediaPlayerProxy::mediaPlayerEnterFullscreen()
-{
- notImplemented();
-}
-
-void RemoteMediaPlayerProxy::mediaPlayerExitFullscreen()
-{
- notImplemented();
-}
-
bool RemoteMediaPlayerProxy::mediaPlayerIsFullscreen() const
{
return false;
Modified: trunk/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h (272413 => 272414)
--- trunk/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h 2021-02-05 15:11:44 UTC (rev 272413)
+++ trunk/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h 2021-02-05 15:50:53 UTC (rev 272414)
@@ -239,8 +239,6 @@
String mediaPlayerReferrer() const final;
String mediaPlayerUserAgent() const final;
- void mediaPlayerEnterFullscreen() final;
- void mediaPlayerExitFullscreen() final;
bool mediaPlayerIsFullscreen() const final;
bool mediaPlayerIsFullscreenPermitted() const final;
bool mediaPlayerIsVideo() const final;
@@ -312,7 +310,7 @@
RefPtr<RemoteMediaSourceProxy> m_mediaSourceProxy;
#endif
- WebCore::LayoutRect m_videoContentBoxRect;
+ WebCore::IntSize m_videoInlineSize;
float m_videoContentScale { 1.0 };
bool m_bufferedChanged { true };
Modified: trunk/Source/WebKit/GPUProcess/media/cocoa/RemoteMediaPlayerProxyCocoa.mm (272413 => 272414)
--- trunk/Source/WebKit/GPUProcess/media/cocoa/RemoteMediaPlayerProxyCocoa.mm 2021-02-05 15:11:44 UTC (rev 272413)
+++ trunk/Source/WebKit/GPUProcess/media/cocoa/RemoteMediaPlayerProxyCocoa.mm 2021-02-05 15:50:53 UTC (rev 272414)
@@ -36,6 +36,18 @@
namespace WebKit {
+static void setVideoInlineSizeIfPossible(LayerHostingContext& context, const WebCore::IntSize& size)
+{
+ if (!context.rootLayer())
+ return;
+
+ // We do not want animations here.
+ [CATransaction begin];
+ [CATransaction setDisableActions:YES];
+ [context.rootLayer() setFrame:CGRectMake(0, 0, size.width(), size.height())];
+ [CATransaction commit];
+}
+
void RemoteMediaPlayerProxy::prepareForPlayback(bool privateMode, WebCore::MediaPlayerEnums::Preload preload, bool preservesPitch, bool prepareForRendering, float videoContentScale, CompletionHandler<void(Optional<LayerHostingContextID>&& inlineLayerHostingContextId)>&& completionHandler)
{
m_player->setPrivateBrowsingMode(privateMode);
@@ -50,8 +62,9 @@
void RemoteMediaPlayerProxy::mediaPlayerFirstVideoFrameAvailable()
{
- // Initially the size of the platformLayer will be 0x0 because we do not provide mediaPlayerContentBoxRect() in this class.
+ // Initially the size of the platformLayer may be 0x0 because we do not provide mediaPlayerContentBoxRect() in this class.
m_inlineLayerHostingContext->setRootLayer(m_player->platformLayer());
+ setVideoInlineSizeIfPossible(*m_inlineLayerHostingContext, m_videoInlineSize);
m_webProcessConnection->send(Messages::MediaPlayerPrivateRemote::FirstVideoFrameAvailable(), m_id);
}
@@ -58,11 +71,9 @@
void RemoteMediaPlayerProxy::setVideoInlineSizeFenced(const WebCore::IntSize& size, const WTF::MachSendRight& machSendRight)
{
m_inlineLayerHostingContext->setFencePort(machSendRight.sendRight());
- // We do not want animations here
- [CATransaction begin];
- [CATransaction setDisableActions:YES];
- [m_inlineLayerHostingContext->rootLayer() setFrame:CGRectMake(0, 0, size.width(), size.height())];
- [CATransaction commit];
+
+ m_videoInlineSize = size;
+ setVideoInlineSizeIfPossible(*m_inlineLayerHostingContext, size);
}
} // namespace WebKit
Modified: trunk/Tools/ChangeLog (272413 => 272414)
--- trunk/Tools/ChangeLog 2021-02-05 15:11:44 UTC (rev 272413)
+++ trunk/Tools/ChangeLog 2021-02-05 15:50:53 UTC (rev 272414)
@@ -1,3 +1,16 @@
+2021-02-05 Chris Dumez <[email protected]>
+
+ [GPUProcess][iOS] Audio is lost after media playback recovers from the GPUProcess crash
+ https://bugs.webkit.org/show_bug.cgi?id=221305
+ <rdar://problem/73904864>
+
+ Reviewed by Eric Carlson.
+
+ Unskip API test checks on iOS now that they are passing.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/GPUProcess.mm:
+ (TEST):
+
2021-02-05 Alicia Boya García <[email protected]>
[GTK] run-gtk-tests: Support running individual tests for GTest test suites
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/GPUProcess.mm (272413 => 272414)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/GPUProcess.mm 2021-02-05 15:11:44 UTC (rev 272413)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/GPUProcess.mm 2021-02-05 15:50:53 UTC (rev 272414)
@@ -215,14 +215,11 @@
EXPECT_NE([processPool _gpuProcessIdentifier], gpuProcessPID);
gpuProcessPID = [processPool _gpuProcessIdentifier];
- // FIXME: On iOS, video resumes after the GPU process crash but audio does not.
-#if !PLATFORM(IOS)
// Audio should be playing again.
timeout = 0;
while (![webView _isPlayingAudio] && timeout++ < 100)
TestWebKitAPI::Util::sleep(0.1);
EXPECT_TRUE([webView _isPlayingAudio]);
-#endif
}
TEST(GPUProcess, CrashWhilePlayingVideo)
@@ -278,14 +275,11 @@
// Make sure the WebProcess did not crash.
EXPECT_EQ(webViewPID, [webView _webProcessIdentifier]);
- // FIXME: On iOS, video resumes after the GPU process crash but audio does not.
-#if !PLATFORM(IOS)
// Audio should resume playing.
timeout = 0;
while (![webView _isPlayingAudio] && timeout++ < 100)
TestWebKitAPI::Util::sleep(0.1);
EXPECT_TRUE([webView _isPlayingAudio]);
-#endif
EXPECT_EQ(gpuProcessPID, [processPool _gpuProcessIdentifier]);
EXPECT_EQ(webViewPID, [webView _webProcessIdentifier]);
