[webkit-changes] [255116] trunk

Sat, 25 Jan 2020 04:28:18 -0800

Title: [255116] trunk
Revision
255116
Author
you...@apple.com
Date
2020-01-25 04:27:39 -0800 (Sat, 25 Jan 2020)

Log 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.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (255115 => 255116)


--- trunk/Source/WebCore/ChangeLog	2020-01-25 04:24:10 UTC (rev 255115)
+++ trunk/Source/WebCore/ChangeLog	2020-01-25 12:27:39 UTC (rev 255116)
@@ -1,3 +1,44 @@
+2020-01-25  youenn fablet  <you...@apple.com>
+
+        HTMLMediaElement should not remove the media session at DOM suspension time
+        https://bugs.webkit.org/show_bug.cgi?id=206661
+        <rdar://problem/58800787>
+
+        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):
+
 2020-01-24  Jack Lee  <shihchieh_...@apple.com>
 
         Null Ptr Deref READ @ WebCore::RenderMultiColumnFlow::lastMultiColumnSet const

Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (255115 => 255116)


--- trunk/Source/WebCore/html/HTMLMediaElement.cpp	2020-01-25 04:24:10 UTC (rev 255115)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp	2020-01-25 12:27:39 UTC (rev 255116)
@@ -2326,6 +2326,13 @@
 
 void HTMLMediaElement::mediaPlayerReadyStateChanged()
 {
+    if (isSuspended()) {
+        queueTaskKeepingObjectAlive(*this, TaskSource::MediaElement, [this] {
+            mediaPlayerReadyStateChanged();
+        });
+        return;
+    }
+
     beginProcessingMediaPlayerCallback();
 
     setReadyState(m_player->readyState());
@@ -5626,7 +5633,7 @@
     // Stop the playback without generating events
     setPlaying(false);
     pauseAndUpdatePlayStateImmediately();
-    m_mediaSession->stopSession();
+    m_mediaSession->clientWillBeDOMSuspended();
 
     setAutoplayEventPlaybackState(AutoplayEventPlaybackState::None);

Modified: trunk/Source/WebCore/platform/audio/PlatformMediaSession.cpp (255115 => 255116)


--- trunk/Source/WebCore/platform/audio/PlatformMediaSession.cpp	2020-01-25 04:24:10 UTC (rev 255115)
+++ trunk/Source/WebCore/platform/audio/PlatformMediaSession.cpp	2020-01-25 12:27:39 UTC (rev 255116)
@@ -224,7 +224,7 @@
     return true;
 }
 
-bool PlatformMediaSession::clientWillPausePlayback()
+bool PlatformMediaSession::processClientWillPausePlayback(DelayCallingUpdateNowPlaying shouldDelayCallingUpdateNowPlaying)
 {
     if (m_notifyingClient)
         return true;
@@ -237,10 +237,20 @@
     }
     
     setState(Paused);
-    PlatformMediaSessionManager::sharedManager().sessionWillEndPlayback(*this);
+    PlatformMediaSessionManager::sharedManager().sessionWillEndPlayback(*this, shouldDelayCallingUpdateNowPlaying);
     return true;
 }
 
+bool PlatformMediaSession::clientWillPausePlayback()
+{
+    return processClientWillPausePlayback(DelayCallingUpdateNowPlaying::No);
+}
+
+void PlatformMediaSession::clientWillBeDOMSuspended()
+{
+    processClientWillPausePlayback(DelayCallingUpdateNowPlaying::Yes);
+}
+
 void PlatformMediaSession::pauseSession()
 {
     INFO_LOG(LOGIDENTIFIER);

Modified: trunk/Source/WebCore/platform/audio/PlatformMediaSession.h (255115 => 255116)


--- trunk/Source/WebCore/platform/audio/PlatformMediaSession.h	2020-01-25 04:24:10 UTC (rev 255115)
+++ trunk/Source/WebCore/platform/audio/PlatformMediaSession.h	2020-01-25 12:27:39 UTC (rev 255116)
@@ -41,6 +41,7 @@
 class Document;
 class MediaPlaybackTarget;
 class PlatformMediaSessionClient;
+enum class DelayCallingUpdateNowPlaying { No, Yes };
 
 class PlatformMediaSession
     : public CanMakeWeakPtr<PlatformMediaSession>
@@ -113,6 +114,8 @@
     virtual bool clientWillBeginPlayback();
     virtual bool clientWillPausePlayback();
 
+    void clientWillBeDOMSuspended();
+
     void pauseSession();
     void stopSession();
 
@@ -199,6 +202,8 @@
     PlatformMediaSessionClient& client() const { return m_client; }
 
 private:
+    bool processClientWillPausePlayback(DelayCallingUpdateNowPlaying);
+
     PlatformMediaSessionClient& m_client;
     State m_state;
     State m_stateToRestore;

Modified: trunk/Source/WebCore/platform/audio/PlatformMediaSessionManager.cpp (255115 => 255116)


--- trunk/Source/WebCore/platform/audio/PlatformMediaSessionManager.cpp	2020-01-25 04:24:10 UTC (rev 255115)
+++ trunk/Source/WebCore/platform/audio/PlatformMediaSessionManager.cpp	2020-01-25 12:27:39 UTC (rev 255116)
@@ -235,7 +235,7 @@
     return true;
 }
     
