From 2ae724e7508ca5547ce939a8a7d23bf79c72ceaf Mon Sep 17 00:00:00 2001 From: alwu Date: Wed, 13 May 2020 22:08:01 +0000 Subject: [PATCH] Bug 1633010 - part6 : use `IMediaController` if we only want to access control related methods r=chunmin This patch will do : - create an interface `IMediaController` including only control related methods The advantage of doing so : - It's clear to use `IMediaController` as the only surface to control media - explictly restrict which functions are available which can avoid using control related functions on those situations Differential Revision: https://phabricator.services.mozilla.com/D73490 --- dom/media/mediacontrol/AudioFocusManager.cpp | 5 +- dom/media/mediacontrol/AudioFocusManager.h | 8 +-- .../mediacontrol/MediaControlKeysEvent.cpp | 4 +- dom/media/mediacontrol/MediaController.cpp | 10 ++-- dom/media/mediacontrol/MediaController.h | 50 ++++++++++++++----- .../mediasession/MediaSessionController.h | 1 - 6 files changed, 54 insertions(+), 24 deletions(-) diff --git a/dom/media/mediacontrol/AudioFocusManager.cpp b/dom/media/mediacontrol/AudioFocusManager.cpp index e8ecffa744a3..e95f8ddada28 100644 --- a/dom/media/mediacontrol/AudioFocusManager.cpp +++ b/dom/media/mediacontrol/AudioFocusManager.cpp @@ -4,6 +4,7 @@ #include "AudioFocusManager.h" +#include "MediaController.h" #include "MediaControlUtils.h" #include "MediaControlService.h" #include "mozilla/dom/CanonicalBrowsingContext.h" @@ -18,7 +19,7 @@ namespace mozilla { namespace dom { -void AudioFocusManager::RequestAudioFocus(MediaController* aController) { +void AudioFocusManager::RequestAudioFocus(IMediaController* aController) { MOZ_ASSERT(aController); if (mOwningFocusControllers.Contains(aController)) { return; @@ -28,7 +29,7 @@ void AudioFocusManager::RequestAudioFocus(MediaController* aController) { mOwningFocusControllers.AppendElement(aController); } -void AudioFocusManager::RevokeAudioFocus(MediaController* aController) { +void AudioFocusManager::RevokeAudioFocus(IMediaController* aController) { MOZ_ASSERT(aController); if (!mOwningFocusControllers.Contains(aController)) { return; diff --git a/dom/media/mediacontrol/AudioFocusManager.h b/dom/media/mediacontrol/AudioFocusManager.h index f4798169779f..5c9bacc4f6c4 100644 --- a/dom/media/mediacontrol/AudioFocusManager.h +++ b/dom/media/mediacontrol/AudioFocusManager.h @@ -11,7 +11,7 @@ namespace mozilla { namespace dom { -class MediaController; +class IMediaController; class MediaControlService; /** @@ -25,8 +25,8 @@ class MediaControlService; */ class AudioFocusManager { public: - void RequestAudioFocus(MediaController* aController); - void RevokeAudioFocus(MediaController* aController); + void RequestAudioFocus(IMediaController* aController); + void RevokeAudioFocus(IMediaController* aController); explicit AudioFocusManager() = default; ~AudioFocusManager() = default; @@ -37,7 +37,7 @@ class AudioFocusManager { friend class MediaControlService; void ClearFocusControllersIfNeeded(); - nsTArray> mOwningFocusControllers; + nsTArray> mOwningFocusControllers; }; } // namespace dom diff --git a/dom/media/mediacontrol/MediaControlKeysEvent.cpp b/dom/media/mediacontrol/MediaControlKeysEvent.cpp index 10573f7362d7..4c5f9e04b8ce 100644 --- a/dom/media/mediacontrol/MediaControlKeysEvent.cpp +++ b/dom/media/mediacontrol/MediaControlKeysEvent.cpp @@ -32,7 +32,7 @@ void MediaControlKeysHandler::OnKeyPressed(MediaControlKeysEvent aKeyEvent) { RefPtr service = MediaControlService::GetService(); MOZ_ASSERT(service); - RefPtr controller = service->GetMainController(); + RefPtr controller = service->GetMainController(); if (!controller) { return; } @@ -48,7 +48,7 @@ void MediaControlKeysHandler::OnKeyPressed(MediaControlKeysEvent aKeyEvent) { controller->Pause(); return; case MediaControlKeysEvent::ePlayPause: { - if (controller->GetState() == MediaSessionPlaybackState::Playing) { + if (controller->IsPlaying()) { controller->Pause(); } else { controller->Play(); diff --git a/dom/media/mediacontrol/MediaController.cpp b/dom/media/mediacontrol/MediaController.cpp index 65959526465a..63357bc854b0 100644 --- a/dom/media/mediacontrol/MediaController.cpp +++ b/dom/media/mediacontrol/MediaController.cpp @@ -84,6 +84,12 @@ void MediaController::Stop() { MediaControlKeysEvent::eStop); } +uint64_t MediaController::Id() const { return mTopLevelBCId; } + +bool MediaController::IsAudible() const { return IsMediaAudible(); } + +bool MediaController::IsPlaying() const { return IsMediaPlaying(); } + void MediaController::UpdateMediaControlKeysEventToContentMediaIfNeeded( MediaControlKeysEvent aEvent) { // There is no controlled media existing or controller has been shutdown, we @@ -98,7 +104,7 @@ void MediaController::UpdateMediaControlKeysEventToContentMediaIfNeeded( RefPtr context = mActiveMediaSessionContextId ? BrowsingContext::Get(*mActiveMediaSessionContextId) - : BrowsingContext::Get(mTopLevelBCId); + : BrowsingContext::Get(Id()); if (context && !context->IsDiscarded()) { context->Canonical()->UpdateMediaControlKeysEvent(aEvent); } @@ -216,7 +222,5 @@ bool MediaController::IsInPictureInPictureMode() const { return mIsInPictureInPictureMode; } -bool MediaController::IsAudible() const { return IsMediaAudible(); } - } // namespace dom } // namespace mozilla diff --git a/dom/media/mediacontrol/MediaController.h b/dom/media/mediacontrol/MediaController.h index e5d7ffe6f5b8..5eee0a600f84 100644 --- a/dom/media/mediacontrol/MediaController.h +++ b/dom/media/mediacontrol/MediaController.h @@ -21,6 +21,30 @@ namespace dom { class BrowsingContext; enum class MediaControlKeysEvent : uint32_t; +/** + * IMediaController is an interface which includes control related methods and + * methods used to know its playback state. + */ +class IMediaController { + public: + NS_INLINE_DECL_PURE_VIRTUAL_REFCOUNTING + + // Focus the window currently playing media. + virtual void Focus() = 0; + virtual void Play() = 0; + virtual void Pause() = 0; + virtual void Stop() = 0; + virtual void PrevTrack() = 0; + virtual void NextTrack() = 0; + virtual void SeekBackward() = 0; + virtual void SeekForward() = 0; + + // Return the ID of the top level browsing context within a tab. + virtual uint64_t Id() const = 0; + virtual bool IsAudible() const = 0; + virtual bool IsPlaying() const = 0; +}; + /** * MediaController is a class, which is used to control all media within a tab. * It can only be used in Chrome process and the controlled media are usually @@ -46,22 +70,26 @@ enum class MediaControlKeysEvent : uint32_t; * controller from `MediaControlService`. */ class MediaController final - : public MediaSessionController, + : public IMediaController, + public MediaSessionController, public LinkedListElement> { public: NS_INLINE_DECL_THREADSAFE_REFCOUNTING(MediaController, override); explicit MediaController(uint64_t aContextId); - // Focus the window currently playing media. - void Focus(); - void Play(); - void Pause(); - void Stop(); - void PrevTrack(); - void NextTrack(); - void SeekBackward(); - void SeekForward(); + // IMediaController's methods + void Focus() override; + void Play() override; + void Pause() override; + void Stop() override; + void PrevTrack() override; + void NextTrack() override; + void SeekBackward() override; + void SeekForward() override; + uint64_t Id() const override; + bool IsAudible() const override; + bool IsPlaying() const override; // IMediaInfoUpdater's methods void NotifyMediaPlaybackChanged(uint64_t aBrowsingContextId, @@ -78,8 +106,6 @@ class MediaController final // then calling any its method won't take any effect. void Shutdown(); - bool IsAudible() const; - private: ~MediaController(); diff --git a/dom/media/mediasession/MediaSessionController.h b/dom/media/mediasession/MediaSessionController.h index b97a1f1389c5..86792a75b412 100644 --- a/dom/media/mediasession/MediaSessionController.h +++ b/dom/media/mediasession/MediaSessionController.h @@ -111,7 +111,6 @@ class MediaSessionController : public IMediaInfoUpdater { // it has already set its metadata. Otherwise, return default media metadata // which is based on website's title and favicon. MediaMetadataBase GetCurrentMediaMetadata() const; - uint64_t Id() const { return mTopLevelBCId; } bool IsMediaAudible() const; bool IsMediaPlaying() const;