Add a MediaRouterDialogController for Views dialog

- Add MRDialogControllerViewsImpl stub
- Rename MRDialogControllerImpl to MRDialogControllerWebUIImpl
- Factor out parts that are common across WebUI and Views into
  MRDialogControllerImplBase
- MediaRouterDialogControllerImplBase::GetOrCreateForWebContents() is
  implemented in two places: mrdc_views_impl.cc for builds with Views,
  mrdc_webui_impl.cc for builds without.

Bug: 826091
Change-Id: I276d46ba870b837c5b35f48a4921a3df356aa565
Reviewed-on: https://chromium-review.googlesource.com/990572Reviewed-by: Derek Cheng <imcheng@chromium.org>
Reviewed-by: Adam Parker <amp@chromium.org>
Commit-Queue: Takumi Fujimoto <takumif@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548290}

chrome/browser/media/router/media_router_dialog_controller.cc

@@ -17,7 +17,7 @@
 #if defined(OS_ANDROID)
 #include "chrome/browser/media/android/router/media_router_dialog_controller_android.h"
 #else
-#include "chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl.h"
+#include "chrome/browser/ui/media_router/media_router_dialog_controller_impl_base.h"
 #endif
 
 namespace media_router {
@@ -30,7 +30,8 @@ MediaRouterDialogController::GetOrCreateForWebContents(
   return MediaRouterDialogControllerAndroid::GetOrCreateForWebContents(
       contents);
 #else
-  return MediaRouterDialogControllerImpl::GetOrCreateForWebContents(contents);
+  return MediaRouterDialogControllerImplBase::GetOrCreateForWebContents(
+      contents);
 #endif
 }
 

chrome/browser/ui/BUILD.gn

@@ -1390,6 +1390,8 @@ split_static_library("ui") {
       "media_router/cast_modes_with_media_sources.h",
       "media_router/media_cast_mode.cc",
       "media_router/media_cast_mode.h",
+      "media_router/media_router_dialog_controller_impl_base.cc",
+      "media_router/media_router_dialog_controller_impl_base.h",
       "media_router/media_router_file_dialog.cc",
       "media_router/media_router_file_dialog.h",
       "media_router/media_sink_with_cast_modes.cc",
@@ -1595,8 +1597,8 @@ split_static_library("ui") {
       "webui/md_downloads/md_downloads_ui.h",
       "webui/md_history_ui.cc",
       "webui/md_history_ui.h",
-      "webui/media_router/media_router_dialog_controller_impl.cc",
-      "webui/media_router/media_router_dialog_controller_impl.h",
+      "webui/media_router/media_router_dialog_controller_webui_impl.cc",
+      "webui/media_router/media_router_dialog_controller_webui_impl.h",
       "webui/media_router/media_router_internals_ui.cc",
       "webui/media_router/media_router_internals_ui.h",
       "webui/media_router/media_router_internals_webui_message_handler.cc",
@@ -3162,6 +3164,8 @@ split_static_library("ui") {
         "views/location_bar/star_view.h",
         "views/location_bar/zoom_view.cc",
         "views/location_bar/zoom_view.h",
+        "views/media_router/media_router_dialog_controller_views.cc",
+        "views/media_router/media_router_dialog_controller_views.h",
         "views/media_router/presentation_receiver_window_factory.cc",
         "views/media_router/presentation_receiver_window_frame.cc",
         "views/media_router/presentation_receiver_window_frame.h",

chrome/browser/ui/media_router/media_router_dialog_controller_impl_base.cc

+// Copyright 2018 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 "chrome/browser/ui/media_router/media_router_dialog_controller_impl_base.h"
+
+#include <utility>
+
+#include "chrome/browser/media/router/presentation/presentation_service_delegate_impl.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/toolbar/media_router_action.h"
+#include "chrome/browser/ui/webui/media_router/media_router_ui.h"
+#include "chrome/browser/ui/webui/media_router/media_router_ui_service.h"
+
+using content::WebContents;
+
+namespace media_router {
+
+namespace {
+
+MediaRouterActionController* GetActionController(WebContents* web_contents) {
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  // TODO(crbug.com/826091): Move MRUIService to c/b/ui/media_router/.
+  return MediaRouterUIService::Get(profile)->action_controller();
+}
+
+}  // namespace
+
+MediaRouterDialogControllerImplBase::~MediaRouterDialogControllerImplBase() =
+    default;
+
+void MediaRouterDialogControllerImplBase::SetMediaRouterAction(
+    const base::WeakPtr<MediaRouterAction>& action) {
+  action_ = action;
+}
+
+void MediaRouterDialogControllerImplBase::CreateMediaRouterDialog() {
+  // The |action_controller_| must be notified after |action_| to avoid a UI
+  // bug in which the drop shadow is drawn in an incorrect position.
+  if (action_)
+    action_->OnDialogShown();
+  action_controller_->OnDialogShown();
+}
+
+void MediaRouterDialogControllerImplBase::Reset() {
+  if (IsShowingMediaRouterDialog()) {
+    if (action_)
+      action_->OnDialogHidden();
+    action_controller_->OnDialogHidden();
+  }
+  MediaRouterDialogController::Reset();
+}
+
+MediaRouterDialogControllerImplBase::MediaRouterDialogControllerImplBase(
+    WebContents* web_contents)
+    : MediaRouterDialogController(web_contents),
+      action_controller_(GetActionController(web_contents)) {
+  DCHECK(action_controller_);
+}
+
+void MediaRouterDialogControllerImplBase::InitializeMediaRouterUI(
+    MediaRouterUI* media_router_ui) {
+  auto start_presentation_context = std::move(start_presentation_context_);
+  PresentationServiceDelegateImpl* delegate =
+      PresentationServiceDelegateImpl::FromWebContents(initiator());
+  if (!start_presentation_context) {
+    media_router_ui->InitWithDefaultMediaSource(initiator(), delegate);
+  } else {
+    media_router_ui->InitWithStartPresentationContext(
+        initiator(), delegate, std::move(start_presentation_context));
+  }
+}
+
+}  // namespace media_router

chrome/browser/ui/media_router/media_router_dialog_controller_impl_base.h

+// Copyright 2018 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_MEDIA_ROUTER_MEDIA_ROUTER_DIALOG_CONTROLLER_IMPL_BASE_H_
+#define CHROME_BROWSER_UI_MEDIA_ROUTER_MEDIA_ROUTER_DIALOG_CONTROLLER_IMPL_BASE_H_
+
+#include "base/macros.h"
+#include "chrome/browser/media/router/media_router_dialog_controller.h"
+
+class MediaRouterAction;
+class MediaRouterActionController;
+
+namespace media_router {
+
+class MediaRouterUI;
+
+// The base class for desktop implementations of MediaRouterDialogController.
+// This class is not thread safe and must be called on the UI thread.
+class MediaRouterDialogControllerImplBase : public MediaRouterDialogController {
+ public:
+  ~MediaRouterDialogControllerImplBase() override;
+
+  static MediaRouterDialogControllerImplBase* GetOrCreateForWebContents(
+      content::WebContents* web_contents);
+
+  // Sets the action to notify when a dialog gets shown or hidden.
+  void SetMediaRouterAction(const base::WeakPtr<MediaRouterAction>& action);
+
+  // MediaRouterDialogController:
+  void CreateMediaRouterDialog() override;
+  void Reset() override;
+
+  MediaRouterAction* action() { return action_.get(); }
+
+ protected:
+  // Use MediaRouterDialogControllerImplBase::CreateForWebContents() to create
+  // an instance.
+  explicit MediaRouterDialogControllerImplBase(
+      content::WebContents* web_contents);
+
+  // TODO(crbug.com/826091): Replace MediaRouterUI with MediaRouterUIBase.
+  void InitializeMediaRouterUI(MediaRouterUI* media_router_ui);
+
+ private:
+  // |action_| refers to the MediaRouterAction on the toolbar, rather than
+  // overflow menu. A MediaRouterAction is always created for the toolbar
+  // first. Any subsequent creations for the overflow menu will not be set as
+  // |action_|.
+  // The lifetime of |action_| is dependent on the creation and destruction of
+  // a browser window. The overflow menu's MediaRouterAction is only created
+  // when the overflow menu is opened and destroyed when the menu is closed.
+  base::WeakPtr<MediaRouterAction> action_;
+
+  // |action_controller_| is responsible for showing and hiding the toolbar
+  // action. It's owned by MediaRouterUIService, which outlives |this|.
+  MediaRouterActionController* const action_controller_;
+
+  DISALLOW_COPY_AND_ASSIGN(MediaRouterDialogControllerImplBase);
+};
+
+}  // namespace media_router
+
+#endif  // CHROME_BROWSER_UI_MEDIA_ROUTER_MEDIA_ROUTER_DIALOG_CONTROLLER_IMPL_BASE_H_

chrome/browser/ui/toolbar/media_router_action.cc

@@ -11,11 +11,11 @@
 #include "chrome/browser/media/router/media_router_metrics.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/media_router/media_router_dialog_controller_impl_base.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/toolbar/component_toolbar_actions_factory.h"
 #include "chrome/browser/ui/toolbar/media_router_action_platform_delegate.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_delegate.h"
-#include "chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl.h"
 #include "chrome/common/media_router/issue.h"
 #include "chrome/common/media_router/media_route.h"
 #include "chrome/grit/generated_resources.h"
@@ -26,7 +26,7 @@
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/gfx/vector_icon_types.h"
 
-using media_router::MediaRouterDialogControllerImpl;
+using media_router::MediaRouterDialogControllerImplBase;
 
 namespace {
 
@@ -226,7 +226,7 @@ void MediaRouterAction::OnDialogShown() {
 }
 
 void MediaRouterAction::RegisterWithDialogController() {
-  MediaRouterDialogControllerImpl* controller =
+  MediaRouterDialogControllerImplBase* controller =
       GetMediaRouterDialogController();
 
   if (!controller)
@@ -252,12 +252,12 @@ void MediaRouterAction::UpdateDialogState() {
     OnDialogHidden();
 }
 
-MediaRouterDialogControllerImpl*
+MediaRouterDialogControllerImplBase*
 MediaRouterAction::GetMediaRouterDialogController() {
   DCHECK(delegate_);
   content::WebContents* web_contents = delegate_->GetCurrentWebContents();
   DCHECK(web_contents);
-  return MediaRouterDialogControllerImpl::GetOrCreateForWebContents(
+  return MediaRouterDialogControllerImplBase::GetOrCreateForWebContents(
       web_contents);
 }
 

chrome/browser/ui/toolbar/media_router_action.h

@@ -7,6 +7,7 @@
 
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "base/macros.h"
 #include "base/scoped_observer.h"
@@ -27,7 +28,7 @@ struct VectorIcon;
 }
 
 namespace media_router {
-class MediaRouterDialogControllerImpl;
+class MediaRouterDialogControllerImplBase;
 }  // namespace media_router
 
 // The class for the Media Router component action that will be shown in
@@ -86,8 +87,9 @@ class MediaRouterAction : public ToolbarActionViewController,
   void OnDialogShown();
 
  private:
-  // Registers |this| with the MediaRouterDialogControllerImpl associated with
-  // |delegate_|'s current WebContents if |this| is not shown in overflow mode.
+  // Registers |this| with the MediaRouterDialogControllerImplBase associated
+  // with |delegate_|'s current WebContents if |this| is not shown in overflow
+  // mode.
   void RegisterWithDialogController();
 
   // Called when a new browser window is opened or when |delegate_| is swapped
@@ -96,12 +98,12 @@ class MediaRouterAction : public ToolbarActionViewController,
   // on a per-tab basis.
   void UpdateDialogState();
 
-  // Returns a reference to the MediaRouterDialogControllerImpl associated with
-  // |delegate_|'s current WebContents. Guaranteed to be non-null.
+  // Returns a reference to the MediaRouterDialogControllerImplBase associated
+  // with |delegate_|'s current WebContents. Guaranteed to be non-null.
   // |delegate_| and its current WebContents must not be null.
   // Marked virtual for tests.
-  virtual media_router::MediaRouterDialogControllerImpl*
-      GetMediaRouterDialogController();
+  virtual media_router::MediaRouterDialogControllerImplBase*
+  GetMediaRouterDialogController();
 
   // Overridden by tests.
   virtual MediaRouterActionPlatformDelegate* GetPlatformDelegate();

chrome/browser/ui/toolbar/media_router_action_unittest.cc

@@ -2,14 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/ui/toolbar/media_router_action.h"
+#include <vector>
+
 #include "base/macros.h"
 #include "chrome/browser/extensions/extension_action_test_util.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/extensions/browser_action_test_util.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/toolbar/media_router_action.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_delegate.h"
-#include "chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl.h"
+#include "chrome/browser/ui/webui/media_router/media_router_dialog_controller_webui_impl.h"
 #include "chrome/browser/ui/webui/media_router/media_router_web_ui_test.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/vector_icons/vector_icons.h"
@@ -22,7 +24,7 @@
 #include "ui/gfx/paint_vector_icon.h"
 
 using content::WebContents;
-using media_router::MediaRouterDialogControllerImpl;
+using media_router::MediaRouterDialogControllerWebUIImpl;
 
 class MockToolbarActionViewDelegate : public ToolbarActionViewDelegate {
  public:
@@ -58,14 +60,14 @@ class TestMediaRouterAction : public MediaRouterAction {
   }
 
   void SetMediaRouterDialogController(
-      MediaRouterDialogControllerImpl* controller) {
+      MediaRouterDialogControllerWebUIImpl* controller) {
     DCHECK(controller);
     controller_ = controller;
   }
 
  private:
   // MediaRouterAction:
-  MediaRouterDialogControllerImpl* GetMediaRouterDialogController()
+  MediaRouterDialogControllerWebUIImpl* GetMediaRouterDialogController()
       override {
     return controller_;
   }
@@ -73,7 +75,7 @@ class TestMediaRouterAction : public MediaRouterAction {
     return platform_delegate_;
   }
 
-  MediaRouterDialogControllerImpl* controller_;
+  MediaRouterDialogControllerWebUIImpl* controller_;
   MediaRouterActionPlatformDelegate* platform_delegate_;
 };
 
@@ -318,9 +320,9 @@ TEST_F(MediaRouterActionUnitTest, IconPressedState) {
   EXPECT_EQ(1, browser()->tab_strip_model()->count());
 
   WebContents* initiator = browser()->tab_strip_model()->GetActiveWebContents();
-  MediaRouterDialogControllerImpl::CreateForWebContents(initiator);
-  MediaRouterDialogControllerImpl* dialog_controller =
-      MediaRouterDialogControllerImpl::FromWebContents(initiator);
+  MediaRouterDialogControllerWebUIImpl::CreateForWebContents(initiator);
+  MediaRouterDialogControllerWebUIImpl* dialog_controller =
+      MediaRouterDialogControllerWebUIImpl::FromWebContents(initiator);
   ASSERT_TRUE(dialog_controller);
 
   // Sets the controller to use for TestMediaRouterAction.

chrome/browser/ui/views/media_router/media_router_dialog_controller_views.cc

+// Copyright 2018 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 "chrome/browser/ui/views/media_router/media_router_dialog_controller_views.h"
+
+#include "base/feature_list.h"
+#include "chrome/browser/ui/webui/media_router/media_router_dialog_controller_webui_impl.h"
+#include "chrome/common/chrome_features.h"
+
+DEFINE_WEB_CONTENTS_USER_DATA_KEY(
+    media_router::MediaRouterDialogControllerViews);
+
+namespace media_router {
+
+// static
+MediaRouterDialogControllerImplBase*
+MediaRouterDialogControllerImplBase::GetOrCreateForWebContents(
+    content::WebContents* web_contents) {
+  if (base::FeatureList::IsEnabled(features::kViewsCastDialog)) {
+    return MediaRouterDialogControllerViews::GetOrCreateForWebContents(
+        web_contents);
+  } else {
+    return MediaRouterDialogControllerWebUIImpl::GetOrCreateForWebContents(
+        web_contents);
+  }
+}
+
+MediaRouterDialogControllerViews::~MediaRouterDialogControllerViews() {
+  Reset();
+}
+
+// static
+MediaRouterDialogControllerViews*
+MediaRouterDialogControllerViews::GetOrCreateForWebContents(
+    content::WebContents* web_contents) {
+  DCHECK(web_contents);
+  // This call does nothing if the controller already exists.
+  MediaRouterDialogControllerViews::CreateForWebContents(web_contents);
+  return MediaRouterDialogControllerViews::FromWebContents(web_contents);
+}
+
+void MediaRouterDialogControllerViews::CreateMediaRouterDialog() {
+  MediaRouterDialogControllerImplBase::CreateMediaRouterDialog();
+  // TODO(crbug.com/826091): Implement this method.
+}
+
+void MediaRouterDialogControllerViews::CloseMediaRouterDialog() {
+  // TODO(crbug.com/826091): Implement this method.
+}
+
+bool MediaRouterDialogControllerViews::IsShowingMediaRouterDialog() const {
+  // TODO(crbug.com/826091): Implement this method.
+  return false;
+}
+
+void MediaRouterDialogControllerViews::Reset() {
+  MediaRouterDialogControllerImplBase::Reset();
+}
+
+MediaRouterDialogControllerViews::MediaRouterDialogControllerViews(
+    content::WebContents* web_contents)
+    : MediaRouterDialogControllerImplBase(web_contents) {}
+
+}  // namespace media_router

chrome/browser/ui/views/media_router/media_router_dialog_controller_views.h

+// Copyright 2018 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_MEDIA_ROUTER_MEDIA_ROUTER_DIALOG_CONTROLLER_VIEWS_H_
+#define CHROME_BROWSER_UI_VIEWS_MEDIA_ROUTER_MEDIA_ROUTER_DIALOG_CONTROLLER_VIEWS_H_
+
+#include "base/macros.h"
+#include "chrome/browser/ui/media_router/media_router_dialog_controller_impl_base.h"
+#include "content/public/browser/web_contents_user_data.h"
+
+namespace media_router {
+
+// A Views implementation of MediaRouterDialogController.
+class MediaRouterDialogControllerViews
+    : public content::WebContentsUserData<MediaRouterDialogControllerViews>,
+      public MediaRouterDialogControllerImplBase {
+ public:
+  ~MediaRouterDialogControllerViews() override;
+
+  static MediaRouterDialogControllerViews* GetOrCreateForWebContents(
+      content::WebContents* web_contents);
+
+  // MediaRouterDialogController:
+  void CreateMediaRouterDialog() override;
+  void CloseMediaRouterDialog() override;
+  bool IsShowingMediaRouterDialog() const override;
+  void Reset() override;
+
+ private:
+  friend class content::WebContentsUserData<MediaRouterDialogControllerViews>;
+
+  // Use MediaRouterDialogController::GetOrCreateForWebContents() to create
+  // an instance.
+  explicit MediaRouterDialogControllerViews(content::WebContents* web_contents);
+
+  DISALLOW_COPY_AND_ASSIGN(MediaRouterDialogControllerViews);
+};
+
+}  // namespace media_router
+
+#endif  // CHROME_BROWSER_UI_VIEWS_MEDIA_ROUTER_MEDIA_ROUTER_DIALOG_CONTROLLER_VIEWS_H_

chrome/browser/ui/views/media_router/media_router_ui_browsertest.cc

@@ -12,13 +12,13 @@
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/extensions/browser_action_test_util.h"
+#include "chrome/browser/ui/media_router/media_router_dialog_controller_impl_base.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/toolbar/component_toolbar_actions_factory.h"
 #include "chrome/browser/ui/toolbar/media_router_action.h"
 #include "chrome/browser/ui/toolbar/media_router_action_controller.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_delegate.h"
 #include "chrome/browser/ui/views/media_router/app_menu_test_api.h"
-#include "chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl.h"
 #include "chrome/browser/ui/webui/media_router/media_router_ui_service.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
@@ -79,7 +79,7 @@ class MediaRouterUIBrowserTest : public InProcessBrowserTest {
   }
 
   MediaRouterAction* GetMediaRouterAction() {
-    return MediaRouterDialogControllerImpl::GetOrCreateForWebContents(
+    return MediaRouterDialogControllerImplBase::GetOrCreateForWebContents(
                browser()->tab_strip_model()->GetActiveWebContents())
         ->action();
   }

chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl.h → chrome/browser/ui/webui/media_router/media_router_dialog_controller_webui_impl.h

@@ -2,61 +2,55 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_UI_WEBUI_MEDIA_ROUTER_MEDIA_ROUTER_DIALOG_CONTROLLER_IMPL_H_
-#define CHROME_BROWSER_UI_WEBUI_MEDIA_ROUTER_MEDIA_ROUTER_DIALOG_CONTROLLER_IMPL_H_
+#ifndef CHROME_BROWSER_UI_WEBUI_MEDIA_ROUTER_MEDIA_ROUTER_DIALOG_CONTROLLER_WEBUI_IMPL_H_
+#define CHROME_BROWSER_UI_WEBUI_MEDIA_ROUTER_MEDIA_ROUTER_DIALOG_CONTROLLER_WEBUI_IMPL_H_
+
+#include <memory>
 
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
-#include "chrome/browser/media/router/media_router_dialog_controller.h"
+#include "chrome/browser/ui/media_router/media_router_dialog_controller_impl_base.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
 
 FORWARD_DECLARE_TEST(MediaRouterActionUnitTest, IconPressedState);
 
-class MediaRouterAction;
-class MediaRouterActionController;
-
 namespace media_router {
 
-// A desktop implementation of MediaRouterDialogController.
+// A WebUI implementation of MediaRouterDialogController.
 // This class is not thread safe and must be called on the UI thread.
-class MediaRouterDialogControllerImpl :
-    public content::WebContentsUserData<MediaRouterDialogControllerImpl>,
-    public MediaRouterDialogController {
+class MediaRouterDialogControllerWebUIImpl
+    : public content::WebContentsUserData<MediaRouterDialogControllerWebUIImpl>,
+      public MediaRouterDialogControllerImplBase {
  public:
-  ~MediaRouterDialogControllerImpl() override;
-
-  static MediaRouterDialogControllerImpl* GetOrCreateForWebContents(
+  static MediaRouterDialogControllerWebUIImpl* GetOrCreateForWebContents(
       content::WebContents* web_contents);
 
+  ~MediaRouterDialogControllerWebUIImpl() override;
+
   // Returns the media router dialog WebContents.
   // Returns nullptr if there is no dialog.
   content::WebContents* GetMediaRouterDialog() const;
 
-  // Sets the action to notify when a dialog gets shown or hidden.
-  void SetMediaRouterAction(const base::WeakPtr<MediaRouterAction>& action);
-
   // MediaRouterDialogController:
+  void CreateMediaRouterDialog() override;
+  void CloseMediaRouterDialog() override;
   bool IsShowingMediaRouterDialog() const override;
+  void Reset() override;
 
   void UpdateMaxDialogSize();
 
-  MediaRouterAction* action() { return action_.get(); }
-
  private:
   class DialogWebContentsObserver;
-  friend class content::WebContentsUserData<MediaRouterDialogControllerImpl>;
+  friend class content::WebContentsUserData<
+      MediaRouterDialogControllerWebUIImpl>;
   FRIEND_TEST_ALL_PREFIXES(::MediaRouterActionUnitTest, IconPressedState);
 
-  // Use MediaRouterDialogControllerImpl::CreateForWebContents() to create an
-  // instance.
-  explicit MediaRouterDialogControllerImpl(content::WebContents* web_contents);
-
-  // MediaRouterDialogController:
-  void CreateMediaRouterDialog() override;
-  void CloseMediaRouterDialog() override;
-  void Reset() override;
+  // Use MediaRouterDialogControllerWebUIImpl::CreateForWebContents() to create
+  // an instance.
+  explicit MediaRouterDialogControllerWebUIImpl(
+      content::WebContents* web_contents);
 
   // Invoked when the dialog WebContents has navigated.
   void OnDialogNavigated(const content::LoadCommittedDetails& details);
@@ -69,24 +63,11 @@ class MediaRouterDialogControllerImpl :
   // created.
   bool media_router_dialog_pending_;
 
-  // |action_| refers to the MediaRouterAction on the toolbar, rather than
-  // overflow menu. A MediaRouterAction is always created for the toolbar
-  // first. Any subsequent creations for the overflow menu will not be set as
-  // |action_|.
-  // The lifetime of |action_| is dependent on the creation and destruction of
-  // a browser window. The overflow menu's MediaRouterAction is only created
-  // when the overflow menu is opened and destroyed when the menu is closed.
-  base::WeakPtr<MediaRouterAction> action_;
-
-  // |action_controller_| is responsible for showing and hiding the toolbar
-  // action. It's owned by MediaRouterUIService, which outlives |this|.
-  MediaRouterActionController* action_controller_;
-
-  base::WeakPtrFactory<MediaRouterDialogControllerImpl> weak_ptr_factory_;
+  base::WeakPtrFactory<MediaRouterDialogControllerWebUIImpl> weak_ptr_factory_;
 
-  DISALLOW_COPY_AND_ASSIGN(MediaRouterDialogControllerImpl);
+  DISALLOW_COPY_AND_ASSIGN(MediaRouterDialogControllerWebUIImpl);
 };
 
 }  // namespace media_router
 
-#endif  // CHROME_BROWSER_UI_WEBUI_MEDIA_ROUTER_MEDIA_ROUTER_DIALOG_CONTROLLER_IMPL_H_
+#endif  // CHROME_BROWSER_UI_WEBUI_MEDIA_ROUTER_MEDIA_ROUTER_DIALOG_CONTROLLER_WEBUI_IMPL_H_

chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl_browsertest.cc → chrome/browser/ui/webui/media_router/media_router_dialog_controller_webui_impl_browsertest.cc

@@ -6,7 +6,7 @@
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
-#include "chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl.h"
+#include "chrome/browser/ui/webui/media_router/media_router_dialog_controller_webui_impl.h"
 #include "chrome/browser/ui/webui/media_router/media_router_ui.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -19,13 +19,14 @@ using content::TestNavigationObserver;
 
 namespace media_router {
 
-class MediaRouterDialogControllerBrowserTest : public InProcessBrowserTest {
+class MediaRouterDialogControllerWebUIBrowserTest
+    : public InProcessBrowserTest {
  public:
-  MediaRouterDialogControllerBrowserTest()
+  MediaRouterDialogControllerWebUIBrowserTest()
       : dialog_controller_(nullptr),
         initiator_(nullptr),
         media_router_dialog_(nullptr) {}
-  ~MediaRouterDialogControllerBrowserTest() override {}
+  ~MediaRouterDialogControllerWebUIBrowserTest() override {}
 
  protected:
   void SetUpOnMainThread() override {
@@ -35,9 +36,9 @@ class MediaRouterDialogControllerBrowserTest : public InProcessBrowserTest {
 
     initiator_ = browser()->tab_strip_model()->GetActiveWebContents();
     ASSERT_TRUE(initiator_);
-    MediaRouterDialogControllerImpl::CreateForWebContents(initiator_);
+    MediaRouterDialogControllerWebUIImpl::CreateForWebContents(initiator_);
     dialog_controller_ =
-        MediaRouterDialogControllerImpl::FromWebContents(initiator_);
+        MediaRouterDialogControllerWebUIImpl::FromWebContents(initiator_);
     ASSERT_TRUE(dialog_controller_);
 
     // Get the media router dialog for the initiator.
@@ -46,15 +47,16 @@ class MediaRouterDialogControllerBrowserTest : public InProcessBrowserTest {
     ASSERT_TRUE(media_router_dialog_);
   }
 
-  MediaRouterDialogControllerImpl* dialog_controller_;
+  MediaRouterDialogControllerWebUIImpl* dialog_controller_;
   WebContents* initiator_;
   WebContents* media_router_dialog_;
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(MediaRouterDialogControllerBrowserTest);
+  DISALLOW_COPY_AND_ASSIGN(MediaRouterDialogControllerWebUIBrowserTest);
 };
 
-IN_PROC_BROWSER_TEST_F(MediaRouterDialogControllerBrowserTest, ShowDialog) {
+IN_PROC_BROWSER_TEST_F(MediaRouterDialogControllerWebUIBrowserTest,
+                       ShowDialog) {
   // Waits for the dialog to initialize.
   TestNavigationObserver nav_observer(media_router_dialog_);
   nav_observer.Wait();
@@ -72,7 +74,7 @@ IN_PROC_BROWSER_TEST_F(MediaRouterDialogControllerBrowserTest, ShowDialog) {
   ASSERT_TRUE(media_router_ui);
 }
 
-IN_PROC_BROWSER_TEST_F(MediaRouterDialogControllerBrowserTest, Navigate) {
+IN_PROC_BROWSER_TEST_F(MediaRouterDialogControllerWebUIBrowserTest, Navigate) {
   {
     // Wait for the dialog to initialize.
     TestNavigationObserver nav_observer(media_router_dialog_);
@@ -115,8 +117,8 @@ IN_PROC_BROWSER_TEST_F(MediaRouterDialogControllerBrowserTest, Navigate) {
   EXPECT_FALSE(dialog_controller_->GetMediaRouterDialog());
 }
 
-IN_PROC_BROWSER_TEST_F(MediaRouterDialogControllerBrowserTest,
-    RenderProcessHost) {
+IN_PROC_BROWSER_TEST_F(MediaRouterDialogControllerWebUIBrowserTest,
+                       RenderProcessHost) {
   // New media router dialog is a constrained window, so the number of
   // tabs is still 1.
   EXPECT_EQ(1, browser()->tab_strip_model()->count());
@@ -124,8 +126,8 @@ IN_PROC_BROWSER_TEST_F(MediaRouterDialogControllerBrowserTest,
 
   // Crash initiator_'s renderer process.
   content::WebContentsDestroyedWatcher dialog_watcher(media_router_dialog_);
-  content::RenderProcessHostWatcher rph_watcher(initiator_,
-      content::RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
+  content::RenderProcessHostWatcher rph_watcher(
+      initiator_, content::RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
 
   ui_test_utils::NavigateToURL(browser(), GURL(content::kChromeUICrashURL));
 

chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl_unittest.cc → chrome/browser/ui/webui/media_router/media_router_dialog_controller_webui_impl_unittest.cc

@@ -10,7 +10,7 @@
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/toolbar/mock_media_router_action_controller.h"
-#include "chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl.h"
+#include "chrome/browser/ui/webui/media_router/media_router_dialog_controller_webui_impl.h"
 #include "chrome/browser/ui/webui/media_router/media_router_ui.h"
 #include "chrome/browser/ui/webui/media_router/media_router_ui_service.h"
 #include "chrome/browser/ui/webui/media_router/media_router_web_ui_test.h"
@@ -21,10 +21,10 @@ using content::WebContents;
 
 namespace media_router {
 
-class MediaRouterDialogControllerImplTest : public MediaRouterWebUITest {
+class MediaRouterDialogControllerWebUIImplTest : public MediaRouterWebUITest {
  public:
-  MediaRouterDialogControllerImplTest() : MediaRouterWebUITest(true) {}
-  ~MediaRouterDialogControllerImplTest() override {}
+  MediaRouterDialogControllerWebUIImplTest() : MediaRouterWebUITest(true) {}
+  ~MediaRouterDialogControllerWebUIImplTest() override {}
 
   void OpenMediaRouterDialog();
 
@@ -35,14 +35,14 @@ class MediaRouterDialogControllerImplTest : public MediaRouterWebUITest {
 
  protected:
   WebContents* initiator_ = nullptr;
-  MediaRouterDialogControllerImpl* dialog_controller_ = nullptr;
+  MediaRouterDialogControllerWebUIImpl* dialog_controller_ = nullptr;
   WebContents* media_router_dialog_ = nullptr;
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(MediaRouterDialogControllerImplTest);
+  DISALLOW_COPY_AND_ASSIGN(MediaRouterDialogControllerWebUIImplTest);
 };
 
-void MediaRouterDialogControllerImplTest::OpenMediaRouterDialog() {
+void MediaRouterDialogControllerWebUIImplTest::OpenMediaRouterDialog() {
   // Start with one window with one tab.
   EXPECT_EQ(1u, chrome::GetTotalBrowserCount());
   EXPECT_EQ(0, browser()->tab_strip_model()->count());
@@ -53,7 +53,8 @@ void MediaRouterDialogControllerImplTest::OpenMediaRouterDialog() {
   initiator_ = browser()->tab_strip_model()->GetActiveWebContents();
 
   dialog_controller_ =
-      MediaRouterDialogControllerImpl::GetOrCreateForWebContents(initiator_);
+      MediaRouterDialogControllerWebUIImpl::GetOrCreateForWebContents(
+          initiator_);
   ASSERT_TRUE(dialog_controller_);
 
   // Get the media router dialog for the initiator.
@@ -69,7 +70,7 @@ void MediaRouterDialogControllerImplTest::OpenMediaRouterDialog() {
 }
 
 // Create/Get a media router dialog for initiator.
-TEST_F(MediaRouterDialogControllerImplTest, ShowMediaRouterDialog) {
+TEST_F(MediaRouterDialogControllerWebUIImplTest, ShowMediaRouterDialog) {
   OpenMediaRouterDialog();
 
   // Show media router dialog for the same initiator again.
@@ -88,7 +89,7 @@ TEST_F(MediaRouterDialogControllerImplTest, ShowMediaRouterDialog) {
 // Tests multiple media router dialogs exist in the same browser for different
 // initiators. If a dialog already exists for an initiator, that initiator
 // gets focused.
-TEST_F(MediaRouterDialogControllerImplTest, MultipleMediaRouterDialogs) {
+TEST_F(MediaRouterDialogControllerWebUIImplTest, MultipleMediaRouterDialogs) {
   // Let's start with one window and two tabs.
   EXPECT_EQ(1u, chrome::GetTotalBrowserCount());
   TabStripModel* tab_strip_model = browser()->tab_strip_model();
@@ -106,10 +107,9 @@ TEST_F(MediaRouterDialogControllerImplTest, MultipleMediaRouterDialogs) {
   ASSERT_TRUE(web_contents_2);
   EXPECT_EQ(2, tab_strip_model->count());
 
-
   // Create media router dialog for |web_contents_1|.
-  MediaRouterDialogControllerImpl* dialog_controller_1 =
-      MediaRouterDialogControllerImpl::GetOrCreateForWebContents(
+  MediaRouterDialogControllerWebUIImpl* dialog_controller_1 =
+      MediaRouterDialogControllerWebUIImpl::GetOrCreateForWebContents(
           web_contents_1);
   ASSERT_TRUE(dialog_controller_1);
 
@@ -123,8 +123,8 @@ TEST_F(MediaRouterDialogControllerImplTest, MultipleMediaRouterDialogs) {
   EXPECT_EQ(2, tab_strip_model->count());
 
   // Create media router dialog for |web_contents_2|.
-  MediaRouterDialogControllerImpl* dialog_controller_2 =
-      MediaRouterDialogControllerImpl::GetOrCreateForWebContents(
+  MediaRouterDialogControllerWebUIImpl* dialog_controller_2 =
+      MediaRouterDialogControllerWebUIImpl::GetOrCreateForWebContents(
           web_contents_2);
   ASSERT_TRUE(dialog_controller_2);
 
@@ -166,7 +166,7 @@ TEST_F(MediaRouterDialogControllerImplTest, MultipleMediaRouterDialogs) {
   EXPECT_EQ(tab_2_index, tab_strip_model->active_index());
 }
 
-TEST_F(MediaRouterDialogControllerImplTest, CloseDialogFromWebUI) {
+TEST_F(MediaRouterDialogControllerWebUIImplTest, CloseDialogFromWebUI) {
   OpenMediaRouterDialog();
 
   // Close the dialog.
@@ -199,7 +199,8 @@ TEST_F(MediaRouterDialogControllerImplTest, CloseDialogFromWebUI) {
   EXPECT_EQ(media_router_dialog_2, dialog_controller_->GetMediaRouterDialog());
 }
 
-TEST_F(MediaRouterDialogControllerImplTest, CloseDialogFromDialogController) {
+TEST_F(MediaRouterDialogControllerWebUIImplTest,
+       CloseDialogFromDialogController) {
   OpenMediaRouterDialog();
 
   // Close the dialog.
@@ -217,15 +218,15 @@ TEST_F(MediaRouterDialogControllerImplTest, CloseDialogFromDialogController) {
   EXPECT_FALSE(dialog_controller_->GetMediaRouterDialog());
 }
 
-TEST_F(MediaRouterDialogControllerImplTest, CloseInitiator) {
+TEST_F(MediaRouterDialogControllerWebUIImplTest, CloseInitiator) {
   OpenMediaRouterDialog();
 
   // Close the initiator. This should also close the dialog WebContents.
   content::WebContentsDestroyedWatcher initiator_watcher(initiator_);
   content::WebContentsDestroyedWatcher dialog_watcher(media_router_dialog_);
 
-  int initiator_index = browser()->tab_strip_model()
-      ->GetIndexOfWebContents(initiator_);
+  int initiator_index =
+      browser()->tab_strip_model()->GetIndexOfWebContents(initiator_);
   EXPECT_NE(-1, initiator_index);
   EXPECT_TRUE(browser()->tab_strip_model()->CloseWebContentsAt(
       initiator_index, TabStripModel::CLOSE_NONE));
@@ -240,7 +241,7 @@ TEST_F(MediaRouterDialogControllerImplTest, CloseInitiator) {
   // The dialog controller is deleted when the WebContents is closed/destroyed.
 }
 
-TEST_F(MediaRouterDialogControllerImplTest, NotifyActionController) {
+TEST_F(MediaRouterDialogControllerWebUIImplTest, NotifyActionController) {
   MockMediaRouterActionController* action_controller =
       static_cast<MockMediaRouterActionController*>(
           MediaRouterUIService::Get(browser()->profile())->action_controller());
@@ -256,11 +257,11 @@ TEST_F(MediaRouterDialogControllerImplTest, NotifyActionController) {
       content::PresentationRequest(
           {1, 2}, {GURL("http://test.com"), GURL("http://test2.com")},
           url::Origin::Create(GURL("http://example.com"))),
-      base::Bind(
-          &MediaRouterDialogControllerImplTest::PresentationSuccessCallback,
-          base::Unretained(this)),
-      base::Bind(
-          &MediaRouterDialogControllerImplTest::PresentationErrorCallback,
+      base::BindOnce(&MediaRouterDialogControllerWebUIImplTest::
+                         PresentationSuccessCallback,
+                     base::Unretained(this)),
+      base::BindOnce(
+          &MediaRouterDialogControllerWebUIImplTest::PresentationErrorCallback,
           base::Unretained(this))));
 
   // When |dialog_controller_| is destroyed with its dialog open,

chrome/test/BUILD.gn

@@ -1336,7 +1336,7 @@ test("browser_tests") {
         "../browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model_browsertest_win.cc",
         "../browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_test_utils.cc",
         "../browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_test_utils.h",
-        "../browser/ui/webui/media_router/media_router_dialog_controller_impl_browsertest.cc",
+        "../browser/ui/webui/media_router/media_router_dialog_controller_webui_impl_browsertest.cc",
       ]
 
       if (enable_background_mode) {
@@ -3121,7 +3121,7 @@ test("unit_tests") {
       "../browser/ui/toolbar/media_router_contextual_menu_unittest.cc",
       "../browser/ui/toolbar/mock_media_router_action_controller.cc",
       "../browser/ui/toolbar/mock_media_router_action_controller.h",
-      "../browser/ui/webui/media_router/media_router_dialog_controller_impl_unittest.cc",
+      "../browser/ui/webui/media_router/media_router_dialog_controller_webui_impl_unittest.cc",
       "../browser/ui/webui/media_router/media_router_ui_service_factory_unittest.cc",
       "../browser/ui/webui/media_router/media_router_ui_unittest.cc",
       "../browser/ui/webui/media_router/media_router_web_ui_test.cc",

chrome/test/media_router/media_router_integration_browsertest.cc

@@ -22,7 +22,7 @@
 #include "chrome/browser/ui/media_router/media_router_file_dialog.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
-#include "chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl.h"
+#include "chrome/browser/ui/webui/media_router/media_router_dialog_controller_webui_impl.h"
 #include "chrome/browser/ui/webui/media_router/media_router_ui.h"
 #include "chrome/common/media_router/issue.h"
 #include "chrome/common/url_constants.h"
@@ -363,8 +363,9 @@ void MediaRouterIntegrationBrowserTest::ClickHeader(
 
 WebContents* MediaRouterIntegrationBrowserTest::GetMRDialog(
     WebContents* web_contents) {
-  MediaRouterDialogControllerImpl* controller =
-      MediaRouterDialogControllerImpl::GetOrCreateForWebContents(web_contents);
+  MediaRouterDialogControllerWebUIImpl* controller =
+      MediaRouterDialogControllerWebUIImpl::GetOrCreateForWebContents(
+          web_contents);
   WebContents* dialog_contents = controller->GetMediaRouterDialog();
   CHECK(dialog_contents);
   WaitUntilDialogFullyLoaded(dialog_contents);
@@ -373,8 +374,9 @@ WebContents* MediaRouterIntegrationBrowserTest::GetMRDialog(
 
 bool MediaRouterIntegrationBrowserTest::IsDialogClosed(
     WebContents* web_contents) {
-  MediaRouterDialogControllerImpl* controller =
-      MediaRouterDialogControllerImpl::GetOrCreateForWebContents(web_contents);
+  MediaRouterDialogControllerWebUIImpl* controller =
+      MediaRouterDialogControllerWebUIImpl::GetOrCreateForWebContents(
+          web_contents);
   return !controller->GetMediaRouterDialog();
 }
 
@@ -417,8 +419,9 @@ void MediaRouterIntegrationBrowserTest::SetTestData(
 
 WebContents* MediaRouterIntegrationBrowserTest::OpenMRDialog(
     WebContents* web_contents) {
-  MediaRouterDialogControllerImpl* controller =
-      MediaRouterDialogControllerImpl::GetOrCreateForWebContents(web_contents);
+  MediaRouterDialogControllerWebUIImpl* controller =
+      MediaRouterDialogControllerWebUIImpl::GetOrCreateForWebContents(
+          web_contents);
   test_navigation_observer_.reset(
         new content::TestNavigationObserver(web_contents, 1));
   test_navigation_observer_->StartWatchingNewWebContents();
@@ -530,8 +533,9 @@ bool MediaRouterIntegrationBrowserTest::IsRouteClosedOnUI() {
   // after 3s. But sometimes it takes more than 3s to close the route, so
   // we need to re-open the dialog if it is closed.
   WebContents* web_contents = GetActiveWebContents();
-  MediaRouterDialogControllerImpl* controller =
-      MediaRouterDialogControllerImpl::GetOrCreateForWebContents(web_contents);
+  MediaRouterDialogControllerWebUIImpl* controller =
+      MediaRouterDialogControllerWebUIImpl::GetOrCreateForWebContents(
+          web_contents);
   WebContents* dialog_contents = controller->GetMediaRouterDialog();
   if (!dialog_contents) {
     VLOG(0) << "Media router dialog was closed, reopen it again.";
@@ -598,11 +602,12 @@ void MediaRouterIntegrationBrowserTest::CheckSessionValidity(
   EXPECT_EQ(session_id, default_request_session_id);
 }
 
-MediaRouterDialogControllerImpl*
+MediaRouterDialogControllerWebUIImpl*
 MediaRouterIntegrationBrowserTest::GetControllerForShownDialog(
     WebContents* web_contents) {
-  MediaRouterDialogControllerImpl* controller =
-      MediaRouterDialogControllerImpl::GetOrCreateForWebContents(web_contents);
+  MediaRouterDialogControllerWebUIImpl* controller =
+      MediaRouterDialogControllerWebUIImpl::GetOrCreateForWebContents(
+          web_contents);
   EXPECT_TRUE(controller->IsShowingMediaRouterDialog());
   return controller;
 }

chrome/test/media_router/media_router_integration_browsertest.h

@@ -19,6 +19,7 @@
 
 namespace media_router {
 
+class MediaRouterDialogControllerWebUIImpl;
 class MediaRouterUI;
 struct IssueInfo;
 
@@ -170,7 +171,7 @@ class MediaRouterIntegrationBrowserTest : public MediaRouterBaseBrowserTest {
 
   // Checks that a Media Router dialog is shown for |web_contents|, and returns
   // its controller.
-  MediaRouterDialogControllerImpl* GetControllerForShownDialog(
+  MediaRouterDialogControllerWebUIImpl* GetControllerForShownDialog(
       content::WebContents* web_contents);
 
   // Returns the active WebContents for the current window.