Bug 1605769 - part1 : use `PlaybackState` in media controller and add a method to monitor its change r=MeFisto94
authoralwu <alwu@mozilla.com>
Thu, 09 Jan 2020 04:42:26 +0000
changeset 509768 914bdf2c92e24eb1a7eaf8952bfd44e189e9ef18
parent 509767 6868755789c3a93ec277953166a738c32b471d92
child 509769 2aff5b65e5279a99235d182b96662c7191ac4de4
push id37004
push usershindli@mozilla.com
push dateSat, 11 Jan 2020 09:48:29 +0000
treeherdermozilla-central@fb64636dad2c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMeFisto94
bugs1605769
milestone74.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1605769 - part1 : use `PlaybackState` in media controller and add a method to monitor its change r=MeFisto94 We use `PlaybackState` to replace `boolean` which can clearly indicate what controller's current state is and introduce a new method `PlayStateChangedEvent()` which can be used to monitor the play state change of the media controller. Differential Revision: https://phabricator.services.mozilla.com/D58174
dom/media/mediacontrol/MediaControlKeysEvent.cpp
dom/media/mediacontrol/MediaController.cpp
dom/media/mediacontrol/MediaController.h
dom/media/mediacontrol/tests/gtest/TestMediaController.cpp
--- a/dom/media/mediacontrol/MediaControlKeysEvent.cpp
+++ b/dom/media/mediacontrol/MediaControlKeysEvent.cpp
@@ -31,29 +31,31 @@ void MediaControlKeysHandler::OnKeyPress
 
   RefPtr<MediaControlService> service = MediaControlService::GetService();
   MOZ_ASSERT(service);
   RefPtr<MediaController> controller = service->GetLastAddedController();
   if (!controller) {
     return;
   }
 
