Bug 1632301 - part3 : use MediaAudibleState to replace boolean value. r=bryce
authoralwu <alwu@mozilla.com>
Tue, 28 Apr 2020 07:14:05 +0000
changeset 526399 ddf995c1bb1d16c79e4f48c4537e714f45fcceea
parent 526398 636b863fce4b5370f9b306b56b7ffee0257d68e4
child 526400 55ac8f5b94725889afb734edbc27bc3d799277c7
push id114250
push useralwu@mozilla.com
push dateTue, 28 Apr 2020 07:14:58 +0000
treeherderautoland@55ac8f5b9472 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbryce
bugs1632301
milestone77.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 1632301 - part3 : use MediaAudibleState to replace boolean value. r=bryce This patch will do : - replace `boolean` with enum class `MediaAudibleState` The advantage of doing so : - It's easier to understand what actually meaning of the parameter we set Differential Revision: https://phabricator.services.mozilla.com/D72058
dom/html/HTMLMediaElement.cpp
dom/ipc/ContentParent.cpp
dom/ipc/ContentParent.h
dom/ipc/PContent.ipdl
dom/media/mediacontrol/ContentMediaController.cpp
dom/media/mediacontrol/ContentMediaController.h
dom/media/mediacontrol/MediaControlIPC.h
dom/media/mediacontrol/MediaController.cpp
dom/media/mediacontrol/MediaController.h
dom/media/mediacontrol/tests/gtest/TestMediaController.cpp
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -442,29 +442,29 @@ class HTMLMediaElement::MediaControlEven
     MOZ_ASSERT(IsStarted());
     if (mState == ControlledMediaState::eStarted ||
         mState == ControlledMediaState::ePaused) {
       NotifyMediaStateChanged(ControlledMediaState::ePlayed);
       // If media is `inaudible` in the beginning, then we don't need to notify
       // the state, because notifying `inaudible` should always come after
       // notifying `audible`.
       if (mIsOwnerAudible) {
-        NotifyAudibleStateChanged(true);
+        NotifyAudibleStateChanged(MediaAudibleState::eAudible);
       }
     }
   }
 
   void NotifyMediaStoppedPlaying() {
     MOZ_ASSERT(NS_IsMainThread());
     MOZ_ASSERT(IsStarted());
     if (mState == ControlledMediaState::ePlayed) {
       NotifyMediaStateChanged(ControlledMediaState::ePaused);
       // As media are going to be paused, so no sound is possible to be heard.
       if (mIsOwnerAudible) {
-        NotifyAudibleStateChanged(false);
+        NotifyAudibleStateChanged(MediaAudibleState::eInaudible);
       }
     }
   }
 
   void UpdateMediaAudibleState(bool aIsOwnerAudible) {
     MOZ_ASSERT(NS_IsMainThread());
     MOZ_ASSERT(IsStarted());
     if (mIsOwnerAudible == aIsOwnerAudible) {
@@ -472,17 +472,19 @@ class HTMLMediaElement::MediaControlEven
     }
     mIsOwnerAudible = aIsOwnerAudible;
     MEDIACONTROL_LOG("Media becomes %s",
                      mIsOwnerAudible ? "audible" : "inaudible");
     // If media hasn't started playing, it doesn't make sense to update media
     // audible state. Therefore, in that case we would noitfy the audible state
     // when media starts playing.
     if (mState == ControlledMediaState::ePlayed) {
-      NotifyAudibleStateChanged(mIsOwnerAudible);
+      NotifyAudibleStateChanged(mIsOwnerAudible
+                                    ? MediaAudibleState::eAudible
+                                    : MediaAudibleState::eInaudible);
     }
   }
 
   void SetPictureInPictureModeEnabled(bool aIsEnabled) {
     MOZ_ASSERT(NS_IsMainThread());
     MOZ_ASSERT(IsStarted());
     if (mIsPictureInPictureEnabled == aIsEnabled) {
       return;
@@ -534,20 +536,20 @@ class HTMLMediaElement::MediaControlEven
     MEDIACONTROL_LOG("NotifyMediaState from state='%s' to state='%s'",
                      ToControlledMediaStateStr(mState),
                      ToControlledMediaStateStr(aState));
     MOZ_ASSERT(mState != aState, "Should not notify same state again!");
     mState = aState;
     mControlAgent->NotifyMediaStateChanged(this, mState);
   }
 
-  void NotifyAudibleStateChanged(bool aIsOwnerAudible) {
+  void NotifyAudibleStateChanged(MediaAudibleState aState) {
     MOZ_ASSERT(NS_IsMainThread());
     MOZ_ASSERT(IsStarted());
-    mControlAgent->NotifyAudibleStateChanged(this, aIsOwnerAudible);
+    mControlAgent->NotifyAudibleStateChanged(this, aState);
   }
 
   ControlledMediaState mState = ControlledMediaState::eStopped;
   WeakPtr<HTMLMediaElement> mElement;
   RefPtr<ContentMediaAgent> mControlAgent;
   bool mIsPictureInPictureEnabled = false;
   bool mIsOwnerAudible = false;
 };
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -5797,23 +5797,23 @@ mozilla::ipc::IPCResult ContentParent::R
   if (RefPtr<MediaController> controller =
           aContext.get_canonical()->GetMediaController()) {
     controller->NotifyMediaStateChanged(aState);
   }
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult ContentParent::RecvNotifyMediaAudibleChanged(
-    const MaybeDiscarded<BrowsingContext>& aContext, bool aAudible) {
+    const MaybeDiscarded<BrowsingContext>& aContext, MediaAudibleState aState) {
   if (aContext.IsNullOrDiscarded()) {
     return IPC_OK();
   }
   if (RefPtr<MediaController> controller =
           aContext.get_canonical()->GetMediaController()) {
-    controller->NotifyMediaAudibleChanged(aAudible);
+    controller->NotifyMediaAudibleChanged(aState);
   }
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult ContentParent::RecvNotifyPictureInPictureModeChanged(
     const MaybeDiscarded<BrowsingContext>& aContext, bool aEnabled) {
   if (aContext.IsNullOrDiscarded()) {
     return IPC_OK();
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -1243,17 +1243,18 @@ class ContentParent final
   mozilla::ipc::IPCResult RecvStoreUserInteractionAsPermission(
       const Principal& aPrincipal);
 
   mozilla::ipc::IPCResult RecvNotifyMediaStateChanged(
       const MaybeDiscarded<BrowsingContext>& aContext,
       ControlledMediaState aState);
 
   mozilla::ipc::IPCResult RecvNotifyMediaAudibleChanged(
-      const MaybeDiscarded<BrowsingContext>& aContext, bool aAudible);
+      const MaybeDiscarded<BrowsingContext>& aContext,
+      MediaAudibleState aState);
 
   mozilla::ipc::IPCResult RecvNotifyPictureInPictureModeChanged(
       const MaybeDiscarded<BrowsingContext>& aContext, bool aEnabled);
 
   mozilla::ipc::IPCResult RecvNotifyMediaSessionUpdated(
       const MaybeDiscarded<BrowsingContext>& aContext, bool aIsCreated);
 
   mozilla::ipc::IPCResult RecvNotifyUpdateMediaMetadata(
--- a/dom/ipc/PContent.ipdl
+++ b/dom/ipc/PContent.ipdl
@@ -113,16 +113,17 @@ using mozilla::dom::WindowContextTransac
 using mozilla::dom::WindowContextInitializer from "mozilla/dom/WindowContext.h";
 using base::SharedMemoryHandle from "base/shared_memory.h";
 using mozilla::ipc::SharedMemoryBasic::Handle from "mozilla/ipc/SharedMemoryBasic.h";
 using mozilla::fontlist::Pointer from "SharedFontList.h";
 using gfxSparseBitSet from "gfxFontUtils.h";
 using FontVisibility from "gfxFontEntry.h";
 using mozilla::dom::MediaControlKeysEvent from "ipc/MediaControlIPC.h";
 using mozilla::dom::ControlledMediaState from "ipc/MediaControlIPC.h";
+using mozilla::dom::MediaAudibleState from "ipc/MediaControlIPC.h";
 using mozilla::dom::MaybeMediaMetadataBase from "mozilla/dom/MediaSessionIPCUtils.h";
 using mozilla::dom::MediaSessionPlaybackState from "mozilla/dom/MediaSessionBinding.h";
 using refcounted class nsDocShellLoadState from "nsDocShellLoadState.h";
 using mozilla::dom::ServiceWorkerShutdownState::Progress from "mozilla/dom/ServiceWorkerShutdownState.h";
 using refcounted class mozilla::dom::CrossProcessSHEntry from "mozilla/dom/MaybeNewPSHEntry.h";
 using mozilla::ContentBlockingNotifier::StorageAccessGrantedReason from "mozilla/ContentBlockingNotifier.h";
 using mozilla::ContentBlockingNotifier::BlockingDecision from "mozilla/ContentBlockingNotifier.h";
 using mozilla::ContentBlocking::StorageAccessPromptChoices from "mozilla/ContentBlocking.h";
@@ -1510,17 +1511,17 @@ parent:
     async NotifyMediaStateChanged(MaybeDiscardedBrowsingContext aContext,
                                   ControlledMediaState aState);
 
    /**
     * When media became audible or inaudible in content process, we have to
     * notify chrome process in order to which tab is audible.
     */
     async NotifyMediaAudibleChanged(MaybeDiscardedBrowsingContext aContext,
-                                    bool aAudible);
+                                    MediaAudibleState aState);
 
    /**
     * When media enabled or disabled the Picture-in-Picture mode, we have to
     * update that to the media controller in the chrome process.
     */
     async NotifyPictureInPictureModeChanged(
         MaybeDiscardedBrowsingContext aContext, bool aEnabled);
 
--- a/dom/media/mediacontrol/ContentMediaController.cpp
+++ b/dom/media/mediacontrol/ContentMediaController.cpp
@@ -115,38 +115,39 @@ void ContentMediaController::NotifyMedia
     if (RefPtr<MediaController> controller =
             bc->Canonical()->GetMediaController()) {
       controller->NotifyMediaStateChanged(aState);
     }
   }
 }
 
 void ContentMediaController::NotifyAudibleStateChanged(
-    const ContentControlKeyEventReceiver* aMedia, bool aAudible) {
+    const ContentControlKeyEventReceiver* aMedia, MediaAudibleState aState) {
   MOZ_ASSERT(NS_IsMainThread());
   if (!mReceivers.Contains(aMedia)) {
     return;
   }
 
   RefPtr<BrowsingContext> bc = aMedia->GetBrowsingContext();
   if (!bc || bc->IsDiscarded()) {
     return;
   }
 
   LOG("Notify media became %s in BC %" PRId64,
-      aAudible ? "audible" : "inaudible", bc->Id());
+      aState == MediaAudibleState::eAudible ? "audible" : "inaudible",
+      bc->Id());
   if (XRE_IsContentProcess()) {
     ContentChild* contentChild = ContentChild::GetSingleton();
-    Unused << contentChild->SendNotifyMediaAudibleChanged(bc, aAudible);
+    Unused << contentChild->SendNotifyMediaAudibleChanged(bc, aState);
   } else {
     // Currently this only happen when we disable e10s, otherwise all controlled
     // media would be run in the content process.
     if (RefPtr<MediaController> controller =
             bc->Canonical()->GetMediaController()) {
-      controller->NotifyMediaAudibleChanged(aAudible);
+      controller->NotifyMediaAudibleChanged(aState);
     }
   }
 }
 
 void ContentMediaController::NotifyPictureInPictureModeChanged(
     const ContentControlKeyEventReceiver* aMedia, bool aEnabled) {
   MOZ_ASSERT(NS_IsMainThread());
   if (!mReceivers.Contains(aMedia)) {
--- a/dom/media/mediacontrol/ContentMediaController.h
+++ b/dom/media/mediacontrol/ContentMediaController.h
@@ -29,16 +29,25 @@ class BrowsingContext;
 enum class ControlledMediaState : uint32_t {
   eStarted,
   ePlayed,
   ePaused,
   eStopped,
 };
 
 /**
+ * This enum is used to update controlled media audible audible state to the
+ * media controller in the chrome process.
+ */
+enum class MediaAudibleState : bool {
+  eInaudible = false,
+  eAudible = true,
+};
+
+/**
  * ContentControlKeyEventReceiver is an interface which is used to receive media
  * control key events sent from the chrome process, this class MUST only be used
  * in PlaybackController.
  *
  * Each browsing context tree would only have one ContentControlKeyEventReceiver
  * that is used to handle media control key events for that browsing context
  * tree.
  */
@@ -89,17 +98,18 @@ class ContentMediaAgent {
   // follow the following rules in which `audible` and `inaudible` should be a
   // pair. `inaudible` should always be notified after `audible`. When audible
   // media paused, `inaudible` should be notified
   // Eg. (O) `audible` -> `inaudible` -> `audible` -> `inaudible`
   //     (X) `inaudible` -> `audible`    [notify `inaudible` before `audible`]
   //     (X) `audible` -> `audible`      [notify `audible` twice]
   //     (X) `audible` -> (media pauses) [forgot to notify `inaudible`]
   virtual void NotifyAudibleStateChanged(
-      const ContentControlKeyEventReceiver* aMedia, bool aAudible) = 0;
+      const ContentControlKeyEventReceiver* aMedia,
+      MediaAudibleState aState) = 0;
 
   // Use this method to update the picture in picture mode state of controlled
   // media, and it's safe to notify same state again.
   virtual void NotifyPictureInPictureModeChanged(
       const ContentControlKeyEventReceiver* aMedia, bool aEnabled) = 0;
 
   // Use these methods to register/unregister `ContentControlKeyEventReceiver`
   // in order to listen to media control key events.
@@ -131,17 +141,17 @@ class ContentMediaController final : pub
 
   explicit ContentMediaController(uint64_t aId);
   // ContentMediaAgent methods
   void AddReceiver(ContentControlKeyEventReceiver* aListener) override;
   void RemoveReceiver(ContentControlKeyEventReceiver* aListener) override;
   void NotifyMediaStateChanged(const ContentControlKeyEventReceiver* aMedia,
                                ControlledMediaState aState) override;
   void NotifyAudibleStateChanged(const ContentControlKeyEventReceiver* aMedia,
-                                 bool aAudible) override;
+                                 MediaAudibleState aState) override;
   void NotifyPictureInPictureModeChanged(
       const ContentControlKeyEventReceiver* aMedia, bool aEnabled) override;
 
   // ContentControlKeyEventReceiver method
   void HandleEvent(MediaControlKeysEvent aEvent) override;
 
  private:
   ~ContentMediaController() = default;
--- a/dom/media/mediacontrol/MediaControlIPC.h
+++ b/dom/media/mediacontrol/MediaControlIPC.h
@@ -22,11 +22,18 @@ struct ParamTraits<mozilla::dom::MediaCo
 
 template <>
 struct ParamTraits<mozilla::dom::ControlledMediaState>
     : public ContiguousEnumSerializerInclusive<
           mozilla::dom::ControlledMediaState,
           mozilla::dom::ControlledMediaState::eStarted,
           mozilla::dom::ControlledMediaState::eStopped> {};
 
+template <>
+struct ParamTraits<mozilla::dom::MediaAudibleState>
+    : public ContiguousEnumSerializerInclusive<
+          mozilla::dom::MediaAudibleState,
+          mozilla::dom::MediaAudibleState::eInaudible,
+          mozilla::dom::MediaAudibleState::eAudible> {};
+
 }  // namespace IPC
 
 #endif  // mozilla_MediaControlIPC_hh
