Changeset 255116 in webkit


Ignore:
Timestamp:
Jan 25, 2020 4:27:39 AM (4 years ago)
Author:
youenn@apple.com
Message:

HTMLMediaElement should not remove the media session at DOM suspension time
​https://bugs.webkit.org/show_bug.cgi?id=206661
<rdar://problem/58800787>

Source/WebCore:

Reviewed by Eric Carlson.

​https://trac.webkit.org/changeset/233560 made it so that, on HTMLMediaElement suspension,
its media session is stopped.
This was done to ensure updateNowPlayingInfo is not called synchronously but asynchronously.
The issue is that, once the media session is stopped, it is removed from the media session vector.
On updating the ready state after suspension, and playing, we try to look into the media session vector and do not find the session.
This triggers the ASSERT.

Partially revert the behavior by calling the same code as clientWillPausePlayback
but make sure updateNowPlayingInfo is calling asynchronously when suspending the media element.
Introduce clientWillBeDOMSuspended for that purpose.

Update mediaPlayerReadyStateChanged to enqueue a task to do the update if the media element is suspended.

Covered by test no longer crashing in debug.

  • html/HTMLMediaElement.cpp:

(WebCore::HTMLMediaElement::mediaPlayerReadyStateChanged):
(WebCore::HTMLMediaElement::stopWithoutDestroyingMediaPlayer):

  • platform/audio/PlatformMediaSession.cpp:

(WebCore::PlatformMediaSession::processClientWillPausePlayback):
(WebCore::PlatformMediaSession::clientWillPausePlayback):
(WebCore::PlatformMediaSession::clientWillBeDOMSuspended):

  • platform/audio/PlatformMediaSession.h:
  • platform/audio/PlatformMediaSessionManager.cpp:

(WebCore::PlatformMediaSessionManager::sessionWillEndPlayback):

  • platform/audio/PlatformMediaSessionManager.h:
  • platform/audio/cocoa/MediaSessionManagerCocoa.h:
  • platform/audio/cocoa/MediaSessionManagerCocoa.mm:

(MediaSessionManagerCocoa::sessionWillEndPlayback):

  • platform/audio/ios/MediaSessionManagerIOS.h:
  • platform/audio/ios/MediaSessionManagerIOS.mm:

(WebCore::MediaSessionManageriOS::sessionWillEndPlayback):

Tools:

Reviewed by Eric Carlson.

  • TestWebKitAPI/Tests/WebKitLegacy/ios/ScrollingDoesNotPauseMedia.mm:

(TestWebKitAPI::TEST):
Suspend/resume Active DOM Objects from time to time as would do scrolling.
This allows pending tasks to be executed asynchronously when not scrolling.

