Changeset 269750 in webkit


Ignore:
Timestamp:
Nov 12, 2020 2:10:48 PM (3 years ago)
Author:
Chris Dumez
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):

Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r269747 r269750  
     12020-11-12  Chris Dumez  <cdumez@apple.com>
     2
     3        [GPUProcess] Add basic GPUProcess crash handling for media playback
     4        https://bugs.webkit.org/show_bug.cgi?id=218825
     5
     6        Reviewed by Eric Carlson.
     7
     8        Add utility function to MediaPlayer to reload the media engine and resume
     9        playback if necessary. This is called after the GPU process has crashed
     10        so that the playback picks up where it left off.
     11
     12        Also add a new seekWhenPossible() function that only attempts to seek
     13        once we've received the metadata. Any attempt to seek before the metadata
     14        has been received gets ignored otherwise.
     15
     16        * platform/graphics/MediaPlayer.cpp:
     17        (WebCore::MediaPlayer::reloadAndResumePlaybackIfNeeded):
     18        (WebCore::MediaPlayer::seekWhenPossible):
     19        (WebCore::MediaPlayer::readyStateChanged):
     20        * platform/graphics/MediaPlayer.h:
     21
    1222020-11-12  Antti Koivisto  <antti@apple.com>
    223

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

    r269711 r269750  
    527527}
    528528
     529void MediaPlayer::reloadAndResumePlaybackIfNeeded()
     530{
     531    auto previousMediaTime = currentTime();
     532    bool wasPaused = paused();
     533
     534    m_currentMediaEngine = nullptr;
     535    loadWithNextMediaEngine(nullptr);
     536
     537    prepareToPlay();
     538    if (!wasPaused)
     539        play();
     540    if (previousMediaTime)
     541        seekWhenPossible(previousMediaTime);
     542}
     543
    529544void MediaPlayer::loadWithNextMediaEngine(const MediaPlayerFactory* current)
    530545{
    … …  
    718733}
    719734
     735void MediaPlayer::seekWhenPossible(const MediaTime& time)
     736{
     737    if (m_private->readyState() < MediaPlayer::ReadyState::HaveMetadata)
     738        m_pendingSeekRequest = time;
     739    else
     740        seek(time);
     741}
     742
    720743bool MediaPlayer::paused() const
    721744{
    … …  
    12321255{
    12331256    client().mediaPlayerReadyStateChanged();
     1257    if (m_pendingSeekRequest && m_private->readyState() == MediaPlayer::ReadyState::HaveMetadata)
     1258        seek(*std::exchange(m_pendingSeekRequest, WTF::nullopt));
    12341259}
    12351260

  • trunk/Source/WebCore/platform/graphics/MediaPlayer.h

    r269711 r269750  
    308308    PlatformLayer* platformLayer() const;
    309309
     310    void reloadAndResumePlaybackIfNeeded();
     311
    310312#if ENABLE(VIDEO_PRESENTATION_MODE)
    311313    RetainPtr<PlatformLayer> createVideoFullscreenLayer();
    … …  
    382384    MediaTime currentTime() const;
    383385    void seek(const MediaTime&);
     386    void seekWhenPossible(const MediaTime&);
    384387    void seekWithTolerance(const MediaTime&, const MediaTime& negativeTolerance, const MediaTime& positiveTolerance);
    385388
    … …  
    657660    String m_keySystem;
    658661    Optional<MediaPlayerEnums::MediaEngineIdentifier> m_activeEngineIdentifier;
     662    Optional<MediaTime> m_pendingSeekRequest;
    659663    IntSize m_size;
    660664    Preload m_preload { Preload::Auto };

  • trunk/Source/WebKit/ChangeLog

    r269749 r269750  
     12020-11-12  Chris Dumez  <cdumez@apple.com>
     2
     3        [GPUProcess] Add basic GPUProcess crash handling for media playback
     4        https://bugs.webkit.org/show_bug.cgi?id=218825
     5
     6        Reviewed by Eric Carlson.
     7
     8        When the GPU process crashes and there are pending media players, we
     9        now relaunch the GPU process and ask those media players to reconstuct
     10        their private media players, reload the file and resume playback if
     11        necessary. This ensures that in case of a GPU process crash while the
     12        user is playing a video, the video just seamlessly keeps playing from
     13        where it was before the crash.
     14
     15        Before this patch, we would end up busy looping and the WebContent process
     16        would use 100% CPU after a GPU process crash.
     17
     18        * WebProcess/GPU/GPUProcessConnection.h:
     19        (WebKit::GPUProcessConnection::addClient):
     20        (WebKit::GPUProcessConnection::removeClient):
     21        * WebProcess/GPU/media/MediaPlayerPrivateRemote.h:
     22        * WebProcess/GPU/media/RemoteMediaPlayerManager.cpp:
     23        (WebKit::proxyConfigurationForPlayer):
     24        (WebKit::RemoteMediaPlayerManager::gpuProcessConnection const):
     25        (WebKit::RemoteMediaPlayerManager::gpuProcessConnectionDidClose):
     26        * WebProcess/GPU/media/RemoteMediaPlayerManager.h:
     27
    1282020-11-12  Chris Dumez  <cdumez@apple.com>
    229

  • trunk/Source/WebKit/WebProcess/GPU/GPUProcessConnection.h

    r269698 r269750  
    9494        virtual void gpuProcessConnectionDidClose(GPUProcessConnection&) { }
    9595    };
    96     void addClient(Client& client) { m_clients.add(client); }
    97     void removeClient(Client& client) { m_clients.remove(client); }
     96    void addClient(const Client& client) { m_clients.add(client); }
     97    void removeClient(const Client& client) { m_clients.remove(client); }
    9898
    9999private:

  • trunk/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h

    r269614 r269750  
    8787    WebCore::MediaPlayerIdentifier itentifier() const { return m_id; }
    8888    IPC::Connection& connection() const { return m_manager.gpuProcessConnection().connection(); }
     89    WebCore::MediaPlayer* player() const { return m_player; }
    8990
    9091    void networkStateChanged(RemoteMediaPlayerState&&);

  • trunk/Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.cpp

    r269711 r269750  
    266266GPUProcessConnection& RemoteMediaPlayerManager::gpuProcessConnection() const
    267267{
    268     if (!m_gpuProcessConnection)
     268    if (!m_gpuProcessConnection) {
    269269        m_gpuProcessConnection = &WebProcess::singleton().ensureGPUProcessConnection();
     270        m_gpuProcessConnection->addClient(*this);
     271    }
    270272
    271273    return *m_gpuProcessConnection;
    272274}
    273275
     276void RemoteMediaPlayerManager::gpuProcessConnectionDidClose(GPUProcessConnection& connection)
     277{
     278    ASSERT(m_gpuProcessConnection == &connection);
     279    connection.removeClient(*this);
     280
     281    m_gpuProcessConnection = nullptr;
     282
     283    auto players = m_players;
     284    for (auto& player : players.values()) {
     285        if (player) {
     286            player->player()->reloadAndResumePlaybackIfNeeded();
     287            ASSERT_WITH_MESSAGE(!player, "reloadAndResumePlaybackIfNeeded should destroy this player and construct a new one");
     288        }
     289    }
     290}
     291
    274292} // namespace WebKit
    275293

  • trunk/Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.h

    r269416 r269750  
    4848class RemoteMediaPlayerManager
    4949    : public WebProcessSupplement
    50     , public CanMakeWeakPtr<RemoteMediaPlayerManager> {
     50    , public GPUProcessConnection::Client {
    5151    WTF_MAKE_FAST_ALLOCATED;
    5252public:
    … …  
    7373    void initialize(const WebProcessCreationParameters&) final;
    7474
     75    // GPUProcessConnection::Client
     76    void gpuProcessConnectionDidClose(GPUProcessConnection&) final;
     77
    7578    friend class MediaPlayerRemoteFactory;
    7679    void getSupportedTypes(WebCore::MediaPlayerEnums::MediaEngineIdentifier, HashSet<String, ASCIICaseInsensitiveHash>&);

  • trunk/Tools/ChangeLog

    r269738 r269750  
     12020-11-12  Chris Dumez  <cdumez@apple.com>
     2
     3        [GPUProcess] Add basic GPUProcess crash handling for media playback
     4        https://bugs.webkit.org/show_bug.cgi?id=218825
     5
     6        Reviewed by Eric Carlson.
     7
     8        Add API test coverage.
     9
     10        * TestWebKitAPI/Tests/WebKitCocoa/GPUProcess.mm:
     11        (TEST):
     12
    1132020-11-12  Aakash Jain  <aakash_jain@apple.com>
    214

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/GPUProcess.mm

    r269703 r269750  
    198198    EXPECT_TRUE([webView _isPlayingAudio]);
    199199}
     200
     201TEST(GPUProcess, CrashWhilePlayingVideo)
     202{
     203    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
     204    for (_WKInternalDebugFeature *feature in [WKPreferences _internalDebugFeatures]) {
     205        if ([feature.key isEqualToString:@"UseGPUProcessForMediaEnabled"]) {
     206            [[configuration preferences] _setEnabled:YES forInternalDebugFeature:feature];
     207            break;
     208        }
     209    }
     210
     211    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 400, 400) configuration:configuration.get()]);
     212    [webView synchronouslyLoadTestPageNamed:@"large-videos-with-audio"];
     213
     214    __block bool done = false;
     215    [webView evaluateJavaScript:@"document.getElementsByTagName('video')[0].play() && true" completionHandler:^(id result, NSError *error) {
     216        EXPECT_TRUE(!error);
     217        done = true;
     218    }];
     219    TestWebKitAPI::Util::run(&done);
     220
     221    auto webViewPID = [webView _webProcessIdentifier];
     222
     223    // The GPU process should get launched.
     224    auto* processPool = configuration.get().processPool;
     225    unsigned timeout = 0;
     226    while (![processPool _gpuProcessIdentifier] && timeout++ < 100)
     227        TestWebKitAPI::Util::sleep(0.1);
     228
     229    EXPECT_NE([processPool _gpuProcessIdentifier], 0);
     230    if (![processPool _gpuProcessIdentifier])
     231        return;
     232    auto gpuProcessPID = [processPool _gpuProcessIdentifier];
     233
     234    // Audio should be playing.
     235    timeout = 0;
     236    while (![webView _isPlayingAudio] && timeout++ < 100)
     237        TestWebKitAPI::Util::sleep(0.1);
     238    EXPECT_TRUE([webView _isPlayingAudio]);
     239
     240    // Kill the GPU Process.
     241    kill(gpuProcessPID, 9);
     242
     243    // GPU Process should get relaunched.
     244    timeout = 0;
     245    while ((![processPool _gpuProcessIdentifier] || [processPool _gpuProcessIdentifier] == gpuProcessPID) && timeout++ < 100)
     246        TestWebKitAPI::Util::sleep(0.1);
     247    EXPECT_NE([processPool _gpuProcessIdentifier], 0);
     248    EXPECT_NE([processPool _gpuProcessIdentifier], gpuProcessPID);
     249    gpuProcessPID = [processPool _gpuProcessIdentifier];
     250
     251    // Make sure the WebProcess did not crash.
     252    EXPECT_EQ(webViewPID, [webView _webProcessIdentifier]);
     253
     254    // Audio should resume playing.
     255    timeout = 0;
     256    while (![webView _isPlayingAudio] && timeout++ < 100)
     257        TestWebKitAPI::Util::sleep(0.1);
     258    EXPECT_TRUE([webView _isPlayingAudio]);
     259
     260    EXPECT_EQ(gpuProcessPID, [processPool _gpuProcessIdentifier]);
     261    EXPECT_EQ(webViewPID, [webView _webProcessIdentifier]);
     262}
Note: See TracChangeset for help on using the changeset viewer.