\ No newline at end of file
--- a/dom/media/mediacontrol/MediaController.cpp
+++ b/dom/media/mediacontrol/MediaController.cpp
@@ -133,24 +133,24 @@ void MediaController::NotifyMediaStateCh
     DecreaseControlledMediaNum();
   } else if (aState == ControlledMediaState::ePlayed) {
     IncreasePlayingControlledMediaNum();
   } else if (aState == ControlledMediaState::ePaused) {
     DecreasePlayingControlledMediaNum();
   }
 }
 
-void MediaController::NotifyMediaAudibleChanged(bool aAudible) {
+void MediaController::NotifyMediaAudibleChanged(MediaAudibleState aState) {
   if (mShutdown) {
     return;
   }
-  mAudible = aAudible;
+  mAudibleState = aState;
   RefPtr<MediaControlService> service = MediaControlService::GetService();
   MOZ_ASSERT(service);
-  if (mAudible) {
+  if (mAudibleState == MediaAudibleState::eAudible) {
     service->GetAudioFocusManager().RequestAudioFocus(this);
   } else {
     service->GetAudioFocusManager().RevokeAudioFocus(this);
   }
 }
 
 void MediaController::IncreaseControlledMediaNum() {
   MOZ_ASSERT(!mShutdown);
@@ -271,17 +271,17 @@ bool MediaController::IsInPictureInPictu
 }
 
 MediaSessionPlaybackState MediaController::GetState() const {
   return mActualPlaybackState;
 }
 
 bool MediaController::IsAudible() const {
   return mGuessedPlaybackState == MediaSessionPlaybackState::Playing &&
-         mAudible;
+         mAudibleState == MediaAudibleState::eAudible;
 }
 
 uint64_t MediaController::ControlledMediaNum() const {
   return mControlledMediaNum;
 }
 
 }  // namespace dom
 }  // namespace mozilla