-void PlatformMediaSessionManager::sessionWillEndPlayback(PlatformMediaSession& session)
+void PlatformMediaSessionManager::sessionWillEndPlayback(PlatformMediaSession& session, DelayCallingUpdateNowPlaying)
 {
     ALWAYS_LOG(LOGIDENTIFIER, session.logIdentifier());

Modified: trunk/Source/WebCore/platform/audio/PlatformMediaSessionManager.h (255115 => 255116)


--- trunk/Source/WebCore/platform/audio/PlatformMediaSessionManager.h	2020-01-25 04:24:10 UTC (rev 255115)
+++ trunk/Source/WebCore/platform/audio/PlatformMediaSessionManager.h	2020-01-25 12:27:39 UTC (rev 255116)
@@ -113,7 +113,8 @@
     virtual void resetRestrictions();
 
     virtual bool sessionWillBeginPlayback(PlatformMediaSession&);
-    virtual void sessionWillEndPlayback(PlatformMediaSession&);
+
+    virtual void sessionWillEndPlayback(PlatformMediaSession&, DelayCallingUpdateNowPlaying);
     virtual void sessionStateChanged(PlatformMediaSession&);
     virtual void sessionDidEndRemoteScrubbing(const PlatformMediaSession&) { };
     virtual void clientCharacteristicsChanged(PlatformMediaSession&) { }

Modified: trunk/Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.h (255115 => 255116)


--- trunk/Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.h	2020-01-25 04:24:10 UTC (rev 255115)
+++ trunk/Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.h	2020-01-25 12:27:39 UTC (rev 255116)
@@ -53,7 +53,7 @@
     void removeSession(PlatformMediaSession&) override;
     
     bool sessionWillBeginPlayback(PlatformMediaSession&) override;
-    void sessionWillEndPlayback(PlatformMediaSession&) override;
+    void sessionWillEndPlayback(PlatformMediaSession&, DelayCallingUpdateNowPlaying) override;
     void sessionDidEndRemoteScrubbing(const PlatformMediaSession&) override;
     void clientCharacteristicsChanged(PlatformMediaSession&) override;
     void sessionCanProduceAudioChanged() override;

Modified: trunk/Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm (255115 => 255116)


--- trunk/Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm	2020-01-25 04:24:10 UTC (rev 255115)
+++ trunk/Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm	2020-01-25 12:27:39 UTC (rev 255116)
@@ -161,10 +161,14 @@
     scheduleUpdateNowPlayingInfo();
 }
 
-void MediaSessionManagerCocoa::sessionWillEndPlayback(PlatformMediaSession& session)
+void MediaSessionManagerCocoa::sessionWillEndPlayback(PlatformMediaSession& session, DelayCallingUpdateNowPlaying delayCallingUpdateNowPlaying)
 {
-    PlatformMediaSessionManager::sessionWillEndPlayback(session);
-    updateNowPlayingInfo();
+    PlatformMediaSessionManager::sessionWillEndPlayback(session, delayCallingUpdateNowPlaying);
+    if (delayCallingUpdateNowPlaying == DelayCallingUpdateNowPlaying::No) {
+        updateNowPlayingInfo();
+        return;
+    }
+    scheduleUpdateNowPlayingInfo();
 }
 
 void MediaSessionManagerCocoa::clientCharacteristicsChanged(PlatformMediaSession& session)

