GMC: Wait for actions before unfreezing

This CL prevents media notifications from unfreezing if they are still
waiting on new MediaSessionActions. This prevents an issue where
notifications would change sizes briefly because they had no actions to
show.

Bug: 1055254
Change-Id: I2345188626b332846a7b51c93545a1b40764044e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090291Reviewed-by: Becca Hughes <beccahughes@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747363}

components/media_message_center/media_notification_view_impl_unittest.cc

@@ -1219,6 +1219,87 @@ TEST_F(MAYBE_MediaNotificationViewImplTest, UnfreezingWaitsForArtwork_Timeout) {
   EXPECT_TRUE(GetArtworkImage().isNull());
 }
 
+TEST_F(MAYBE_MediaNotificationViewImplTest, UnfreezingWaitsForActions) {
+  EnableAction(MediaSessionAction::kPlay);
+  EnableAction(MediaSessionAction::kPause);
+  EnableAction(MediaSessionAction::kNextTrack);
+  EnableAction(MediaSessionAction::kPreviousTrack);
+
+  // Freeze the item and clear the metadata and actions.
+  base::MockOnceClosure unfrozen_callback;
+  EXPECT_CALL(unfrozen_callback, Run).Times(0);
+  GetItem()->Freeze(unfrozen_callback.Get());
+  GetItem()->MediaSessionInfoChanged(nullptr);
+  GetItem()->MediaSessionMetadataChanged(base::nullopt);
+  DisableAction(MediaSessionAction::kPlay);
+  DisableAction(MediaSessionAction::kPause);
+  DisableAction(MediaSessionAction::kNextTrack);
+  DisableAction(MediaSessionAction::kPreviousTrack);
+
+  // The item should be frozen and the view should contain the old data.
+  EXPECT_TRUE(GetItem()->frozen());
+  EXPECT_TRUE(GetButtonForAction(MediaSessionAction::kPlay));
+  EXPECT_FALSE(GetButtonForAction(MediaSessionAction::kPause));
+  EXPECT_TRUE(GetButtonForAction(MediaSessionAction::kNextTrack));
+  EXPECT_TRUE(GetButtonForAction(MediaSessionAction::kPreviousTrack));
+  EXPECT_EQ(base::ASCIIToUTF16("title"), title_label()->GetText());
+  EXPECT_EQ(base::ASCIIToUTF16("artist"), artist_label()->GetText());
+
+  // Bind the item to a new controller that's playing instead of paused.
+  auto new_media_controller = std::make_unique<TestMediaController>();
+  media_session::mojom::MediaSessionInfoPtr session_info(
+      media_session::mojom::MediaSessionInfo::New());
+  session_info->playback_state =
+      media_session::mojom::MediaPlaybackState::kPlaying;
+  session_info->is_controllable = true;
+  GetItem()->SetController(new_media_controller->CreateMediaControllerRemote(),
+                           session_info.Clone());
+
+  // The item will receive a MediaSessionInfoChanged.
+  GetItem()->MediaSessionInfoChanged(session_info.Clone());
+
+  // The item should still be frozen, and the view should contain the old data.
+  EXPECT_TRUE(GetItem()->frozen());
+  EXPECT_TRUE(GetButtonForAction(MediaSessionAction::kPlay));
+  EXPECT_FALSE(GetButtonForAction(MediaSessionAction::kPause));
+  EXPECT_TRUE(GetButtonForAction(MediaSessionAction::kNextTrack));
+  EXPECT_TRUE(GetButtonForAction(MediaSessionAction::kPreviousTrack));
+  EXPECT_EQ(base::ASCIIToUTF16("title"), title_label()->GetText());
+  EXPECT_EQ(base::ASCIIToUTF16("artist"), artist_label()->GetText());
+
+  // Update the metadata.
+  media_session::MediaMetadata metadata;
+  metadata.title = base::ASCIIToUTF16("title2");
+  metadata.artist = base::ASCIIToUTF16("artist2");
+  GetItem()->MediaSessionMetadataChanged(metadata);
+
+  // The item should still be frozen, and waiting for new actions.
+  EXPECT_TRUE(GetItem()->frozen());
+  testing::Mock::VerifyAndClearExpectations(&unfrozen_callback);
+  EXPECT_TRUE(GetButtonForAction(MediaSessionAction::kPlay));
+  EXPECT_FALSE(GetButtonForAction(MediaSessionAction::kPause));
+  EXPECT_TRUE(GetButtonForAction(MediaSessionAction::kNextTrack));
+  EXPECT_TRUE(GetButtonForAction(MediaSessionAction::kPreviousTrack));
+  EXPECT_EQ(base::ASCIIToUTF16("title"), title_label()->GetText());
+  EXPECT_EQ(base::ASCIIToUTF16("artist"), artist_label()->GetText());
+
+  // Once we receive actions, the item should unfreeze.
+  EXPECT_CALL(unfrozen_callback, Run);
+  EnableAction(MediaSessionAction::kPlay);
+  EnableAction(MediaSessionAction::kPause);
+  EnableAction(MediaSessionAction::kSeekForward);
+  EnableAction(MediaSessionAction::kSeekBackward);
+
+  EXPECT_FALSE(GetItem()->frozen());
+  testing::Mock::VerifyAndClearExpectations(&unfrozen_callback);
+  EXPECT_FALSE(GetButtonForAction(MediaSessionAction::kPlay));
+  EXPECT_TRUE(GetButtonForAction(MediaSessionAction::kPause));
+  EXPECT_TRUE(GetButtonForAction(MediaSessionAction::kSeekForward));
+  EXPECT_TRUE(GetButtonForAction(MediaSessionAction::kSeekBackward));
+  EXPECT_EQ(base::ASCIIToUTF16("title2"), title_label()->GetText());
+  EXPECT_EQ(base::ASCIIToUTF16("artist2"), artist_label()->GetText());
+}
+
 TEST_F(MAYBE_MediaNotificationViewImplTest,
        UnfreezingWaitsForArtwork_ReceiveArtwork) {
   EnableAction(MediaSessionAction::kPlay);

components/media_message_center/media_session_notification_item.cc

@@ -104,6 +104,8 @@ void MediaSessionNotificationItem::MediaSessionActionsChanged(
   if (view_ && !frozen_) {
     DCHECK(view_);
     view_->UpdateWithMediaActions(session_actions_);
+  } else if (waiting_for_actions_) {
+    MaybeUnfreeze();
   }
 }
 
@@ -204,6 +206,7 @@ void MediaSessionNotificationItem::Freeze(base::OnceClosure unfrozen_callback) {
     return;
 
   frozen_ = true;
+  frozen_with_actions_ = HasActions();
   frozen_with_artwork_ = HasArtwork();
 
   freeze_timer_.Start(
@@ -233,12 +236,23 @@ void MediaSessionNotificationItem::MaybeUnfreeze() {
   if (!frozen_)
     return;
 
+  if (waiting_for_actions_ && !HasActions())
+    return;
+
   if (waiting_for_artwork_ && !HasArtwork())
     return;
 
   if (!ShouldShowNotification() || !is_bound_)
     return;
 
+  // If the currently frozen view has actions and the new session currently has
+  // no actions, then wait until either the freeze timer ends or the new actions
+  // are received.
+  if (frozen_with_actions_ && !HasActions()) {
+    waiting_for_actions_ = true;
+    return;
+  }
+
   // If the currently frozen view has artwork and the new session currently has
   // no artwork, then wait until either the freeze timer ends or the new artwork
   // is downloaded.
@@ -252,6 +266,8 @@ void MediaSessionNotificationItem::MaybeUnfreeze() {
 
 void MediaSessionNotificationItem::Unfreeze() {
   frozen_ = false;
+  waiting_for_actions_ = false;
+  frozen_with_actions_ = false;
   waiting_for_artwork_ = false;
   frozen_with_artwork_ = false;
   freeze_timer_.Stop();
@@ -273,6 +289,10 @@ void MediaSessionNotificationItem::Unfreeze() {
   std::move(unfrozen_callback_).Run();
 }
 
+bool MediaSessionNotificationItem::HasActions() const {
+  return !session_actions_.empty();
+}
+
 bool MediaSessionNotificationItem::HasArtwork() const {
   return session_artwork_.has_value() && !session_artwork_->isNull();
 }
@@ -280,9 +300,10 @@ bool MediaSessionNotificationItem::HasArtwork() const {
 void MediaSessionNotificationItem::OnFreezeTimerFired() {
   DCHECK(frozen_);
 
-  // If we've just been waiting for artwork, stop waiting and just show what we
-  // have.
-  if (waiting_for_artwork_ && ShouldShowNotification() && is_bound_) {
+  // If we've just been waiting for actions or artwork, stop waiting and just
+  // show what we have.
+  if ((waiting_for_actions_ || waiting_for_artwork_) &&
+      ShouldShowNotification() && is_bound_) {
     Unfreeze();
     return;
   }

components/media_message_center/media_session_notification_item.h

@@ -99,6 +99,8 @@ class COMPONENT_EXPORT(MEDIA_MESSAGE_CENTER) MediaSessionNotificationItem
 
   void Unfreeze();
 
+  bool HasActions() const;
+
   bool HasArtwork() const;
 
   void OnFreezeTimerFired();
@@ -139,6 +141,14 @@ class COMPONENT_EXPORT(MEDIA_MESSAGE_CENTER) MediaSessionNotificationItem
   // data and no actions will be executed.
   bool frozen_ = false;
 
+  // True if we're currently frozen and the frozen view contains at least 1
+  // action.
+  bool frozen_with_actions_ = false;
+
+  // True if we have the necessary metadata to unfreeze, but we're waiting for
+  // new actions.
+  bool waiting_for_actions_ = false;
+
   // True if we're currently frozen and the frozen view contains non-null
   // artwork.
   bool frozen_with_artwork_ = false;