From 691fd661539f80b16cdaa70d6fcd0b7e21aa996e Mon Sep 17 00:00:00 2001 From: Narcis Beleuzu Date: Wed, 23 Sep 2020 15:06:17 +0300 Subject: [PATCH] Backed out 4 changesets (bug 1665527) for wpt leakchecks on setactionhandler.html . CLOSED TREE Backed out changeset 216b96d6a2b3 (bug 1665527) Backed out changeset a683efbf01c1 (bug 1665527) Backed out changeset a18103008464 (bug 1665527) Backed out changeset b97b8759b686 (bug 1665527) --- dom/media/mediacontrol/MediaController.cpp | 14 +-- dom/media/mediacontrol/tests/browser.ini | 1 - ...ontrollable_media_for_active_controller.js | 115 ------------------ ...stop_control_after_media_reaches_to_end.js | 33 +---- dom/media/mediasession/MediaSession.cpp | 46 ++----- dom/media/mediasession/MediaSession.h | 20 ++- 6 files changed, 19 insertions(+), 210 deletions(-) delete mode 100644 dom/media/mediacontrol/tests/browser_remove_controllable_media_for_active_controller.js diff --git a/dom/media/mediacontrol/MediaController.cpp b/dom/media/mediacontrol/MediaController.cpp index 6e80331263d9..102b96057040 100644 --- a/dom/media/mediacontrol/MediaController.cpp +++ b/dom/media/mediacontrol/MediaController.cpp @@ -352,14 +352,7 @@ bool MediaController::ShouldActivateController() const { bool MediaController::ShouldDeactivateController() const { MOZ_ASSERT(!mShutdown); - // If we don't have an active media session and no controlled media exists, - // then we don't need to keep controller active, because there is nothing to - // control. However, if we still have an active media session, then we should - // keep controller active in order to receive media keys even if we don't have - // any controlled media existing, because a website might start other media - // when media session receives media keys. - return !IsAnyMediaBeingControlled() && mIsActive && - !mActiveMediaSessionContextId; + return !IsAnyMediaBeingControlled() && mIsActive; } void MediaController::Activate() { @@ -506,11 +499,6 @@ void MediaController::HandleMetadataChanged( // to use `getMetadata()` to get metadata, because it would throw an error if // we fail to allocate artwork. DispatchAsyncEvent(u"metadatachange"_ns); - // If metadata change is because of resetting active media session, then we - // should check if controller needs to be deactivated. - if (ShouldDeactivateController()) { - Deactivate(); - } } void MediaController::DispatchAsyncEvent(const nsAString& aName) { diff --git a/dom/media/mediacontrol/tests/browser.ini b/dom/media/mediacontrol/tests/browser.ini index 050472608455..f86e94006640 100644 --- a/dom/media/mediacontrol/tests/browser.ini +++ b/dom/media/mediacontrol/tests/browser.ini @@ -39,5 +39,4 @@ support-files = [browser_nosrc_and_error_media.js] [browser_seek_captured_audio.js] [browser_stop_control_after_media_reaches_to_end.js] -[browser_remove_controllable_media_for_active_controller.js] [browser_resume_latest_paused_media.js] diff --git a/dom/media/mediacontrol/tests/browser_remove_controllable_media_for_active_controller.js b/dom/media/mediacontrol/tests/browser_remove_controllable_media_for_active_controller.js deleted file mode 100644 index 06a9b39fcfc8..000000000000 --- a/dom/media/mediacontrol/tests/browser_remove_controllable_media_for_active_controller.js +++ /dev/null @@ -1,115 +0,0 @@ -/* eslint-disable no-undef */ -const PAGE_URL = - "https://example.com/browser/dom/media/mediacontrol/tests/file_non_autoplay.html"; - -const testVideoId = "video"; - -add_task(async function setupTestingPref() { - await SpecialPowers.pushPrefEnv({ - set: [["media.mediacontrol.testingevents.enabled", true]], - }); -}); - -/** - * If an active controller has an active media session, then it can still be - * controlled via media key even if there is no controllable media presents. - * As active media session could still create other controllable media in its - * action handler, it should still receive media key. However, if a controller - * doesn't have an active media session, then it won't be controlled via media - * key when no controllable media presents. - */ -add_task( - async function testControllerWithActiveMediaSessionShouldStillBeActiveWhenNoControllableMediaPresents() { - info(`open media page`); - const tab = await createTabAndLoad(PAGE_URL); - - info(`play media would activate controller and media session`); - await setupMediaSession(tab); - await playMedia(tab, testVideoId); - await checkOrWaitControllerBecomesActive(tab); - - info(`remove playing media so we don't have any controllable media now`); - await removePlayingMedia(tab); - - info(`despite that, controller should still be active`); - await checkOrWaitControllerBecomesActive(tab); - - info(`active media session can still receive media key`); - await ensureActiveMediaSessionReceivedMediaKey(tab); - - info(`remove tab`); - await BrowserTestUtils.removeTab(tab); - } -); - -add_task( - async function testControllerWithoutActiveMediaSessionShouldBecomeInactiveWhenNoControllableMediaPresents() { - info(`open media page`); - const tab = await createTabAndLoad(PAGE_URL); - - info(`play media would activate controller`); - await playMedia(tab, testVideoId); - await checkOrWaitControllerBecomesActive(tab); - - info(`remove playing media so we don't have any controllable media now`); - await removePlayingMedia(tab); - - info(`without having media session, controller should be deactivated`); - await checkOrWaitControllerBecomesInactive(tab); - - info(`remove tab`); - await BrowserTestUtils.removeTab(tab); - } -); - -/** - * The following are helper functions. - */ -function setupMediaSession(tab) { - return SpecialPowers.spawn(tab.linkedBrowser, [], _ => { - // except `play/pause/stop`, set an action handler for arbitrary key in - // order to later verify if the session receives that media key by listening - // to session's `onpositionstatechange`. - content.navigator.mediaSession.setActionHandler("seekforward", _ => { - content.navigator.mediaSession.setPositionState({ - duration: 60, - }); - }); - }); -} - -async function ensureActiveMediaSessionReceivedMediaKey(tab) { - const controller = tab.linkedBrowser.browsingContext.mediaController; - const positionChangePromise = new Promise( - r => (controller.onpositionstatechange = r) - ); - MediaControlService.generateMediaControlKey("seekforward"); - await positionChangePromise; - ok(true, "active media session received media key"); -} - -function removePlayingMedia(tab) { - const controller = tab.linkedBrowser.browsingContext.mediaController; - return Promise.all([ - new Promise(r => (controller.onplaybackstatechange = r)), - SpecialPowers.spawn(tab.linkedBrowser, [testVideoId], Id => { - content.document.getElementById(Id).remove(); - }), - ]); -} - -async function checkOrWaitControllerBecomesActive(tab) { - const controller = tab.linkedBrowser.browsingContext.mediaController; - if (!controller.isActive) { - await new Promise(r => (controller.onactivated = r)); - } - ok(controller.isActive, `controller is active`); -} - -async function checkOrWaitControllerBecomesInactive(tab) { - const controller = tab.linkedBrowser.browsingContext.mediaController; - if (controller.isActive) { - await new Promise(r => (controller.ondeactivated = r)); - } - ok(!controller.isActive, `controller is inacitve`); -} diff --git a/dom/media/mediacontrol/tests/browser_stop_control_after_media_reaches_to_end.js b/dom/media/mediacontrol/tests/browser_stop_control_after_media_reaches_to_end.js index 71cd9b07f838..76e4f96fcc21 100644 --- a/dom/media/mediacontrol/tests/browser_stop_control_after_media_reaches_to_end.js +++ b/dom/media/mediacontrol/tests/browser_stop_control_after_media_reaches_to_end.js @@ -10,11 +10,9 @@ add_task(async function setupTestingPref() { /** * This test is used to ensure that we would stop controlling media after it - * reaches to the end when a controller doesn't have an active media session. - * If a controller has an active media session, it would keep active despite - * media reaches to the end. + * reaches to the end. */ -add_task(async function testControllerShouldStopAfterMediaReachesToTheEnd() { +add_task(async function testControlShouldStopAfterMediaReachesToTheEnd() { info(`open media page and play media until the end`); const tab = await createTabAndLoad(PAGE_URL); await Promise.all([ @@ -26,21 +24,6 @@ add_task(async function testControllerShouldStopAfterMediaReachesToTheEnd() { await BrowserTestUtils.removeTab(tab); }); -add_task(async function testControllerWontStopAfterMediaReachesToTheEnd() { - info(`open media page and create media session`); - const tab = await createTabAndLoad(PAGE_URL); - await createMediaSession(tab); - - info(`play media until the end`); - await playMediaUntilItReachesToTheEnd(tab); - - info(`controller is still active because of having active media session`); - await checkControllerIsActive(tab); - - info(`remove tab`); - await BrowserTestUtils.removeTab(tab); -}); - /** * The following are helper functions. */ @@ -94,15 +77,3 @@ function playMediaUntilItReachesToTheEnd(tab) { await new Promise(r => (video.onended = r)); }); } - -function createMediaSession(tab) { - return SpecialPowers.spawn(tab.linkedBrowser, [], _ => { - // simply create a media session, which would become the active media session later. - content.navigator.mediaSession; - }); -} - -function checkControllerIsActive(tab) { - const controller = tab.linkedBrowser.browsingContext.mediaController; - ok(controller.isActive, `controller is active`); -} diff --git a/dom/media/mediasession/MediaSession.cpp b/dom/media/mediasession/MediaSession.cpp index 0d515abb41e8..6b6907dbc58d 100644 --- a/dom/media/mediasession/MediaSession.cpp +++ b/dom/media/mediasession/MediaSession.cpp @@ -27,40 +27,12 @@ NS_IMPL_CYCLE_COLLECTING_ADDREF(MediaSession) NS_IMPL_CYCLE_COLLECTING_RELEASE(MediaSession) NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(MediaSession) NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY - NS_INTERFACE_MAP_ENTRY(nsIDocumentActivity) + NS_INTERFACE_MAP_ENTRY(nsISupports) NS_INTERFACE_MAP_END -MediaSession::MediaSession(nsPIDOMWindowInner* aParent) - : mParent(aParent), mDoc(mParent->GetExtantDoc()) { +MediaSession::MediaSession(nsPIDOMWindowInner* aParent) : mParent(aParent) { MOZ_ASSERT(mParent); - MOZ_ASSERT(mDoc); - mDoc->RegisterActivityObserver(this); - if (mDoc->IsCurrentActiveDocument()) { - SetMediaSessionDocStatus(SessionDocStatus::eActive); - } -} - -void MediaSession::Shutdown() { - mDoc->UnregisterActivityObserver(this); - SetMediaSessionDocStatus(SessionDocStatus::eInactive); -} - -void MediaSession::NotifyOwnerDocumentActivityChanged() { - const bool isDocActive = mDoc->IsCurrentActiveDocument(); - LOG("Document activity changed, isActive=%d", isDocActive); - if (isDocActive) { - SetMediaSessionDocStatus(SessionDocStatus::eActive); - } else { - SetMediaSessionDocStatus(SessionDocStatus::eInactive); - } -} - -void MediaSession::SetMediaSessionDocStatus(SessionDocStatus aState) { - if (mSessionDocState == aState) { - return; - } - mSessionDocState = aState; - NotifyMediaSessionDocStatus(mSessionDocState); + NotifyMediaSessionStatus(SessionStatus::eCreated); } nsPIDOMWindowInner* MediaSession::GetParentObject() const { return mParent; } @@ -73,14 +45,12 @@ JSObject* MediaSession::WrapObject(JSContext* aCx, MediaMetadata* MediaSession::GetMetadata() const { return mMediaMetadata; } void MediaSession::SetMetadata(MediaMetadata* aMetadata) { - MOZ_ASSERT(mSessionDocState == SessionDocStatus::eActive); mMediaMetadata = aMetadata; NotifyMetadataUpdated(); } void MediaSession::SetPlaybackState( const MediaSessionPlaybackState& aPlaybackState) { - MOZ_ASSERT(mSessionDocState == SessionDocStatus::eActive); if (mDeclaredPlaybackState == aPlaybackState) { return; } @@ -100,7 +70,6 @@ MediaSessionPlaybackState MediaSession::PlaybackState() const { void MediaSession::SetActionHandler(MediaSessionAction aAction, MediaSessionActionHandler* aHandler) { - MOZ_ASSERT(mSessionDocState == SessionDocStatus::eActive); MOZ_ASSERT(size_t(aAction) < ArrayLength(mActionHandlers)); // If the media session changes its supported action, then we would propagate // this information to the chrome process in order to run the media session @@ -123,7 +92,6 @@ MediaSessionActionHandler* MediaSession::GetActionHandler( void MediaSession::SetPositionState(const MediaPositionState& aState, ErrorResult& aRv) { - MOZ_ASSERT(mSessionDocState == SessionDocStatus::eActive); // https://w3c.github.io/mediasession/#dom-mediasession-setpositionstate // If the state is an empty dictionary then clear the position state. if (!aState.IsAnyMemberPresent()) { @@ -222,7 +190,11 @@ bool MediaSession::IsActive() const { return *activeSessionContextId == currentBC->Id(); } -void MediaSession::NotifyMediaSessionDocStatus(SessionDocStatus aState) { +void MediaSession::Shutdown() { + NotifyMediaSessionStatus(SessionStatus::eDestroyed); +} + +void MediaSession::NotifyMediaSessionStatus(SessionStatus aState) { RefPtr currentBC = GetParentObject()->GetBrowsingContext(); MOZ_ASSERT(currentBC, "Update session status after context destroyed!"); @@ -230,7 +202,7 @@ void MediaSession::NotifyMediaSessionDocStatus(SessionDocStatus aState) { if (!updater) { return; } - if (aState == SessionDocStatus::eActive) { + if (aState == SessionStatus::eCreated) { updater->NotifySessionCreated(currentBC->Id()); } else { updater->NotifySessionDestroyed(currentBC->Id()); diff --git a/dom/media/mediasession/MediaSession.h b/dom/media/mediasession/MediaSession.h index 6b8174577565..82beee7e8ad8 100644 --- a/dom/media/mediasession/MediaSession.h +++ b/dom/media/mediasession/MediaSession.h @@ -15,7 +15,6 @@ #include "mozilla/ErrorResult.h" #include "mozilla/EnumeratedArray.h" #include "nsCycleCollectionParticipant.h" -#include "nsIDocumentActivity.h" #include "nsWrapperCache.h" class nsPIDOMWindowInner; @@ -36,12 +35,11 @@ struct PositionState { double mLastReportedPlaybackPosition; }; -class MediaSession final : public nsIDocumentActivity, public nsWrapperCache { +class MediaSession final : public nsISupports, public nsWrapperCache { public: // Ref counting and cycle collection NS_DECL_CYCLE_COLLECTING_ISUPPORTS NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(MediaSession) - NS_DECL_NSIDOCUMENTACTIVITY explicit MediaSession(nsPIDOMWindowInner* aParent); @@ -80,18 +78,16 @@ class MediaSession final : public nsIDocumentActivity, public nsWrapperCache { bool IsActive() const; private: - // When the document which media session belongs to is going to be destroyed, - // or is in the bfcache, then the session would be inactive. Otherwise, it's - // active all the time. - enum class SessionDocStatus : bool { - eInactive = false, - eActive = true, + // Propagate media context status to the media session controller in the + // chrome process when we create or destroy the media session. + enum class SessionStatus : bool { + eDestroyed = false, + eCreated = true, }; - void SetMediaSessionDocStatus(SessionDocStatus aState); // These methods are used to propagate media session's status to the chrome // process. - void NotifyMediaSessionDocStatus(SessionDocStatus aState); + void NotifyMediaSessionStatus(SessionStatus aState); void NotifyMetadataUpdated(); void NotifyEnableSupportedAction(MediaSessionAction aAction); void NotifyDisableSupportedAction(MediaSessionAction aAction); @@ -118,8 +114,6 @@ class MediaSession final : public nsIDocumentActivity, public nsWrapperCache { MediaSessionPlaybackState::None; Maybe mPositionState; - RefPtr mDoc; - SessionDocStatus mSessionDocState = SessionDocStatus::eInactive; }; } // namespace dom