+  const bool isControllerPlaying =
+      controller->GetState() == PlaybackState::ePlaying;
   switch (aKeyEvent) {
     case MediaControlKeysEvent::ePlay:
-      if (!controller->IsPlaying()) {
+      if (!isControllerPlaying) {
         controller->Play();
       }
       return;
     case MediaControlKeysEvent::ePause:
-      if (controller->IsPlaying()) {
+      if (isControllerPlaying) {
         controller->Pause();
       }
       return;
     case MediaControlKeysEvent::ePlayPause: {
-      if (controller->IsPlaying()) {
+      if (isControllerPlaying) {
         controller->Pause();
       } else {
         controller->Play();
       }
       return;
     }
     case MediaControlKeysEvent::ePrevTrack:
     case MediaControlKeysEvent::eNextTrack:
--- a/dom/media/mediacontrol/MediaController.cpp
+++ b/dom/media/mediacontrol/MediaController.cpp
@@ -30,31 +30,31 @@ MediaController::MediaController(uint64_
 
 MediaController::~MediaController() {
   LOG("Destroy controller %" PRId64, Id());
   MOZ_DIAGNOSTIC_ASSERT(!mControlledMediaNum);
 };
 
 void MediaController::Play() {
   LOG("Play");
-  mIsPlaying = true;
+  SetPlayState(PlaybackState::ePlaying);
   UpdateMediaControlKeysEventToContentMediaIfNeeded(
       MediaControlKeysEvent::ePlay);
 }
 
 void MediaController::Pause() {
   LOG("Pause");
-  mIsPlaying = false;
+  SetPlayState(PlaybackState::ePaused);
   UpdateMediaControlKeysEventToContentMediaIfNeeded(
       MediaControlKeysEvent::ePause);
 }
 
 void MediaController::Stop() {
   LOG("Stop");
-  mIsPlaying = false;
+  SetPlayState(PlaybackState::eStopped);
   UpdateMediaControlKeysEventToContentMediaIfNeeded(
       MediaControlKeysEvent::eStop);
 }
 
 void MediaController::UpdateMediaControlKeysEventToContentMediaIfNeeded(
     MediaControlKeysEvent aEvent) {
   // There is no controlled media existing, we have no need to update media
   // action to the content process.
@@ -63,17 +63,17 @@ void MediaController::UpdateMediaControl
   }
   RefPtr<BrowsingContext> context = BrowsingContext::Get(mBrowsingContextId);
   if (context) {
     context->Canonical()->UpdateMediaControlKeysEvent(aEvent);
   }
 }
 
 void MediaController::Shutdown() {
-  mIsPlaying = false;
+  SetPlayState(PlaybackState::eStopped);
   mControlledMediaNum = 0;
   RefPtr<MediaControlService> service = MediaControlService::GetService();
   MOZ_ASSERT(service);
   service->GetAudioFocusManager().RevokeAudioFocus(Id());
 }
 
 void MediaController::NotifyMediaStateChanged(ControlledMediaState aState) {
   if (aState == ControlledMediaState::eStarted) {
@@ -118,27 +118,27 @@ void MediaController::IncreasePlayingCon
   MOZ_ASSERT(mPlayingControlledMediaNum >= 0);
   mPlayingControlledMediaNum++;
   LOG("Increase playing controlled media num to %" PRId64,
       mPlayingControlledMediaNum);
   MOZ_ASSERT(mPlayingControlledMediaNum <= mControlledMediaNum,
              "The number of playing media should not exceed the number of "
              "controlled media!");
   if (mPlayingControlledMediaNum == 1) {
-    mIsPlaying = true;
+    SetPlayState(PlaybackState::ePlaying);
   }
 }
 
 void MediaController::DecreasePlayingControlledMediaNum() {
   mPlayingControlledMediaNum--;
   LOG("Decrease playing controlled media num to %" PRId64,
       mPlayingControlledMediaNum);
   MOZ_ASSERT(mPlayingControlledMediaNum >= 0);
   if (mPlayingControlledMediaNum == 0) {
-    mIsPlaying = false;
+    SetPlayState(PlaybackState::ePaused);
   }
 }
 
 // TODO : Use watchable to moniter mControlledMediaNum
 void MediaController::Activate() {
   RefPtr<MediaControlService> service = MediaControlService::GetService();
   MOZ_ASSERT(service);
   service->AddMediaController(this);
@@ -146,20 +146,31 @@ void MediaController::Activate() {
 
 void MediaController::Deactivate() {
   RefPtr<MediaControlService> service = MediaControlService::GetService();
   MOZ_ASSERT(service);
   service->RemoveMediaController(this);
   service->GetAudioFocusManager().RevokeAudioFocus(Id());
 }
 
+void MediaController::SetPlayState(PlaybackState aState) {
+  if (mState == aState) {
+    return;
+  }
+  LOG("SetPlayState : '%s'", ToPlaybackStateEventStr(aState));
+  mState = aState;
+  mPlaybackStateChangedEvent.Notify(mState);
+}
+
+PlaybackState MediaController::GetState() const { return mState; }
+
 uint64_t MediaController::Id() const { return mBrowsingContextId; }
 
-bool MediaController::IsPlaying() const { return mIsPlaying; }
-
-bool MediaController::IsAudible() const { return IsPlaying() && mAudible; }
+bool MediaController::IsAudible() const {
+  return mState == PlaybackState::ePlaying && mAudible;
+}
 
 uint64_t MediaController::ControlledMediaNum() const {
   return mControlledMediaNum;
 }
 
 }  // namespace dom
 }  // namespace mozilla
--- a/dom/media/mediacontrol/MediaController.h
+++ b/dom/media/mediacontrol/MediaController.h
@@ -3,16 +3,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef DOM_MEDIA_MEDIACONTROL_MEDIACONTROLLER_H_
 #define DOM_MEDIA_MEDIACONTROL_MEDIACONTROLLER_H_
 
 #include "ContentMediaController.h"
+#include "MediaEventSource.h"
 #include "nsDataHashtable.h"
 #include "nsISupportsImpl.h"
 
 namespace mozilla {
 namespace dom {
 
 class BrowsingContext;
 enum class MediaControlKeysEvent : uint32_t;
@@ -58,19 +59,23 @@ class MediaController final {
   explicit MediaController(uint64_t aContextId);
 
   void Play();
   void Pause();
   void Stop();
   void Shutdown();
 
   uint64_t Id() const;
-  bool IsPlaying() const;
   bool IsAudible() const;
   uint64_t ControlledMediaNum() const;
+  PlaybackState GetState() const;
+
+  MediaEventSource<PlaybackState>& PlaybackStateChangedEvent() {
+    return mPlaybackStateChangedEvent;
+  }
 
   // These methods are only being used to notify the state changes of controlled
   // media in ContentParent or MediaControlUtils.
   void NotifyMediaStateChanged(ControlledMediaState aState);
   void NotifyMediaAudibleChanged(bool aAudible);
 
  private:
   ~MediaController();
@@ -80,19 +85,23 @@ class MediaController final {
   void IncreaseControlledMediaNum();
   void DecreaseControlledMediaNum();
   void IncreasePlayingControlledMediaNum();
   void DecreasePlayingControlledMediaNum();
 
   void Activate();
   void Deactivate();
 
+  void SetPlayState(PlaybackState aState);
+
   uint64_t mBrowsingContextId;
-  bool mIsPlaying = false;
   bool mAudible = false;
   int64_t mControlledMediaNum = 0;
   int64_t mPlayingControlledMediaNum = 0;
+
+  PlaybackState mState = PlaybackState::eStopped;
+  MediaEventProducer<PlaybackState> mPlaybackStateChangedEvent;
 };
 
 }  // namespace dom
 }  // namespace mozilla
 
 #endif
--- a/dom/media/mediacontrol/tests/gtest/TestMediaController.cpp
+++ b/dom/media/mediacontrol/tests/gtest/TestMediaController.cpp
@@ -10,17 +10,17 @@ using namespace mozilla::dom;
 
 #define CONTROLLER_ID 0
 
 TEST(MediaController, DefaultValueCheck)
 {
   RefPtr<MediaController> controller = new MediaController(CONTROLLER_ID);
   ASSERT_TRUE(controller->ControlledMediaNum() == 0);
   ASSERT_TRUE(controller->Id() == CONTROLLER_ID);
-  ASSERT_TRUE(!controller->IsPlaying());
+  ASSERT_TRUE(controller->GetState() == PlaybackState::eStopped);
   ASSERT_TRUE(!controller->IsAudible());
 }
 
 TEST(MediaController, NotifyMediaStateChanged)
 {
   RefPtr<MediaController> controller = new MediaController(CONTROLLER_ID);
   ASSERT_TRUE(controller->ControlledMediaNum() == 0);
 
@@ -84,29 +84,29 @@ TEST(MediaController, AlwaysInaudibleIfC
 
   controller->Stop();
   ASSERT_TRUE(!controller->IsAudible());
 }
 
 TEST(MediaController, ChangePlayingStateViaPlayPauseStop)
 {
   RefPtr<MediaController> controller = new MediaController(CONTROLLER_ID);
-  ASSERT_TRUE(!controller->IsPlaying());
+  ASSERT_TRUE(controller->GetState() == PlaybackState::eStopped);
 
   controller->Play();
-  ASSERT_TRUE(controller->IsPlaying());
+  ASSERT_TRUE(controller->GetState() == PlaybackState::ePlaying);
 
   controller->Pause();
-  ASSERT_TRUE(!controller->IsPlaying());
+  ASSERT_TRUE(controller->GetState() == PlaybackState::ePaused);
 
   controller->Play();
-  ASSERT_TRUE(controller->IsPlaying());
+  ASSERT_TRUE(controller->GetState() == PlaybackState::ePlaying);
 
   controller->Stop();
-  ASSERT_TRUE(!controller->IsPlaying());
+  ASSERT_TRUE(controller->GetState() == PlaybackState::eStopped);
 }
 
 class FakeControlledMedia final {
  public:
   explicit FakeControlledMedia(MediaController* aController)
       : mController(aController) {
     mController->NotifyMediaStateChanged(ControlledMediaState::eStarted);
   }
@@ -131,60 +131,58 @@ class FakeControlledMedia final {
  private:
   bool mIsPlaying = false;
   RefPtr<MediaController> mController;
 };
 
 TEST(MediaController, PlayingStateChangeViaControlledMedia)
 {
   RefPtr<MediaController> controller = new MediaController(CONTROLLER_ID);
-  ASSERT_TRUE(!controller->IsPlaying());
 
   // In order to check playing state after FakeControlledMedia destroyed.
   {
     FakeControlledMedia foo(controller);
-    ASSERT_TRUE(!controller->IsPlaying());
+    ASSERT_TRUE(controller->GetState() == PlaybackState::eStopped);
 
     foo.SetPlaying(true);
-    ASSERT_TRUE(controller->IsPlaying());
+    ASSERT_TRUE(controller->GetState() == PlaybackState::ePlaying);
 
     foo.SetPlaying(false);
-    ASSERT_TRUE(!controller->IsPlaying());
+    ASSERT_TRUE(controller->GetState() == PlaybackState::ePaused);
 
     foo.SetPlaying(true);
-    ASSERT_TRUE(controller->IsPlaying());
+    ASSERT_TRUE(controller->GetState() == PlaybackState::ePlaying);
   }
 
   // FakeControlledMedia has been destroyed, no playing media exists.
-  ASSERT_TRUE(!controller->IsPlaying());
+  ASSERT_TRUE(controller->GetState() == PlaybackState::ePaused);
 }
 
 TEST(MediaController, ControllerShouldRemainPlayingIfAnyPlayingMediaExists)
 {
   RefPtr<MediaController> controller = new MediaController(CONTROLLER_ID);
-  ASSERT_TRUE(!controller->IsPlaying());
 
   {
     FakeControlledMedia foo(controller);
-    ASSERT_TRUE(!controller->IsPlaying());
+    ASSERT_TRUE(controller->GetState() == PlaybackState::eStopped);
 
     foo.SetPlaying(true);
-    ASSERT_TRUE(controller->IsPlaying());
+    ASSERT_TRUE(controller->GetState() == PlaybackState::ePlaying);
 
     // foo is playing, so controller is in `playing` state.
     FakeControlledMedia bar(controller);
-    ASSERT_TRUE(controller->IsPlaying());
+    ASSERT_TRUE(controller->GetState() == PlaybackState::ePlaying);
 
     bar.SetPlaying(true);
-    ASSERT_TRUE(controller->IsPlaying());
+    ASSERT_TRUE(controller->GetState() == PlaybackState::ePlaying);
 
     // Although we paused bar, but foo is still playing, so the controller would
     // still be in `playing`.
     bar.SetPlaying(false);
-    ASSERT_TRUE(controller->IsPlaying());
+    ASSERT_TRUE(controller->GetState() == PlaybackState::ePlaying);
 
     foo.SetPlaying(false);
-    ASSERT_TRUE(!controller->IsPlaying());
+    ASSERT_TRUE(controller->GetState() == PlaybackState::ePaused);
   }
 
   // both foo and bar have been destroyed, no playing media exists.
-  ASSERT_TRUE(!controller->IsPlaying());
+  ASSERT_TRUE(controller->GetState() == PlaybackState::ePaused);
 }