Location:
trunk
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r255113 r255116  
     12020-01-25  youenn fablet  <youenn@apple.com>
     2
     3        HTMLMediaElement should not remove the media session at DOM suspension time
     4        https://bugs.webkit.org/show_bug.cgi?id=206661
     5        <rdar://problem/58800787>
     6
     7        Reviewed by Eric Carlson.
     8
     9        https://trac.webkit.org/changeset/233560 made it so that, on HTMLMediaElement suspension,
     10        its media session is stopped.
     11        This was done to ensure updateNowPlayingInfo is not called synchronously but asynchronously.
     12        The issue is that, once the media session is stopped, it is removed from the media session vector.
     13        On updating the ready state after suspension, and playing, we try to look into the media session vector and do not find the session.
     14        This triggers the ASSERT.
     15
     16        Partially revert the behavior by calling the same code as clientWillPausePlayback
     17        but make sure updateNowPlayingInfo is calling asynchronously when suspending the media element.
     18        Introduce clientWillBeDOMSuspended for that purpose.
     19
     20        Update mediaPlayerReadyStateChanged to enqueue a task to do the update if the media element is suspended.
     21
     22        Covered by test no longer crashing in debug.
     23
     24        * html/HTMLMediaElement.cpp:
     25        (WebCore::HTMLMediaElement::mediaPlayerReadyStateChanged):
     26        (WebCore::HTMLMediaElement::stopWithoutDestroyingMediaPlayer):
     27        * platform/audio/PlatformMediaSession.cpp:
     28        (WebCore::PlatformMediaSession::processClientWillPausePlayback):
     29        (WebCore::PlatformMediaSession::clientWillPausePlayback):
     30        (WebCore::PlatformMediaSession::clientWillBeDOMSuspended):
     31        * platform/audio/PlatformMediaSession.h:
     32        * platform/audio/PlatformMediaSessionManager.cpp:
     33        (WebCore::PlatformMediaSessionManager::sessionWillEndPlayback):
     34        * platform/audio/PlatformMediaSessionManager.h:
     35        * platform/audio/cocoa/MediaSessionManagerCocoa.h:
     36        * platform/audio/cocoa/MediaSessionManagerCocoa.mm:
     37        (MediaSessionManagerCocoa::sessionWillEndPlayback):
     38        * platform/audio/ios/MediaSessionManagerIOS.h:
     39        * platform/audio/ios/MediaSessionManagerIOS.mm:
     40        (WebCore::MediaSessionManageriOS::sessionWillEndPlayback):
     41
    1422020-01-24  Jack Lee  <shihchieh_lee@apple.com>
    243

  • trunk/Source/WebCore/html/HTMLMediaElement.cpp

    r254964 r255116  
    23272327void HTMLMediaElement::mediaPlayerReadyStateChanged()
    23282328{
     2329    if (isSuspended()) {
     2330        queueTaskKeepingObjectAlive(*this, TaskSource::MediaElement, [this] {
     2331            mediaPlayerReadyStateChanged();
     2332        });
     2333        return;
     2334    }
     2335
    23292336    beginProcessingMediaPlayerCallback();
    23302337
    … …  
    56275634    setPlaying(false);
    56285635    pauseAndUpdatePlayStateImmediately();
    5629     m_mediaSession->stopSession();
     5636    m_mediaSession->clientWillBeDOMSuspended();
    56305637
    56315638    setAutoplayEventPlaybackState(AutoplayEventPlaybackState::None);

  • trunk/Source/WebCore/platform/audio/PlatformMediaSession.cpp

    r252470 r255116  
    225225}
    226226
    227 bool PlatformMediaSession::clientWillPausePlayback()
     227bool PlatformMediaSession::processClientWillPausePlayback(DelayCallingUpdateNowPlaying shouldDelayCallingUpdateNowPlaying)
    228228{
    229229    if (m_notifyingClient)
    … …  
    238238   
    239239    setState(Paused);
    240     PlatformMediaSessionManager::sharedManager().sessionWillEndPlayback(*this);
     240    PlatformMediaSessionManager::sharedManager().sessionWillEndPlayback(*this, shouldDelayCallingUpdateNowPlaying);
    241241    return true;
     242}
     243
     244bool PlatformMediaSession::clientWillPausePlayback()
     245{
     246    return processClientWillPausePlayback(DelayCallingUpdateNowPlaying::No);
     247}
     248
     249void PlatformMediaSession::clientWillBeDOMSuspended()
     250{
     251    processClientWillPausePlayback(DelayCallingUpdateNowPlaying::Yes);
    242252}
    243253

  • trunk/Source/WebCore/platform/audio/PlatformMediaSession.h

    r251737 r255116  
    4242class MediaPlaybackTarget;
    4343class PlatformMediaSessionClient;
     44enum class DelayCallingUpdateNowPlaying { No, Yes };
    4445
    4546class PlatformMediaSession
    … …  
    113114    virtual bool clientWillBeginPlayback();
    114115    virtual bool clientWillPausePlayback();
     116
     117    void clientWillBeDOMSuspended();
    115118
    116119    void pauseSession();
    … …  
    200203
    201204private:
     205    bool processClientWillPausePlayback(DelayCallingUpdateNowPlaying);
     206
    202207    PlatformMediaSessionClient& m_client;
    203208    State m_state;

  • trunk/Source/WebCore/platform/audio/PlatformMediaSessionManager.cpp

    r252470 r255116  
    236236}
    237237   
    238 void PlatformMediaSessionManager::sessionWillEndPlayback(PlatformMediaSession& session)
     238void PlatformMediaSessionManager::sessionWillEndPlayback(PlatformMediaSession& session, DelayCallingUpdateNowPlaying)
    239239{
    240240    ALWAYS_LOG(LOGIDENTIFIER, session.logIdentifier());

  • trunk/Source/WebCore/platform/audio/PlatformMediaSessionManager.h

    r252470 r255116  
    114114
    115115    virtual bool sessionWillBeginPlayback(PlatformMediaSession&);
    116     virtual void sessionWillEndPlayback(PlatformMediaSession&);
     116
     117    virtual void sessionWillEndPlayback(PlatformMediaSession&, DelayCallingUpdateNowPlaying);
    117118    virtual void sessionStateChanged(PlatformMediaSession&);
    118119    virtual void sessionDidEndRemoteScrubbing(const PlatformMediaSession&) { };

  • trunk/Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.h

    r252470 r255116  
    5454   
    5555    bool sessionWillBeginPlayback(PlatformMediaSession&) override;
    56     void sessionWillEndPlayback(PlatformMediaSession&) override;
     56    void sessionWillEndPlayback(PlatformMediaSession&, DelayCallingUpdateNowPlaying) override;
    5757    void sessionDidEndRemoteScrubbing(const PlatformMediaSession&) override;
    5858    void clientCharacteristicsChanged(PlatformMediaSession&) override;

  • trunk/Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm

    r252470 r255116  
    162162}
    163163
    164 void MediaSessionManagerCocoa::sessionWillEndPlayback(PlatformMediaSession& session)
    165 {
    166     PlatformMediaSessionManager::sessionWillEndPlayback(session);
    167     updateNowPlayingInfo();
     164void MediaSessionManagerCocoa::sessionWillEndPlayback(PlatformMediaSession& session, DelayCallingUpdateNowPlaying delayCallingUpdateNowPlaying)
     165{
     166    PlatformMediaSessionManager::sessionWillEndPlayback(session, delayCallingUpdateNowPlaying);
     167    if (delayCallingUpdateNowPlaying == DelayCallingUpdateNowPlaying::No) {
     168        updateNowPlayingInfo();
     169        return;
     170    }
     171    scheduleUpdateNowPlayingInfo();
    168172}
    169173

  • trunk/Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.h

    r254878 r255116  
    6464    void configureWireLessTargetMonitoring() override;
    6565    void providePresentingApplicationPIDIfNecessary() final;
    66     void sessionWillEndPlayback(PlatformMediaSession&) final;
     66    void sessionWillEndPlayback(PlatformMediaSession&, DelayCallingUpdateNowPlaying) final;
    6767
    6868#if !RELEASE_LOG_DISABLED

  • trunk/Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm

    r254878 r255116  
    195195}
    196196
    197 void MediaSessionManageriOS::sessionWillEndPlayback(PlatformMediaSession& session)
    198 {
    199     MediaSessionManagerCocoa::sessionWillEndPlayback(session);
     197void MediaSessionManageriOS::sessionWillEndPlayback(PlatformMediaSession& session, DelayCallingUpdateNowPlaying delayCallingUpdateNowPlaying)
     198{
     199    MediaSessionManagerCocoa::sessionWillEndPlayback(session, delayCallingUpdateNowPlaying);
    200200
    201201#if USE(AUDIO_SESSION)

  • trunk/Tools/ChangeLog

    r255082 r255116  
     12020-01-25  Youenn Fablet  <youenn@apple.com>
     2
     3        HTMLMediaElement should not remove the media session at DOM suspension time
     4        https://bugs.webkit.org/show_bug.cgi?id=206661
     5        <rdar://problem/58800787>
     6
     7        Reviewed by Eric Carlson.
     8
     9        * TestWebKitAPI/Tests/WebKitLegacy/ios/ScrollingDoesNotPauseMedia.mm:
     10        (TestWebKitAPI::TEST):
     11        Suspend/resume Active DOM Objects from time to time as would do scrolling.
     12        This allows pending tasks to be executed asynchronously when not scrolling.
     13
    1142020-01-23  Matt Lewis  <jlewis3@apple.com>
    215

  • trunk/Tools/TestWebKitAPI/Tests/WebKitLegacy/ios/ScrollingDoesNotPauseMedia.mm

    r252152 r255116  
    9393
    9494    callOnMainThreadAndWait([&] () mutable {
     95        [mainFrame setTimeoutsPaused:YES];
     96
    9597        DOMHTMLMediaElement* video = (DOMHTMLMediaElement*)[mainFrame.DOMDocument querySelector:@"video"];
    9698        ASSERT_TRUE([video isKindOfClass:[DOMHTMLMediaElement class]]);
    9799
    98100        [video addEventListener:@"playing" listener:testController.get() useCapture:NO];
    99 
    100         [mainFrame setTimeoutsPaused:YES];
    101101        didReceivePlaying = false;
    102102        [video play];
     103
     104        [mainFrame setTimeoutsPaused:NO];
    103105    });
    104106
    … …  
    106108
    107109    callOnMainThreadAndWait([&] () mutable {
     110        [mainFrame setTimeoutsPaused:YES];
     111
    108112        DOMHTMLMediaElement* video = (DOMHTMLMediaElement*)[mainFrame.DOMDocument querySelector:@"video"];
    109113        ASSERT_TRUE([video isKindOfClass:[DOMHTMLMediaElement class]]);
    110114
    111115        [video addEventListener:@"pause" listener:testController.get() useCapture:NO];
     116        didReceivePause = false;
     117        [video pause];
    112118
    113119        [mainFrame setTimeoutsPaused:NO];
    114         didReceivePause = false;
    115         [video pause];
    116120    });
    117121
Note: See TracChangeset for help on using the changeset viewer.