GMC: Reland "Add interactive UI tests"

This CL relands the interactive UI tests for the global media controls.
The original CL was flaky for the ShowsMetadataAndControlsMedia test
because sometimes it checked for toolbar icon visibility after media
started but before the media starting caused the icon to appear. This
CL fixes that by waiting for the icon to appear.

TESTED=Was locally flaking about once every 20 runs before the fix.
After the fix, I ran it 300 times and it passed every time.

Description from original CL (crrev.com/c/1753690):
This CL adds some basic interactive UI tests for the global media
controls. This ensures we have better coverage than just unit tests.

Bug: 993879, 999297
Change-Id: I7abf36e4f82b64b262d3d0a7eb9d426e8ae1c5f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1814671Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702882}

ash/media/media_notification_container_impl.h

@@ -41,6 +41,7 @@ class ASH_EXPORT MediaNotificationContainerImpl
   void OnExpanded(bool expanded) override;
   void OnMediaSessionInfoChanged(
       const media_session::mojom::MediaSessionInfoPtr& session_info) override {}
+  void OnMediaSessionMetadataChanged() override {}
   void OnVisibleActionsChanged(
       const std::set<media_session::mojom::MediaSessionAction>& actions)
       override {}

chrome/browser/ui/BUILD.gn

@@ -2811,6 +2811,7 @@ jumbo_split_static_library("ui") {
       "views/global_error_bubble_view.h",
       "views/global_media_controls/media_dialog_view.cc",
       "views/global_media_controls/media_dialog_view.h",
+      "views/global_media_controls/media_dialog_view_observer.h",
       "views/global_media_controls/media_notification_container_impl.cc",
       "views/global_media_controls/media_notification_container_impl.h",
       "views/global_media_controls/media_notification_list_view.cc",

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

@@ -8,6 +8,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/global_media_controls/media_toolbar_button_controller.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
+#include "chrome/browser/ui/views/global_media_controls/media_dialog_view_observer.h"
 #include "chrome/browser/ui/views/global_media_controls/media_notification_container_impl.h"
 #include "chrome/browser/ui/views/global_media_controls/media_notification_list_view.h"
 #include "services/media_session/public/mojom/media_session.mojom.h"
@@ -66,6 +67,9 @@ void MediaDialogView::ShowMediaSession(
       id, std::make_unique<MediaNotificationContainerImpl>(this, controller_,
                                                            id, item));
   OnAnchorBoundsChanged();
+
+  for (auto& observer : observers_)
+    observer.OnMediaSessionShown();
 }
 
 void MediaDialogView::HideMediaSession(const std::string& id) {
@@ -75,6 +79,9 @@ void MediaDialogView::HideMediaSession(const std::string& id) {
     HideDialog();
   else
     OnAnchorBoundsChanged();
+
+  for (auto& observer : observers_)
+    observer.OnMediaSessionHidden();
 }
 
 int MediaDialogView::GetDialogButtons() const {
@@ -111,6 +118,19 @@ gfx::Size MediaDialogView::CalculatePreferredSize() const {
   return gfx::Size(width, 1);
 }
 
+void MediaDialogView::AddObserver(MediaDialogViewObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void MediaDialogView::RemoveObserver(MediaDialogViewObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+void MediaDialogView::OnMediaSessionMetadataChanged() {
+  for (auto& observer : observers_)
+    observer.OnMediaSessionMetadataUpdated();
+}
+
 MediaDialogView::MediaDialogView(views::View* anchor_view,
                                  MediaToolbarButtonController* controller,
                                  service_manager::Connector* connector)

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

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_GLOBAL_MEDIA_CONTROLS_MEDIA_DIALOG_VIEW_H_
 #define CHROME_BROWSER_UI_VIEWS_GLOBAL_MEDIA_CONTROLS_MEDIA_DIALOG_VIEW_H_
 
+#include "base/observer_list.h"
 #include "base/optional.h"
 #include "chrome/browser/ui/global_media_controls/media_dialog_delegate.h"
 #include "ui/views/bubble/bubble_dialog_delegate_view.h"
@@ -13,6 +14,7 @@ namespace service_manager {
 class Connector;
 }  // namespace service_manager
 
+class MediaDialogViewObserver;
 class MediaNotificationListView;
 class MediaToolbarButtonController;
 
@@ -26,6 +28,8 @@ class MediaDialogView : public views::BubbleDialogDelegateView,
   static void HideDialog();
   static bool IsShowing();
 
+  static MediaDialogView* GetDialogViewForTesting() { return instance_; }
+
   // MediaDialogDelegate implementation.
   void ShowMediaSession(
       const std::string& id,
@@ -40,6 +44,11 @@ class MediaDialogView : public views::BubbleDialogDelegateView,
   void AddedToWidget() override;
   gfx::Size CalculatePreferredSize() const override;
 
+  void AddObserver(MediaDialogViewObserver* observer);
+  void RemoveObserver(MediaDialogViewObserver* observer);
+
+  void OnMediaSessionMetadataChanged();
+
  private:
   explicit MediaDialogView(views::View* anchor_view,
                            MediaToolbarButtonController* controller,
@@ -59,6 +68,8 @@ class MediaDialogView : public views::BubbleDialogDelegateView,
 
   MediaNotificationListView* const active_sessions_view_;
 
+  base::ObserverList<MediaDialogViewObserver> observers_;
+
   DISALLOW_COPY_AND_ASSIGN(MediaDialogView);
 };
 

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

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_GLOBAL_MEDIA_CONTROLS_MEDIA_DIALOG_VIEW_OBSERVER_H_
+#define CHROME_BROWSER_UI_VIEWS_GLOBAL_MEDIA_CONTROLS_MEDIA_DIALOG_VIEW_OBSERVER_H_
+
+#include "base/observer_list_types.h"
+
+class MediaDialogViewObserver : public base::CheckedObserver {
+ public:
+  // Called when a new media session is added to the dialog.
+  virtual void OnMediaSessionShown() = 0;
+
+  // Called when a media session is hidden from the dialog.
+  virtual void OnMediaSessionHidden() = 0;
+
+  // Called when a shown media session's metadata is updated.
+  virtual void OnMediaSessionMetadataUpdated() = 0;
+
+ protected:
+  ~MediaDialogViewObserver() override = default;
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_GLOBAL_MEDIA_CONTROLS_MEDIA_DIALOG_VIEW_OBSERVER_H_

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

@@ -108,6 +108,10 @@ void MediaNotificationContainerImpl::OnMediaSessionInfoChanged(
                            media_session::mojom::MediaPlaybackState::kPlaying);
 }
 
+void MediaNotificationContainerImpl::OnMediaSessionMetadataChanged() {
+  parent_->OnMediaSessionMetadataChanged();
+}
+
 void MediaNotificationContainerImpl::OnVisibleActionsChanged(
     const std::set<media_session::mojom::MediaSessionAction>& actions) {
   has_many_actions_ = actions.size() >= kMinVisibleActionsForExpanding;

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

@@ -39,6 +39,7 @@ class MediaNotificationContainerImpl
   void OnExpanded(bool expanded) override;
   void OnMediaSessionInfoChanged(
       const media_session::mojom::MediaSessionInfoPtr& session_info) override;
+  void OnMediaSessionMetadataChanged() override;
   void OnVisibleActionsChanged(
       const std::set<media_session::mojom::MediaSessionAction>& actions)
       override;

chrome/test/BUILD.gn

@@ -5683,6 +5683,9 @@ if (!is_android) {
           deps += [ "//build/config/linux/gtk" ]
         }
       }
+      if (!is_chromeos) {
+        sources += [ "../browser/ui/views/global_media_controls/media_dialog_view_interactive_browsertest.cc" ]
+      }
       if (use_aura || is_mac) {
         deps += [ "//ui/touch_selection" ]
       }

chrome/test/data/media/session/video-with-different-metadata.html

+<!DOCTYPE html>
+<html>
+<head>
+  <title>Test page showing a video with media session actions and metadata</title>
+</head>
+<body>
+  <video id="video" controls loop>
+    <source src="../bigbuck.webm">
+  </video>
+<script>
+const video = document.getElementById('video');
+
+// Called by the browser test to start playback. Note that the browser test
+// needs to explicitly disable user gesture requirements for autoplay for this
+// function to work.
+function play() {
+  video.play().then(_ => {
+    setupMetadata();
+    setupActionHandlers();
+  });
+}
+
+function setupMetadata() {
+  // Use different metadata than the other test file.
+  navigator.mediaSession.metadata = new MediaMetadata({
+    title: "Different Title",
+    artist: "Another Artist"
+  });
+}
+
+function setupActionHandlers() {
+  // Add empty action handlers to tell the browser we want to show all the
+  // possible action buttons.
+  function doNothing() {}
+
+  navigator.mediaSession.setActionHandler('previoustrack', doNothing);
+  navigator.mediaSession.setActionHandler('nexttrack', doNothing);
+  navigator.mediaSession.setActionHandler('seekbackward', doNothing);
+  navigator.mediaSession.setActionHandler('seekforward', doNothing);
+  navigator.mediaSession.setActionHandler('play', _ => { video.play(); });
+  navigator.mediaSession.setActionHandler('pause', _ => { video.pause(); });
+}
+</script>
+</body>
+</html>

chrome/test/data/media/session/video-with-metadata.html

+<!DOCTYPE html>
+<html>
+<head>
+  <title>Test page showing a video with media session actions and metadata</title>
+</head>
+<body>
+  <video id="video" controls loop>
+    <source src="../bigbuck.webm">
+  </video>
+<script>
+const video = document.getElementById('video');
+
+// Called by the browser test to start playback. Note that the browser test
+// needs to explicitly disable user gesture requirements for autoplay for this
+// function to work.
+function play() {
+  video.play().then(_ => {
+    setupMetadata();
+    setupActionHandlers();
+  });
+}
+
+function setupMetadata() {
+  navigator.mediaSession.metadata = new MediaMetadata({
+    title: "Big Buck Bunny",
+    artist: "Blender Foundation"
+  });
+}
+
+function setupActionHandlers() {
+  // Add empty action handlers to tell the browser we want to show all the
+  // possible action buttons.
+  function doNothing() {}
+
+  navigator.mediaSession.setActionHandler('previoustrack', doNothing);
+  navigator.mediaSession.setActionHandler('nexttrack', doNothing);
+  navigator.mediaSession.setActionHandler('seekbackward', doNothing);
+  navigator.mediaSession.setActionHandler('seekforward', doNothing);
+  navigator.mediaSession.setActionHandler('play', _ => { video.play(); });
+  navigator.mediaSession.setActionHandler('pause', _ => { video.pause(); });
+}
+</script>
+</body>
+</html>

components/media_message_center/media_notification_container.h

@@ -28,6 +28,9 @@ class COMPONENT_EXPORT(MEDIA_MESSAGE_CENTER) MediaNotificationContainer {
   virtual void OnMediaSessionInfoChanged(
       const media_session::mojom::MediaSessionInfoPtr& session_info) = 0;
 
+  // Called when the metadata changes.
+  virtual void OnMediaSessionMetadataChanged() = 0;
+
   // TODO(https://crbug.com/1003847): Use base::flat_set isntead.
   // Called when the set of visible MediaSessionActions changes.
   virtual void OnVisibleActionsChanged(

components/media_message_center/media_notification_view.cc

@@ -328,6 +328,8 @@ void MediaNotificationView::UpdateWithMediaMetadata(
 
   RecordMetadataHistogram(Metadata::kCount);
 
+  container_->OnMediaSessionMetadataChanged();
+
   PreferredSizeChanged();
   Layout();
   SchedulePaint();

components/media_message_center/media_notification_view_unittest.cc

@@ -90,6 +90,7 @@ class MockMediaNotificationContainer : public MediaNotificationContainer {
   MOCK_METHOD1(
       OnMediaSessionInfoChanged,
       void(const media_session::mojom::MediaSessionInfoPtr& session_info));
+  MOCK_METHOD0(OnMediaSessionMetadataChanged, void());
   MOCK_METHOD1(OnVisibleActionsChanged,
                void(const std::set<MediaSessionAction>& actions));
   MOCK_METHOD1(OnMediaArtworkChanged, void(const gfx::ImageSkia& image));
@@ -598,6 +599,7 @@ TEST_F(MAYBE_MediaNotificationViewTest, UpdateMetadata_FromObserver) {
   metadata.artist = base::ASCIIToUTF16("artist2");
   metadata.album = base::ASCIIToUTF16("album");
 
+  EXPECT_CALL(container(), OnMediaSessionMetadataChanged());
   GetItem()->MediaSessionMetadataChanged(metadata);
   view()->SetExpanded(true);
 
@@ -962,6 +964,7 @@ TEST_F(MAYBE_MediaNotificationViewTest, Freezing_DoNotUpdateMetadata) {
   metadata.artist = base::ASCIIToUTF16("artist2");
   metadata.album = base::ASCIIToUTF16("album");
 
+  EXPECT_CALL(container(), OnMediaSessionMetadataChanged()).Times(0);
   GetItem()->Freeze();
   GetItem()->MediaSessionMetadataChanged(metadata);
 

content/browser/media/session/media_session_browsertest.cc

@@ -20,6 +20,7 @@
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
+#include "content/public/test/media_start_stop_observer.h"
 #include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
 #include "media/base/media_switches.h"
@@ -177,40 +178,6 @@ class MediaSessionBrowserTest : public ContentBrowserTest {
   base::test::ScopedFeatureList disabled_feature_list_;
 
  private:
-  class MediaStartStopObserver : public WebContentsObserver {
-   public:
-    enum class Type { kStart, kStop };
-
-    MediaStartStopObserver(WebContents* web_contents, Type type)
-        : WebContentsObserver(web_contents), type_(type) {}
-
-    void MediaStartedPlaying(const MediaPlayerInfo& info,
-                             const MediaPlayerId& id) override {
-      if (type_ != Type::kStart)
-        return;
-
-      run_loop_.Quit();
-    }
-
-    void MediaStoppedPlaying(
-        const MediaPlayerInfo& info,
-        const MediaPlayerId& id,
-        WebContentsObserver::MediaStoppedReason reason) override {
-      if (type_ != Type::kStop)
-        return;
-
-      run_loop_.Quit();
-    }
-
-    void Wait() { run_loop_.Run(); }
-
-   private:
-    base::RunLoop run_loop_;
-    Type type_;
-
-    DISALLOW_COPY_AND_ASSIGN(MediaStartStopObserver);
-  };
-
   void OnServerRequest(const net::test_server::HttpRequest& request) {
     // Note this method is called on the EmbeddedTestServer's background thread.
     base::AutoLock lock(visited_urls_lock_);

content/public/test/media_start_stop_observer.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/public/test/media_start_stop_observer.h"
+
+namespace content {
+
+MediaStartStopObserver::MediaStartStopObserver(WebContents* web_contents,
+                                               Type type)
+    : WebContentsObserver(web_contents), type_(type) {}
+
+MediaStartStopObserver::~MediaStartStopObserver() = default;
+
+void MediaStartStopObserver::MediaStartedPlaying(const MediaPlayerInfo& info,
+                                                 const MediaPlayerId& id) {
+  if (type_ != Type::kStart)
+    return;
+
+  run_loop_.Quit();
+}
+
+void MediaStartStopObserver::MediaStoppedPlaying(
+    const MediaPlayerInfo& info,
+    const MediaPlayerId& id,
+    WebContentsObserver::MediaStoppedReason reason) {
+  if (type_ != Type::kStop)
+    return;
+
+  run_loop_.Quit();
+}
+
+void MediaStartStopObserver::Wait() {
+  run_loop_.Run();
+}
+
+}  // namespace content

content/public/test/media_start_stop_observer.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_PUBLIC_TEST_MEDIA_START_STOP_OBSERVER_H_
+#define CONTENT_PUBLIC_TEST_MEDIA_START_STOP_OBSERVER_H_
+
+#include "base/macros.h"
+#include "base/run_loop.h"
+#include "content/public/browser/web_contents_observer.h"
+
+namespace content {
+
+class WebContents;
+
+// Used in tests to wait for media in a WebContents to start or stop playing.
+class MediaStartStopObserver : public WebContentsObserver {
+ public:
+  enum class Type { kStart, kStop };
+
+  MediaStartStopObserver(WebContents* web_contents, Type type);
+  ~MediaStartStopObserver() override;
+
+  // WebContentsObserver implementation.
+  void MediaStartedPlaying(const MediaPlayerInfo& info,
+                           const MediaPlayerId& id) override;
+  void MediaStoppedPlaying(
+      const MediaPlayerInfo& info,
+      const MediaPlayerId& id,
+      WebContentsObserver::MediaStoppedReason reason) override;
+
+  void Wait();
+
+ private:
+  base::RunLoop run_loop_;
+  const Type type_;
+
+  DISALLOW_COPY_AND_ASSIGN(MediaStartStopObserver);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_TEST_MEDIA_START_STOP_OBSERVER_H_

content/test/BUILD.gn

@@ -132,6 +132,8 @@ jumbo_static_library("test_support") {
     "../public/test/hit_test_region_observer.h",
     "../public/test/javascript_test_observer.cc",
     "../public/test/javascript_test_observer.h",
+    "../public/test/media_start_stop_observer.cc",
+    "../public/test/media_start_stop_observer.h",
     "../public/test/mock_browsing_data_remover_delegate.cc",
     "../public/test/mock_browsing_data_remover_delegate.h",
     "../public/test/mock_download_manager.cc",