Modified: trunk/Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.h (255115 => 255116)


--- trunk/Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.h	2020-01-25 04:24:10 UTC (rev 255115)
+++ trunk/Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.h	2020-01-25 12:27:39 UTC (rev 255116)
@@ -63,7 +63,7 @@
 
     void configureWireLessTargetMonitoring() override;
     void providePresentingApplicationPIDIfNecessary() final;
-    void sessionWillEndPlayback(PlatformMediaSession&) final;
+    void sessionWillEndPlayback(PlatformMediaSession&, DelayCallingUpdateNowPlaying) final;
 
 #if !RELEASE_LOG_DISABLED
     const char* logClassName() const final { return "MediaSessionManageriOS"; }

Modified: trunk/Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm (255115 => 255116)


--- trunk/Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm	2020-01-25 04:24:10 UTC (rev 255115)
+++ trunk/Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm	2020-01-25 12:27:39 UTC (rev 255116)
@@ -194,9 +194,9 @@
 #endif
 }
 
-void MediaSessionManageriOS::sessionWillEndPlayback(PlatformMediaSession& session)
+void MediaSessionManageriOS::sessionWillEndPlayback(PlatformMediaSession& session, DelayCallingUpdateNowPlaying delayCallingUpdateNowPlaying)
 {
-    MediaSessionManagerCocoa::sessionWillEndPlayback(session);
+    MediaSessionManagerCocoa::sessionWillEndPlayback(session, delayCallingUpdateNowPlaying);
 
 #if USE(AUDIO_SESSION)
     if (isApplicationInBackground() && !anyOfSessions([] (auto& session) { return session.state() == PlatformMediaSession::Playing; }))

Modified: trunk/Tools/ChangeLog (255115 => 255116)


--- trunk/Tools/ChangeLog	2020-01-25 04:24:10 UTC (rev 255115)
+++ trunk/Tools/ChangeLog	2020-01-25 12:27:39 UTC (rev 255116)
@@ -1,3 +1,16 @@
+2020-01-25  Youenn Fablet  <you...@apple.com>
+
+        HTMLMediaElement should not remove the media session at DOM suspension time
+        https://bugs.webkit.org/show_bug.cgi?id=206661
+        <rdar://problem/58800787>
+
+        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.
+
 2020-01-23  Matt Lewis  <jlew...@apple.com>
 
         Remove Apple windows 7 queues.

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitLegacy/ios/ScrollingDoesNotPauseMedia.mm (255115 => 255116)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitLegacy/ios/ScrollingDoesNotPauseMedia.mm	2020-01-25 04:24:10 UTC (rev 255115)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitLegacy/ios/ScrollingDoesNotPauseMedia.mm	2020-01-25 12:27:39 UTC (rev 255116)
@@ -92,27 +92,31 @@
     Util::run(&gotMainFrame);
 
     callOnMainThreadAndWait([&] () mutable {
+        [mainFrame setTimeoutsPaused:YES];
+
         DOMHTMLMediaElement* video = (DOMHTMLMediaElement*)[mainFrame.DOMDocument querySelector:@"video"];
         ASSERT_TRUE([video isKindOfClass:[DOMHTMLMediaElement class]]);
 
         [video addEventListener:@"playing" listener:testController.get() useCapture:NO];
-
-        [mainFrame setTimeoutsPaused:YES];
         didReceivePlaying = false;
         [video play];
+
+        [mainFrame setTimeoutsPaused:NO];
     });
 
     Util::run(&didReceivePlaying);
 
     callOnMainThreadAndWait([&] () mutable {
+        [mainFrame setTimeoutsPaused:YES];
+
         DOMHTMLMediaElement* video = (DOMHTMLMediaElement*)[mainFrame.DOMDocument querySelector:@"video"];
         ASSERT_TRUE([video isKindOfClass:[DOMHTMLMediaElement class]]);
 
         [video addEventListener:@"pause" listener:testController.get() useCapture:NO];
+        didReceivePause = false;
+        [video pause];
 
         [mainFrame setTimeoutsPaused:NO];
-        didReceivePause = false;
-        [video pause];
     });
 
     Util::run(&didReceivePause);

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to