Delete MediaRouterDialogControllerImplBase

Move logic from MediaRouterDialogControllerImplBase to
MediaRouterDialogControllerViews, and delete the former class.

Bug: 900246
Change-Id: I5216a4f7bbbbda9ea19fbed1d7dcbbd8aa32727c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1691745
Commit-Queue: Takumi Fujimoto <takumif@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676192}

chrome/browser/media/android/router/media_router_dialog_controller_android.cc

@@ -35,8 +35,8 @@ using content::WebContents;
 namespace media_router {
 
 // static
-MediaRouterDialogControllerAndroid*
-MediaRouterDialogControllerAndroid::GetOrCreateForWebContents(
+MediaRouterDialogController*
+MediaRouterDialogController::GetOrCreateForWebContents(
     WebContents* web_contents) {
   DCHECK(web_contents);
   // This call does nothing if the controller already exists.

chrome/browser/media/android/router/media_router_dialog_controller_android.h

@@ -21,9 +21,6 @@ class MediaRouterDialogControllerAndroid
  public:
   ~MediaRouterDialogControllerAndroid() override;
 
-  static MediaRouterDialogControllerAndroid* GetOrCreateForWebContents(
-      content::WebContents* web_contents);
-
   // The methods called by the Java counterpart.
 
   // Notifies the controller that user has selected a sink with |jsink_id| for

chrome/browser/media/router/media_router_dialog_controller.cc

@@ -14,27 +14,8 @@
 #include "content/public/browser/web_contents_delegate.h"
 #include "third_party/blink/public/mojom/presentation/presentation.mojom.h"
 
-#if defined(OS_ANDROID)
-#include "chrome/browser/media/android/router/media_router_dialog_controller_android.h"
-#else
-#include "chrome/browser/ui/media_router/media_router_dialog_controller_impl_base.h"
-#endif
-
 namespace media_router {
 
-// static
-MediaRouterDialogController*
-MediaRouterDialogController::GetOrCreateForWebContents(
-    content::WebContents* contents) {
-#if defined(OS_ANDROID)
-  return MediaRouterDialogControllerAndroid::GetOrCreateForWebContents(
-      contents);
-#else
-  return MediaRouterDialogControllerImplBase::GetOrCreateForWebContents(
-      contents);
-#endif
-}
-
 class MediaRouterDialogController::InitiatorWebContentsObserver
     : public content::WebContentsObserver {
  public:

chrome/browser/media/router/media_router_dialog_controller.h

@@ -33,7 +33,9 @@ class MediaRouterDialogController {
 
   // Gets a reference to the MediaRouterDialogController associated with
   // |web_contents|, creating one if it does not exist. The returned pointer is
-  // guaranteed to be non-null.
+  // guaranteed to be non-null. This method has platform-specific
+  // implementations in directories such as chrome/browser/ui/views/ and
+  // chrome/browser/media/android/.
   static MediaRouterDialogController* GetOrCreateForWebContents(
       content::WebContents* web_contents);
 

chrome/browser/ui/BUILD.gn

@@ -962,8 +962,6 @@ jumbo_split_static_library("ui") {
       "media_router/cloud_services_dialog.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_router_ui_base.cc",

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/media_router/media_router_ui_base.h"
-#include "chrome/browser/ui/media_router/media_router_ui_service.h"
-
-using content::WebContents;
-
-namespace media_router {
-
-namespace {
-
-MediaRouterUIService* GetMediaRouterUIService(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);
-}
-
-}  // namespace
-
-MediaRouterDialogControllerImplBase::~MediaRouterDialogControllerImplBase() {
-  media_router_ui_service_->RemoveObserver(this);
-}
-
-void MediaRouterDialogControllerImplBase::CreateMediaRouterDialog() {
-  if (!GetActionController())
-    return;
-  GetActionController()->OnDialogShown();
-}
-
-void MediaRouterDialogControllerImplBase::Reset() {
-  if (IsShowingMediaRouterDialog() && GetActionController())
-    GetActionController()->OnDialogHidden();
-  MediaRouterDialogController::Reset();
-}
-
-MediaRouterDialogControllerImplBase::MediaRouterDialogControllerImplBase(
-    WebContents* web_contents)
-    : MediaRouterDialogController(web_contents),
-      media_router_ui_service_(GetMediaRouterUIService(web_contents)) {
-  DCHECK(media_router_ui_service_);
-  media_router_ui_service_->AddObserver(this);
-}
-
-void MediaRouterDialogControllerImplBase::InitializeMediaRouterUI(
-    MediaRouterUIBase* 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));
-  }
-}
-
-void MediaRouterDialogControllerImplBase::OnServiceDisabled() {
-  CloseMediaRouterDialog();
-  Reset();
-}
-
-MediaRouterActionController*
-MediaRouterDialogControllerImplBase::GetActionController() {
-  return media_router_ui_service_->action_controller();
-}
-
-}  // 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"
-#include "chrome/browser/ui/media_router/media_router_ui_service.h"
-
-namespace media_router {
-
-class MediaRouterUIBase;
-
-// 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,
-                                            MediaRouterUIService::Observer {
- public:
-  ~MediaRouterDialogControllerImplBase() override;
-
-  static MediaRouterDialogControllerImplBase* GetOrCreateForWebContents(
-      content::WebContents* web_contents);
-  static MediaRouterDialogControllerImplBase* FromWebContents(
-      content::WebContents* web_contents);
-
-  // MediaRouterDialogController:
-  void CreateMediaRouterDialog() override;
-  void Reset() override;
-
- protected:
-  // Use MediaRouterDialogControllerImplBase::CreateForWebContents() to create
-  // an instance.
-  explicit MediaRouterDialogControllerImplBase(
-      content::WebContents* web_contents);
-
-  // Called by subclasses to initialize |media_router_ui| that they use.
-  void InitializeMediaRouterUI(MediaRouterUIBase* media_router_ui);
-
- private:
-  // MediaRouterUIService::Observer:
-  void OnServiceDisabled() override;
-
-  // MediaRouterActionController is responsible for showing and hiding the
-  // toolbar action. It's owned by MediaRouterUIService and it may be nullptr.
-  MediaRouterActionController* GetActionController();
-
-  // |media_router_ui_service_| Service which provides
-  // MediaRouterActionController. It outlives |this|.
-  MediaRouterUIService* const media_router_ui_service_;
-
-  DISALLOW_COPY_AND_ASSIGN(MediaRouterDialogControllerImplBase);
-};
-
-}  // namespace media_router
-
-#endif  // CHROME_BROWSER_UI_MEDIA_ROUTER_MEDIA_ROUTER_DIALOG_CONTROLLER_IMPL_BASE_H_

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

@@ -13,50 +13,45 @@
 #include "chrome/browser/ui/views/frame/top_container_view.h"
 #include "chrome/browser/ui/views/media_router/cast_dialog_view.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
+#include "content/public/browser/web_contents.h"
+
+using content::WebContents;
 
 namespace media_router {
 
-// static
-MediaRouterDialogControllerImplBase*
-MediaRouterDialogControllerImplBase::GetOrCreateForWebContents(
-    content::WebContents* web_contents) {
-  return MediaRouterDialogControllerViews::GetOrCreateForWebContents(
-      web_contents);
+namespace {
+
+MediaRouterUIService* GetMediaRouterUIService(WebContents* web_contents) {
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  return MediaRouterUIService::Get(profile);
 }
 
+}  // namespace
+
 // static
-MediaRouterDialogControllerImplBase*
-MediaRouterDialogControllerImplBase::FromWebContents(
+MediaRouterDialogController*
+MediaRouterDialogController::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);
 }
 
 MediaRouterDialogControllerViews::~MediaRouterDialogControllerViews() {
   Reset();
-  if (dialog_widget_) {
-    dialog_widget_->RemoveObserver(this);
-    dialog_widget_ = nullptr;
-  }
-}
-
-// static
-MediaRouterDialogControllerViews*
-MediaRouterDialogControllerViews::GetOrCreateForWebContents(
-    content::WebContents* web_contents) {
-  DCHECK(web_contents);
-  // This call does nothing if the controller already exists.
-  CreateForWebContents(web_contents);
-  return FromWebContents(web_contents);
+  media_router_ui_service_->RemoveObserver(this);
 }
 
 void MediaRouterDialogControllerViews::CreateMediaRouterDialog() {
   base::Time dialog_creation_time = base::Time::Now();
-  MediaRouterDialogControllerImplBase::CreateMediaRouterDialog();
+  if (GetActionController())
+    GetActionController()->OnDialogShown();
 
   Profile* profile =
       Profile::FromBrowserContext(initiator()->GetBrowserContext());
-  ui_ = std::make_unique<MediaRouterViewsUI>();
-  InitializeMediaRouterUI(ui_.get());
+  InitializeMediaRouterUI();
 
   Browser* browser = chrome::FindBrowserWithWebContents(initiator());
   if (browser) {
@@ -76,8 +71,7 @@ void MediaRouterDialogControllerViews::CreateMediaRouterDialog() {
     CastDialogView::ShowDialogCentered(anchor_bounds, ui_.get(), profile,
                                        dialog_creation_time);
   }
-  dialog_widget_ = CastDialogView::GetCurrentDialogWidget();
-  dialog_widget_->AddObserver(this);
+  scoped_widget_observer_.Add(CastDialogView::GetCurrentDialogWidget());
   if (dialog_creation_callback_)
     dialog_creation_callback_.Run();
 }
@@ -93,16 +87,17 @@ bool MediaRouterDialogControllerViews::IsShowingMediaRouterDialog() const {
 void MediaRouterDialogControllerViews::Reset() {
   // If |ui_| is null, Reset() has already been called.
   if (ui_) {
-    MediaRouterDialogControllerImplBase::Reset();
+    if (IsShowingMediaRouterDialog() && GetActionController())
+      GetActionController()->OnDialogHidden();
     ui_.reset();
+    MediaRouterDialogController::Reset();
   }
 }
 
 void MediaRouterDialogControllerViews::OnWidgetClosing(views::Widget* widget) {
-  DCHECK_EQ(dialog_widget_, widget);
+  DCHECK(scoped_widget_observer_.IsObserving(widget));
   Reset();
-  dialog_widget_->RemoveObserver(this);
-  dialog_widget_ = nullptr;
+  scoped_widget_observer_.Remove(widget);
 }
 
 void MediaRouterDialogControllerViews::SetDialogCreationCallbackForTesting(
@@ -111,8 +106,35 @@ void MediaRouterDialogControllerViews::SetDialogCreationCallbackForTesting(
 }
 
 MediaRouterDialogControllerViews::MediaRouterDialogControllerViews(
-    content::WebContents* web_contents)
-    : MediaRouterDialogControllerImplBase(web_contents) {}
+    WebContents* web_contents)
+    : MediaRouterDialogController(web_contents),
+      scoped_widget_observer_(this),
+      media_router_ui_service_(GetMediaRouterUIService(web_contents)) {
+  DCHECK(media_router_ui_service_);
+  media_router_ui_service_->AddObserver(this);
+}
+
+void MediaRouterDialogControllerViews::OnServiceDisabled() {
+  CloseMediaRouterDialog();
+  Reset();
+}
+
+void MediaRouterDialogControllerViews::InitializeMediaRouterUI() {
+  ui_ = std::make_unique<MediaRouterViewsUI>();
+  PresentationServiceDelegateImpl* delegate =
+      PresentationServiceDelegateImpl::FromWebContents(initiator());
+  if (!start_presentation_context_) {
+    ui_->InitWithDefaultMediaSource(initiator(), delegate);
+  } else {
+    ui_->InitWithStartPresentationContext(
+        initiator(), delegate, std::move(start_presentation_context_));
+  }
+}
+
+MediaRouterActionController*
+MediaRouterDialogControllerViews::GetActionController() {
+  return media_router_ui_service_->action_controller();
+}
 
 WEB_CONTENTS_USER_DATA_KEY_IMPL(MediaRouterDialogControllerViews)
 

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

@@ -9,7 +9,9 @@
 
 #include "base/macros.h"
 #include "base/observer_list.h"
-#include "chrome/browser/ui/media_router/media_router_dialog_controller_impl_base.h"
+#include "base/scoped_observer.h"
+#include "chrome/browser/media/router/media_router_dialog_controller.h"
+#include "chrome/browser/ui/media_router/media_router_ui_service.h"
 #include "chrome/browser/ui/views/media_router/media_router_views_ui.h"
 #include "ui/views/widget/widget_observer.h"
 
@@ -18,16 +20,12 @@ namespace media_router {
 // A Views implementation of MediaRouterDialogController.
 class MediaRouterDialogControllerViews
     : public content::WebContentsUserData<MediaRouterDialogControllerViews>,
-      public MediaRouterDialogControllerImplBase,
-      public views::WidgetObserver {
+      public MediaRouterDialogController,
+      public views::WidgetObserver,
+      public MediaRouterUIService::Observer {
  public:
   ~MediaRouterDialogControllerViews() override;
 
-  static MediaRouterDialogControllerViews* GetOrCreateForWebContents(
-      content::WebContents* web_contents);
-  using content::WebContentsUserData<
-      MediaRouterDialogControllerViews>::FromWebContents;
-
   // MediaRouterDialogController:
   void CreateMediaRouterDialog() override;
   void CloseMediaRouterDialog() override;
@@ -48,6 +46,16 @@ class MediaRouterDialogControllerViews
   // an instance.
   explicit MediaRouterDialogControllerViews(content::WebContents* web_contents);
 
+  // MediaRouterUIService::Observer:
+  void OnServiceDisabled() override;
+
+  // Initializes |ui_|.
+  void InitializeMediaRouterUI();
+
+  // MediaRouterActionController is responsible for showing and hiding the
+  // toolbar action. It's owned by MediaRouterUIService and it may be nullptr.
+  MediaRouterActionController* GetActionController();
+
   MediaRouterViewsUI* ui() { return ui_.get(); }
 
   // Responsible for notifying the dialog view of dialog model updates and
@@ -57,9 +65,10 @@ class MediaRouterDialogControllerViews
 
   base::RepeatingClosure dialog_creation_callback_;
 
-  // This is not null while there is a dialog shown and |this| is observing the
-  // widget.
-  views::Widget* dialog_widget_ = nullptr;
+  ScopedObserver<views::Widget, views::WidgetObserver> scoped_widget_observer_;
+
+  // Service that provides MediaRouterActionController. It outlives |this|.
+  MediaRouterUIService* const media_router_ui_service_;
 
   WEB_CONTENTS_USER_DATA_KEY_DECL();
 

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

@@ -40,8 +40,8 @@ void MediaRouterDialogControllerViewsTest::OpenMediaRouterDialog() {
   // Create a reference to initiator contents.
   initiator_ = browser()->tab_strip_model()->GetActiveWebContents();
 
-  dialog_controller_ =
-      MediaRouterDialogControllerViews::GetOrCreateForWebContents(initiator_);
+  dialog_controller_ = static_cast<MediaRouterDialogControllerViews*>(
+      MediaRouterDialogController::GetOrCreateForWebContents(initiator_));
   ASSERT_TRUE(dialog_controller_);
 
   // Show the media router dialog for the initiator.

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

@@ -8,6 +8,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/media/router/media_router_dialog_controller.h"
 #include "chrome/browser/media/router/media_router_feature.h"
 #include "chrome/browser/prefs/browser_prefs.h"
 #include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h"
@@ -15,7 +16,6 @@
 #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/media_router/media_router_ui_service.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/toolbar/media_router_action_controller.h"
@@ -57,8 +57,8 @@ class MediaRouterUIBrowserTest : public InProcessBrowserTest {
   }
 
   // Returns the dialog controller for the active WebContents.
-  MediaRouterDialogControllerImplBase* GetDialogController() {
-    return MediaRouterDialogControllerImplBase::GetOrCreateForWebContents(
+  MediaRouterDialogController* GetDialogController() {
+    return MediaRouterDialogController::GetOrCreateForWebContents(
         browser()->tab_strip_model()->GetActiveWebContents());
   }
 

chrome/test/media_router/media_router_ui_for_test.cc

@@ -263,9 +263,9 @@ void MediaRouterUiForTest::SetLocalFileSelectionIssue(const IssueInfo& issue) {
 
 MediaRouterUiForTest::MediaRouterUiForTest(content::WebContents* web_contents)
     : web_contents_(web_contents),
-      dialog_controller_(
-          MediaRouterDialogControllerViews::GetOrCreateForWebContents(
-              web_contents)),
+      dialog_controller_(static_cast<MediaRouterDialogControllerViews*>(
+          MediaRouterDialogController::GetOrCreateForWebContents(
+              web_contents))),
       weak_factory_(this) {
   dialog_controller_->SetDialogCreationCallbackForTesting(base::BindRepeating(
       &MediaRouterUiForTest::OnDialogCreated, weak_factory_.GetWeakPtr()));