--- a/dom/media/mediacontrol/MediaController.h
+++ b/dom/media/mediacontrol/MediaController.h
@@ -79,17 +79,17 @@ class MediaController final
   MediaSessionPlaybackState GetState() const;
 
   void SetDeclaredPlaybackState(uint64_t aSessionContextId,
                                 MediaSessionPlaybackState aState) override;
 
   // 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);
+  void NotifyMediaAudibleChanged(MediaAudibleState aState);
 
  private:
   ~MediaController();
 
   void UpdateMediaControlKeysEventToContentMediaIfNeeded(
       MediaControlKeysEvent aEvent);
   void IncreaseControlledMediaNum();
   void DecreaseControlledMediaNum();
@@ -101,17 +101,17 @@ class MediaController final
 
   void SetGuessedPlayState(MediaSessionPlaybackState aState);
 
   // Whenever the declared playback state (from media session controller) or the
   // guessed playback state changes, we should recompute actual playback state
   // to know if we need to update the virtual control interface.
   void UpdateActualPlaybackState();
 
-  bool mAudible = false;
+  MediaAudibleState mAudibleState = MediaAudibleState::eInaudible;
   bool mIsRegisteredToService = false;
   int64_t mControlledMediaNum = 0;
   int64_t mPlayingControlledMediaNum = 0;
   bool mShutdown = false;
   bool mIsInPictureInPictureMode = false;
 
   // This state can match to the `guessed playback state` in the spec [1], it
   // indicates if we have any media element playing within the tab which this
