GMC: Go back to tab when header is clicked

This CL makes MediaNotificationView notify its container when the
header is clicked. This allows GMC's MediaNotificationContainerImplView
to handle header clicks as if the container itself was clicked (which
is fine since clicking the header on GMC has no effect since we hide
the expand button).

This fixes an issue where if the user clicked on the header (which is
visually indistinguishable from the rest of the notification) then we
would not go back to tab.

Bug: 1020300
Change-Id: I7640b4d268871f2fe49c87c3b174f4a49e716500
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1894190Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Jazz Xu <jazzhsu@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711425}

ash/media/media_notification_container_impl.h

@@ -49,6 +49,7 @@ class ASH_EXPORT MediaNotificationContainerImpl
       override {}
   void OnMediaArtworkChanged(const gfx::ImageSkia& image) override {}
   void OnColorsChanged(SkColor foreground, SkColor background) override;
+  void OnHeaderClicked() override {}
 
   // views::View:
   void OnMouseEvent(ui::MouseEvent* event) override;

chrome/browser/ui/views/global_media_controls/media_notification_container_impl_view.cc

@@ -178,6 +178,13 @@ void MediaNotificationContainerImplView::OnColorsChanged(SkColor foreground,
   }
 }
 
+void MediaNotificationContainerImplView::OnHeaderClicked() {
+  // Since we disable the expand button, nothing happens on the
+  // MediaNotificationView when the header is clicked. Treat the click as if we
+  // were clicked directly.
+  ContainerClicked();
+}
+
 ui::Layer* MediaNotificationContainerImplView::GetSlideOutLayer() {
   return swipeable_container_->layer();
 }
@@ -191,8 +198,7 @@ void MediaNotificationContainerImplView::ButtonPressed(views::Button* sender,
   if (sender == dismiss_button_) {
     DismissNotification();
   } else if (sender == this) {
-    for (auto& observer : observers_)
-      observer.OnContainerClicked(id_);
+    ContainerClicked();
   } else {
     NOTREACHED();
   }
@@ -251,3 +257,8 @@ void MediaNotificationContainerImplView::ForceExpandedState() {
     view_->SetForcedExpandedState(&should_expand);
   }
 }
+
+void MediaNotificationContainerImplView::ContainerClicked() {
+  for (auto& observer : observers_)
+    observer.OnContainerClicked(id_);
+}

chrome/browser/ui/views/global_media_controls/media_notification_container_impl_view.h

@@ -64,6 +64,7 @@ class MediaNotificationContainerImplView
       override;
   void OnMediaArtworkChanged(const gfx::ImageSkia& image) override;
   void OnColorsChanged(SkColor foreground, SkColor background) override;
+  void OnHeaderClicked() override;
 
   // views::SlideOutControllerDelegate:
   ui::Layer* GetSlideOutLayer() override;
@@ -98,6 +99,9 @@ class MediaNotificationContainerImplView
   // Updates the forced expanded state of |view_|.
   void ForceExpandedState();
 
+  // Notify observers that we've been clicked.
+  void ContainerClicked();
+
   const std::string id_;
   views::View* swipeable_container_ = nullptr;
 

chrome/browser/ui/views/global_media_controls/media_notification_container_impl_view_unittest.cc

@@ -132,6 +132,13 @@ class MediaNotificationContainerImplViewTest : public views::ViewsTestBase {
     views::test::ButtonTestApi(notification_container_).NotifyClick(event);
   }
 
+  void SimulateHeaderClicked() {
+    ui::MouseEvent event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
+                         ui::EventTimeForNow(), 0, 0);
+    views::test::ButtonTestApi(GetView()->GetHeaderRowForTesting())
+        .NotifyClick(event);
+  }
+
   void SimulateDismissButtonClicked() {
     ui::MouseEvent event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
                          ui::EventTimeForNow(), 0, 0);
@@ -356,6 +363,12 @@ TEST_F(MediaNotificationContainerImplViewTest, SendsDestroyedUpdates) {
 }
 
 TEST_F(MediaNotificationContainerImplViewTest, SendsClicks) {
+  // When the container is clicked directly, it should notify its observers.
   EXPECT_CALL(observer(), OnContainerClicked(kTestNotificationId));
   SimulateContainerClicked();
+  testing::Mock::VerifyAndClearExpectations(&observer());
+
+  // It should also notify its observers when the header is clicked.
+  EXPECT_CALL(observer(), OnContainerClicked(kTestNotificationId));
+  SimulateHeaderClicked();
 }

components/media_message_center/media_notification_container.h

@@ -41,6 +41,9 @@ class COMPONENT_EXPORT(MEDIA_MESSAGE_CENTER) MediaNotificationContainer {
   // Called when MediaNotificationView's colors change.
   virtual void OnColorsChanged(SkColor foreground, SkColor background) = 0;
 
+  // Called when the header row is clicked.
+  virtual void OnHeaderClicked() = 0;
+
  protected:
   virtual ~MediaNotificationContainer() = default;
 };

components/media_message_center/media_notification_view.cc

@@ -275,6 +275,7 @@ void MediaNotificationView::ButtonPressed(views::Button* sender,
                                           const ui::Event& event) {
   if (sender == header_row_) {
     SetExpanded(!expanded_);
+    container_->OnHeaderClicked();
     return;
   }
 
@@ -367,6 +368,10 @@ void MediaNotificationView::UpdateWithMediaArtwork(
   SchedulePaint();
 }
 
+views::Button* MediaNotificationView::GetHeaderRowForTesting() const {
+  return header_row_;
+}
+
 void MediaNotificationView::UpdateActionButtonsVisibility() {
   base::flat_set<MediaSessionAction> ignored_actions = {
       GetPlayPauseIgnoredAction(GetActionFromButtonTag(*play_pause_button_))};

components/media_message_center/media_notification_view.h

@@ -96,6 +96,8 @@ class COMPONENT_EXPORT(MEDIA_MESSAGE_CENTER) MediaNotificationView
 
   const views::Label* artist_label_for_testing() const { return artist_label_; }
 
+  views::Button* GetHeaderRowForTesting() const;
+
  private:
   friend class MediaNotificationViewTest;
 

components/media_message_center/media_notification_view_unittest.cc

@@ -99,6 +99,7 @@ class MockMediaNotificationContainer : public MediaNotificationContainer {
                void(const base::flat_set<MediaSessionAction>& actions));
   MOCK_METHOD1(OnMediaArtworkChanged, void(const gfx::ImageSkia& image));
   MOCK_METHOD2(OnColorsChanged, void(SkColor foreground, SkColor background));
+  MOCK_METHOD0(OnHeaderClicked, void());
 
   MediaNotificationView* view() const { return view_.get(); }
   void SetView(std::unique_ptr<MediaNotificationView> view) {
@@ -1303,4 +1304,9 @@ TEST_F(MAYBE_MediaNotificationViewTest, AllowsHidingOfAppIcon) {
       GetHeaderRow(&hides_icon)->app_icon_view_for_testing()->GetVisible());
 }
 
+TEST_F(MAYBE_MediaNotificationViewTest, ClickHeader_NotifyContainer) {
+  EXPECT_CALL(container(), OnHeaderClicked());
+  SimulateHeaderClick();
+}
+
 }  // namespace media_message_center