Bug 1644479 - replace assertions with early returns. r=chunmin
authoralwu <alwu@mozilla.com>
Tue, 16 Jun 2020 19:05:02 +0000
changeset 535933 37510e9b92b2a7479b45fffec345286a67c3c891
parent 535932 632c0f1ee377060a1efd9d53cd5b56841ad0747e
child 535934 2b5133957b5ef8d8bd3aa609f2231916e39d417e
push id37513
push userrmaries@mozilla.com
push dateWed, 17 Jun 2020 03:41:56 +0000
treeherdermozilla-central@0e023da23571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschunmin
bugs1644479
milestone79.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 1644479 - replace assertions with early returns. r=chunmin Still can not figure out how did we enable the same action twice, because we would only update the action change when the action handler is set from `null` to `something` or from `something` to `nothing` [1]. In addition, the browsing context Id for a media session is always unchanged, so it's guarantee to set the action on the correct `MediaSessionInfo`. However, in order to avoid having more crashes, I have to change the assertions to early returns and add logs to help us be aware of this issue if it happens again. [1] https://searchfox.org/mozilla-central/rev/fac90408bcf52ca88a3dcd2ef30a379b68ab24e2/dom/media/mediasession/MediaSession.cpp#71-75 Differential Revision: https://phabricator.services.mozilla.com/D78947
dom/media/mediacontrol/MediaStatusManager.cpp
--- a/dom/media/mediacontrol/MediaStatusManager.cpp
+++ b/dom/media/mediacontrol/MediaStatusManager.cpp
@@ -305,32 +305,38 @@ void MediaStatusManager::UpdateActualPla
 }
 
 void MediaStatusManager::EnableAction(uint64_t aBrowsingContextId,
                                       MediaSessionAction aAction) {
   if (!mMediaSessionInfoMap.Contains(aBrowsingContextId)) {
     return;
   }
   MediaSessionInfo* info = mMediaSessionInfoMap.GetValue(aBrowsingContextId);
-  MOZ_DIAGNOSTIC_ASSERT(!info->IsActionSupported(aAction),
-                        "Action has already been enabled!");
+  if (info->IsActionSupported(aAction)) {
+    LOG("Action '%s' has already been enabled for context %" PRIu64,
+        ToMediaSessionActionStr(aAction), aBrowsingContextId);
+    return;
+  }
   LOG("Enable action %s for context %" PRIu64, ToMediaSessionActionStr(aAction),
       aBrowsingContextId);
   info->EnableAction(aAction);
   NotifySupportedKeysChangedIfNeeded(aBrowsingContextId);
 }
 
 void MediaStatusManager::DisableAction(uint64_t aBrowsingContextId,
                                        MediaSessionAction aAction) {
   if (!mMediaSessionInfoMap.Contains(aBrowsingContextId)) {
     return;
   }
   MediaSessionInfo* info = mMediaSessionInfoMap.GetValue(aBrowsingContextId);
-  MOZ_DIAGNOSTIC_ASSERT(info->IsActionSupported(aAction),
-                        "Action hasn't been enabled yet!");
+  if (!info->IsActionSupported(aAction)) {
+    LOG("Action '%s' hasn't been enabled yet for context %" PRIu64,
+        ToMediaSessionActionStr(aAction), aBrowsingContextId);
+    return;
+  }
   LOG("Disable action %s for context %" PRIu64,
       ToMediaSessionActionStr(aAction), aBrowsingContextId);
   info->DisableAction(aAction);
   NotifySupportedKeysChangedIfNeeded(aBrowsingContextId);
 }
 
 void MediaStatusManager::NotifySupportedKeysChangedIfNeeded(
     uint64_t aBrowsingContextId) {