--- a/dom/media/mediacontrol/tests/gtest/TestMediaController.cpp
+++ b/dom/media/mediacontrol/tests/gtest/TestMediaController.cpp
@@ -54,29 +54,29 @@ TEST(MediaController, ActiveAndDeactiveC
 }
 
 TEST(MediaController, AudibleChanged)
 {
   RefPtr<MediaController> controller = new MediaController(CONTROLLER_ID);
   controller->Play();
   ASSERT_TRUE(!controller->IsAudible());
 
-  controller->NotifyMediaAudibleChanged(true);
+  controller->NotifyMediaAudibleChanged(MediaAudibleState::eAudible);
   ASSERT_TRUE(controller->IsAudible());
 
-  controller->NotifyMediaAudibleChanged(false);
+  controller->NotifyMediaAudibleChanged(MediaAudibleState::eInaudible);
   ASSERT_TRUE(!controller->IsAudible());
 }
 
 TEST(MediaController, AlwaysInaudibleIfControllerIsNotPlaying)
 {
   RefPtr<MediaController> controller = new MediaController(CONTROLLER_ID);
   ASSERT_TRUE(!controller->IsAudible());
 
-  controller->NotifyMediaAudibleChanged(true);
+  controller->NotifyMediaAudibleChanged(MediaAudibleState::eAudible);
   ASSERT_TRUE(!controller->IsAudible());
 
   controller->Play();
   ASSERT_TRUE(controller->IsAudible());
 
   controller->Pause();
   ASSERT_TRUE(!controller->IsAudible());