- Revision
- 269750
- Author
- [email protected]
- Date
- 2020-11-12 14:10:48 -0800 (Thu, 12 Nov 2020)
Log Message
[GPUProcess] Add basic GPUProcess crash handling for media playback
https://bugs.webkit.org/show_bug.cgi?id=218825
Reviewed by Eric Carlson.
Source/WebCore:
Add utility function to MediaPlayer to reload the media engine and resume
playback if necessary. This is called after the GPU process has crashed
so that the playback picks up where it left off.
Also add a new seekWhenPossible() function that only attempts to seek
once we've received the metadata. Any attempt to seek before the metadata
has been received gets ignored otherwise.
* platform/graphics/MediaPlayer.cpp:
(WebCore::MediaPlayer::reloadAndResumePlaybackIfNeeded):
(WebCore::MediaPlayer::seekWhenPossible):
(WebCore::MediaPlayer::readyStateChanged):
* platform/graphics/MediaPlayer.h:
Source/WebKit:
When the GPU process crashes and there are pending media players, we
now relaunch the GPU process and ask those media players to reconstuct
their private media players, reload the file and resume playback if
necessary. This ensures that in case of a GPU process crash while the
user is playing a video, the video just seamlessly keeps playing from
where it was before the crash.
Before this patch, we would end up busy looping and the WebContent process
would use 100% CPU after a GPU process crash.
* WebProcess/GPU/GPUProcessConnection.h:
(WebKit::GPUProcessConnection::addClient):
(WebKit::GPUProcessConnection::removeClient):
* WebProcess/GPU/media/MediaPlayerPrivateRemote.h:
* WebProcess/GPU/media/RemoteMediaPlayerManager.cpp:
(WebKit::proxyConfigurationForPlayer):
(WebKit::RemoteMediaPlayerManager::gpuProcessConnection const):
(WebKit::RemoteMediaPlayerManager::gpuProcessConnectionDidClose):
* WebProcess/GPU/media/RemoteMediaPlayerManager.h:
Tools:
Add API test coverage.
* TestWebKitAPI/Tests/WebKitCocoa/GPUProcess.mm:
(TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (269749 => 269750)
--- trunk/Source/WebCore/ChangeLog 2020-11-12 22:01:18 UTC (rev 269749)
+++ trunk/Source/WebCore/ChangeLog 2020-11-12 22:10:48 UTC (rev 269750)
@@ -1,3 +1,24 @@
+2020-11-12 Chris Dumez <[email protected]>
+
+ [GPUProcess] Add basic GPUProcess crash handling for media playback
+ https://bugs.webkit.org/show_bug.cgi?id=218825
+
+ Reviewed by Eric Carlson.
+
+ Add utility function to MediaPlayer to reload the media engine and resume
+ playback if necessary. This is called after the GPU process has crashed
+ so that the playback picks up where it left off.
+
+ Also add a new seekWhenPossible() function that only attempts to seek
+ once we've received the metadata. Any attempt to seek before the metadata
+ has been received gets ignored otherwise.
+
+ * platform/graphics/MediaPlayer.cpp:
+ (WebCore::MediaPlayer::reloadAndResumePlaybackIfNeeded):
+ (WebCore::MediaPlayer::seekWhenPossible):
+ (WebCore::MediaPlayer::readyStateChanged):
+ * platform/graphics/MediaPlayer.h:
+
2020-11-12 Antti Koivisto <[email protected]>
[LFC][Integration] VisiblePosition::absoluteSelectionBoundsForLine should use iterator
Modified: trunk/Source/WebCore/platform/graphics/MediaPlayer.cpp (269749 => 269750)
--- trunk/Source/WebCore/platform/graphics/MediaPlayer.cpp 2020-11-12 22:01:18 UTC (rev 269749)
+++ trunk/Source/WebCore/platform/graphics/MediaPlayer.cpp 2020-11-12 22:10:48 UTC (rev 269750)
@@ -526,6 +526,21 @@
return bestMediaEngineForSupportParameters(parameters, current);
}
+void MediaPlayer::reloadAndResumePlaybackIfNeeded()
+{
+ auto previousMediaTime = currentTime();
+ bool wasPaused = paused();
+
+ m_currentMediaEngine = nullptr;
+ loadWithNextMediaEngine(nullptr);
+
+ prepareToPlay();
+ if (!wasPaused)
+ play();
+ if (previousMediaTime)
+ seekWhenPossible(previousMediaTime);
+}
+
void MediaPlayer::loadWithNextMediaEngine(const MediaPlayerFactory* current)
{
#if ENABLE(MEDIA_SOURCE)
@@ -717,6 +732,14 @@
m_private->seek(time);
}
+void MediaPlayer::seekWhenPossible(const MediaTime& time)
+{
+ if (m_private->readyState() < MediaPlayer::ReadyState::HaveMetadata)
+ m_pendingSeekRequest = time;
+ else
+ seek(time);
+}
+
bool MediaPlayer::paused() const
{
return m_private->paused();
@@ -1231,6 +1254,8 @@
void MediaPlayer::readyStateChanged()
{
client().mediaPlayerReadyStateChanged();
+ if (m_pendingSeekRequest && m_private->readyState() == MediaPlayer::ReadyState::HaveMetadata)
+ seek(*std::exchange(m_pendingSeekRequest, WTF::nullopt));
}
void MediaPlayer::volumeChanged(double newVolume)
Modified: trunk/Source/WebCore/platform/graphics/MediaPlayer.h (269749 => 269750)
--- trunk/Source/WebCore/platform/graphics/MediaPlayer.h 2020-11-12 22:01:18 UTC (rev 269749)
+++ trunk/Source/WebCore/platform/graphics/MediaPlayer.h 2020-11-12 22:10:48 UTC (rev 269750)
@@ -307,6 +307,8 @@
bool doesHaveAttribute(const AtomString&, AtomString* value = nullptr) const;
PlatformLayer* platformLayer() const;
+ void reloadAndResumePlaybackIfNeeded();
+
#if ENABLE(VIDEO_PRESENTATION_MODE)
RetainPtr<PlatformLayer> createVideoFullscreenLayer();
void setVideoFullscreenLayer(PlatformLayer*, WTF::Function<void()>&& completionHandler = [] { });
@@ -381,6 +383,7 @@
MediaTime duration() const;
MediaTime currentTime() const;
void seek(const MediaTime&);
+ void seekWhenPossible(const MediaTime&);
void seekWithTolerance(const MediaTime&, const MediaTime& negativeTolerance, const MediaTime& positiveTolerance);
MediaTime startTime() const;
@@ -656,6 +659,7 @@
ContentType m_contentType;
String m_keySystem;
Optional<MediaPlayerEnums::MediaEngineIdentifier> m_activeEngineIdentifier;
+ Optional<MediaTime> m_pendingSeekRequest;
IntSize m_size;
Preload m_preload { Preload::Auto };
double m_volume { 1 };
Modified: trunk/Source/WebKit/ChangeLog (269749 => 269750)
--- trunk/Source/WebKit/ChangeLog 2020-11-12 22:01:18 UTC (rev 269749)
+++ trunk/Source/WebKit/ChangeLog 2020-11-12 22:10:48 UTC (rev 269750)
@@ -1,5 +1,32 @@
2020-11-12 Chris Dumez <[email protected]>
+ [GPUProcess] Add basic GPUProcess crash handling for media playback
+ https://bugs.webkit.org/show_bug.cgi?id=218825
+
+ Reviewed by Eric Carlson.
+
+ When the GPU process crashes and there are pending media players, we
+ now relaunch the GPU process and ask those media players to reconstuct
+ their private media players, reload the file and resume playback if
+ necessary. This ensures that in case of a GPU process crash while the
+ user is playing a video, the video just seamlessly keeps playing from
+ where it was before the crash.
+
+ Before this patch, we would end up busy looping and the WebContent process
+ would use 100% CPU after a GPU process crash.
+
+ * WebProcess/GPU/GPUProcessConnection.h:
+ (WebKit::GPUProcessConnection::addClient):
+ (WebKit::GPUProcessConnection::removeClient):
+ * WebProcess/GPU/media/MediaPlayerPrivateRemote.h:
+ * WebProcess/GPU/media/RemoteMediaPlayerManager.cpp:
+ (WebKit::proxyConfigurationForPlayer):
+ (WebKit::RemoteMediaPlayerManager::gpuProcessConnection const):
+ (WebKit::RemoteMediaPlayerManager::gpuProcessConnectionDidClose):
+ * WebProcess/GPU/media/RemoteMediaPlayerManager.h:
+
+2020-11-12 Chris Dumez <[email protected]>
+
ASSERTION FAILED: isValidIdentifier(m_identifier) seen with TestWebKitAPI.GPUProcess.WebProcessTerminationAfterTooManyGPUProcessCrashes
https://bugs.webkit.org/show_bug.cgi?id=218856
<rdar://problem/71331809>
Modified: trunk/Source/WebKit/WebProcess/GPU/GPUProcessConnection.h (269749 => 269750)
--- trunk/Source/WebKit/WebProcess/GPU/GPUProcessConnection.h 2020-11-12 22:01:18 UTC (rev 269749)
+++ trunk/Source/WebKit/WebProcess/GPU/GPUProcessConnection.h 2020-11-12 22:10:48 UTC (rev 269750)
@@ -93,8 +93,8 @@
virtual void gpuProcessConnectionDidClose(GPUProcessConnection&) { }
};
- void addClient(Client& client) { m_clients.add(client); }
- void removeClient(Client& client) { m_clients.remove(client); }
+ void addClient(const Client& client) { m_clients.add(client); }
+ void removeClient(const Client& client) { m_clients.remove(client); }
private:
GPUProcessConnection(IPC::Connection::Identifier);
Modified: trunk/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h (269749 => 269750)
--- trunk/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h 2020-11-12 22:01:18 UTC (rev 269749)
+++ trunk/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h 2020-11-12 22:10:48 UTC (rev 269750)
@@ -86,6 +86,7 @@
WebCore::MediaPlayerEnums::MediaEngineIdentifier remoteEngineIdentifier() const { return m_remoteEngineIdentifier; }
WebCore::MediaPlayerIdentifier itentifier() const { return m_id; }
IPC::Connection& connection() const { return m_manager.gpuProcessConnection().connection(); }
+ WebCore::MediaPlayer* player() const { return m_player; }
void networkStateChanged(RemoteMediaPlayerState&&);
void readyStateChanged(RemoteMediaPlayerState&&);
Modified: trunk/Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.cpp (269749 => 269750)
--- trunk/Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.cpp 2020-11-12 22:01:18 UTC (rev 269749)
+++ trunk/Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.cpp 2020-11-12 22:10:48 UTC (rev 269750)
@@ -265,12 +265,30 @@
GPUProcessConnection& RemoteMediaPlayerManager::gpuProcessConnection() const
{
- if (!m_gpuProcessConnection)
+ if (!m_gpuProcessConnection) {
m_gpuProcessConnection = &WebProcess::singleton().ensureGPUProcessConnection();
+ m_gpuProcessConnection->addClient(*this);
+ }
return *m_gpuProcessConnection;
}
+void RemoteMediaPlayerManager::gpuProcessConnectionDidClose(GPUProcessConnection& connection)
+{
+ ASSERT(m_gpuProcessConnection == &connection);
+ connection.removeClient(*this);
+
+ m_gpuProcessConnection = nullptr;
+
+ auto players = m_players;
+ for (auto& player : players.values()) {
+ if (player) {
+ player->player()->reloadAndResumePlaybackIfNeeded();
+ ASSERT_WITH_MESSAGE(!player, "reloadAndResumePlaybackIfNeeded should destroy this player and construct a new one");
+ }
+ }
+}
+
} // namespace WebKit
#endif
Modified: trunk/Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.h (269749 => 269750)
--- trunk/Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.h 2020-11-12 22:01:18 UTC (rev 269749)
+++ trunk/Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.h 2020-11-12 22:10:48 UTC (rev 269750)
@@ -47,7 +47,7 @@
class RemoteMediaPlayerManager
: public WebProcessSupplement
- , public CanMakeWeakPtr<RemoteMediaPlayerManager> {
+ , public GPUProcessConnection::Client {
WTF_MAKE_FAST_ALLOCATED;
public:
explicit RemoteMediaPlayerManager(WebProcess&);
@@ -72,6 +72,9 @@
// WebProcessSupplement
void initialize(const WebProcessCreationParameters&) final;
+ // GPUProcessConnection::Client
+ void gpuProcessConnectionDidClose(GPUProcessConnection&) final;
+
friend class MediaPlayerRemoteFactory;
void getSupportedTypes(WebCore::MediaPlayerEnums::MediaEngineIdentifier, HashSet<String, ASCIICaseInsensitiveHash>&);
WebCore::MediaPlayer::SupportsType supportsTypeAndCodecs(WebCore::MediaPlayerEnums::MediaEngineIdentifier, const WebCore::MediaEngineSupportParameters&);
Modified: trunk/Tools/ChangeLog (269749 => 269750)
--- trunk/Tools/ChangeLog 2020-11-12 22:01:18 UTC (rev 269749)
+++ trunk/Tools/ChangeLog 2020-11-12 22:10:48 UTC (rev 269750)
@@ -1,3 +1,15 @@
+2020-11-12 Chris Dumez <[email protected]>
+
+ [GPUProcess] Add basic GPUProcess crash handling for media playback
+ https://bugs.webkit.org/show_bug.cgi?id=218825
+
+ Reviewed by Eric Carlson.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/GPUProcess.mm:
+ (TEST):
+
2020-11-12 Aakash Jain <[email protected]>
[build.webkit.org] configure buildbot not to send Usage Data to buildbot developers
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/GPUProcess.mm (269749 => 269750)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/GPUProcess.mm 2020-11-12 22:01:18 UTC (rev 269749)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/GPUProcess.mm 2020-11-12 22:10:48 UTC (rev 269750)
@@ -197,3 +197,66 @@
TestWebKitAPI::Util::sleep(0.1);
EXPECT_TRUE([webView _isPlayingAudio]);
}
+
+TEST(GPUProcess, CrashWhilePlayingVideo)
+{
+ auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ for (_WKInternalDebugFeature *feature in [WKPreferences _internalDebugFeatures]) {
+ if ([feature.key isEqualToString:@"UseGPUProcessForMediaEnabled"]) {
+ [[configuration preferences] _setEnabled:YES forInternalDebugFeature:feature];
+ break;
+ }
+ }
+
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 400, 400) configuration:configuration.get()]);
+ [webView synchronouslyLoadTestPageNamed:@"large-videos-with-audio"];
+
+ __block bool done = false;
+ [webView evaluateJavaScript:@"document.getElementsByTagName('video')[0].play() && true" completionHandler:^(id result, NSError *error) {
+ EXPECT_TRUE(!error);
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+
+ auto webViewPID = [webView _webProcessIdentifier];
+
+ // The GPU process should get launched.
+ auto* processPool = configuration.get().processPool;
+ unsigned timeout = 0;
+ while (![processPool _gpuProcessIdentifier] && timeout++ < 100)
+ TestWebKitAPI::Util::sleep(0.1);
+
+ EXPECT_NE([processPool _gpuProcessIdentifier], 0);
+ if (![processPool _gpuProcessIdentifier])
+ return;
+ auto gpuProcessPID = [processPool _gpuProcessIdentifier];
+
+ // Audio should be playing.
+ timeout = 0;
+ while (![webView _isPlayingAudio] && timeout++ < 100)
+ TestWebKitAPI::Util::sleep(0.1);
+ EXPECT_TRUE([webView _isPlayingAudio]);
+
+ // Kill the GPU Process.
+ kill(gpuProcessPID, 9);
+
+ // GPU Process should get relaunched.
+ timeout = 0;
+ while ((![processPool _gpuProcessIdentifier] || [processPool _gpuProcessIdentifier] == gpuProcessPID) && timeout++ < 100)
+ TestWebKitAPI::Util::sleep(0.1);
+ EXPECT_NE([processPool _gpuProcessIdentifier], 0);
+ EXPECT_NE([processPool _gpuProcessIdentifier], gpuProcessPID);
+ gpuProcessPID = [processPool _gpuProcessIdentifier];
+
+ // Make sure the WebProcess did not crash.
+ EXPECT_EQ(webViewPID, [webView _webProcessIdentifier]);
+
+ // Audio should resume playing.
+ timeout = 0;
+ while (![webView _isPlayingAudio] && timeout++ < 100)
+ TestWebKitAPI::Util::sleep(0.1);
+ EXPECT_TRUE([webView _isPlayingAudio]);
+
+ EXPECT_EQ(gpuProcessPID, [processPool _gpuProcessIdentifier]);
+ EXPECT_EQ(webViewPID, [webView _webProcessIdentifier]);
+}
