Add a WebContentsPresentationManager interface

Add WebContentsPresentationManager, which is an interface that can be
observed to obtain the default PresentationRequest and MediaRoutes
associated with a WebContents.

Make PresentationServiceDelegateImpl implement
WebContentsPresentationManager, and have MediaRouterViewsUI observe it
through the interface instead of observing directly.

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

chrome/browser/media/router/BUILD.gn

@@ -62,6 +62,8 @@ static_library("router") {
     "presentation/presentation_service_delegate_observers.h",
     "presentation/receiver_presentation_service_delegate_impl.cc",
     "presentation/receiver_presentation_service_delegate_impl.h",
+    "presentation/web_contents_presentation_manager.cc",
+    "presentation/web_contents_presentation_manager.h",
     "route_message_observer.cc",
     "route_message_observer.h",
     "route_message_util.cc",

chrome/browser/media/router/presentation/presentation_service_delegate_impl.cc

@@ -444,8 +444,7 @@ void PresentationServiceDelegateImpl::SetDefaultPresentationUrls(
   DCHECK(!callback.is_null());
   default_presentation_started_callback_ = std::move(callback);
   default_presentation_request_ = request;
-  for (auto& observer : default_presentation_request_observers_)
-    observer.OnDefaultPresentationChanged(*default_presentation_request_);
+  NotifyDefaultPresentationChanged(&request);
 }
 
 void PresentationServiceDelegateImpl::OnJoinRouteResponse(
@@ -503,6 +502,8 @@ void PresentationServiceDelegateImpl::AddPresentation(
     const MediaRoute& route) {
   auto* presentation_frame = GetOrAddPresentationFrame(render_frame_host_id);
   presentation_frame->AddPresentation(presentation_info, route);
+  // TODO(crbug.com/1031672): Notify WebContentsPresentationManager::Observer
+  // that the presentation routes have changed for the WebContents.
 }
 
 void PresentationServiceDelegateImpl::RemovePresentation(
@@ -511,6 +512,8 @@ void PresentationServiceDelegateImpl::RemovePresentation(
   const auto it = presentation_frames_.find(render_frame_host_id);
   if (it != presentation_frames_.end())
     it->second->RemovePresentation(presentation_id);
+  // TODO(crbug.com/1031672): Notify WebContentsPresentationManager::Observer
+  // that the presentation routes have changed for the WebContents.
 }
 
 void PresentationServiceDelegateImpl::StartPresentation(
@@ -668,7 +671,27 @@ void PresentationServiceDelegateImpl::ListenForConnectionStateChange(
     it->second->ListenForConnectionStateChange(connection, state_changed_cb);
 }
 
-void PresentationServiceDelegateImpl::OnRouteResponse(
+void PresentationServiceDelegateImpl::AddObserver(
+    WebContentsPresentationManager::Observer* observer) {
+  presentation_observers_.AddObserver(observer);
+}
+
+void PresentationServiceDelegateImpl::RemoveObserver(
+    WebContentsPresentationManager::Observer* observer) {
+  presentation_observers_.RemoveObserver(observer);
+}
+
+bool PresentationServiceDelegateImpl::HasDefaultPresentationRequest() const {
+  return default_presentation_request_.has_value();
+}
+
+const content::PresentationRequest&
+PresentationServiceDelegateImpl::GetDefaultPresentationRequest() const {
+  DCHECK(HasDefaultPresentationRequest());
+  return *default_presentation_request_;
+}
+
+void PresentationServiceDelegateImpl::OnPresentationResponse(
     const content::PresentationRequest& presentation_request,
     mojom::RoutePresentationConnectionPtr connection,
     const RouteRequestResult& result) {
@@ -695,26 +718,6 @@ void PresentationServiceDelegateImpl::OnRouteResponse(
   }
 }
 
-void PresentationServiceDelegateImpl::AddDefaultPresentationRequestObserver(
-    DefaultPresentationRequestObserver* observer) {
-  default_presentation_request_observers_.AddObserver(observer);
-}
-
-void PresentationServiceDelegateImpl::RemoveDefaultPresentationRequestObserver(
-    DefaultPresentationRequestObserver* observer) {
-  default_presentation_request_observers_.RemoveObserver(observer);
-}
-
-const content::PresentationRequest&
-PresentationServiceDelegateImpl::GetDefaultPresentationRequest() const {
-  DCHECK(HasDefaultPresentationRequest());
-  return *default_presentation_request_;
-}
-
-bool PresentationServiceDelegateImpl::HasDefaultPresentationRequest() const {
-  return !!default_presentation_request_;
-}
-
 base::WeakPtr<PresentationServiceDelegateImpl>
 PresentationServiceDelegateImpl::GetWeakPtr() {
   return weak_factory_.GetWeakPtr();
@@ -737,8 +740,7 @@ void PresentationServiceDelegateImpl::ClearDefaultPresentationRequest() {
     return;
 
   default_presentation_request_.reset();
-  for (auto& observer : default_presentation_request_observers_)
-    observer.OnDefaultPresentationRemoved();
+  NotifyDefaultPresentationChanged(nullptr);
 }
 
 std::unique_ptr<media::FlingingController>
@@ -798,6 +800,14 @@ void PresentationServiceDelegateImpl::EnsurePresentationConnection(
   }
 }
 
+void PresentationServiceDelegateImpl::NotifyDefaultPresentationChanged(
+    const content::PresentationRequest* request) {
+  for (WebContentsPresentationManager::Observer& presentation_observer :
+       presentation_observers_) {
+    presentation_observer.OnDefaultPresentationChanged(request);
+  }
+}
+
 WEB_CONTENTS_USER_DATA_KEY_IMPL(PresentationServiceDelegateImpl)
 
 }  // namespace media_router

chrome/browser/media/router/presentation/presentation_service_delegate_impl.h

@@ -19,6 +19,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/media/router/media_router.h"
 #include "chrome/browser/media/router/presentation/presentation_service_delegate_observers.h"
+#include "chrome/browser/media/router/presentation/web_contents_presentation_manager.h"
 #include "chrome/common/media_router/media_source.h"
 #include "chrome/common/media_router/mojom/media_router.mojom.h"
 #include "content/public/browser/presentation_request.h"
@@ -87,27 +88,14 @@ class StartPresentationContext {
 // it also provides default presentation URL that is required for creating
 // browser-initiated presentations.  It is scoped to the lifetime of a
 // WebContents, and is managed by the associated WebContents.
+// It is accessed through the WebContentsPresentationManager interface by
+// clients (e.g. the UI code) that is interested in the presentation status of
+// the WebContents, but not in other aspects such as the render frame.
 class PresentationServiceDelegateImpl
     : public content::WebContentsUserData<PresentationServiceDelegateImpl>,
-      public content::ControllerPresentationServiceDelegate {
+      public content::ControllerPresentationServiceDelegate,
+      public WebContentsPresentationManager {
  public:
-  // Observer interface for listening to default presentation request
-  // changes for the WebContents.
-  class DefaultPresentationRequestObserver {
-   public:
-    virtual ~DefaultPresentationRequestObserver() = default;
-
-    // Called when default presentation request for the corresponding
-    // WebContents is set or changed.
-    // |default_presentation_info|: New default presentation request.
-    virtual void OnDefaultPresentationChanged(
-        const content::PresentationRequest& default_presentation_request) = 0;
-
-    // Called when default presentation request for the corresponding
-    // WebContents has been removed.
-    virtual void OnDefaultPresentationRemoved() = 0;
-  };
-
   // Retrieves the instance of PresentationServiceDelegateImpl that was attached
   // to the specified WebContents.  If no instance was attached, creates one,
   // and attaches it to the specified WebContents.
@@ -160,34 +148,22 @@ class PresentationServiceDelegateImpl
       const content::PresentationConnectionStateChangedCallback&
           state_changed_cb) override;
 
-  // Callback invoked when a default PresentationRequest is started from a
-  // browser-initiated dialog.
-  void OnRouteResponse(const content::PresentationRequest& request,
-                       mojom::RoutePresentationConnectionPtr connection,
-                       const RouteRequestResult& result);
-
-  // Adds / removes an observer for listening to default PresentationRequest
-  // changes. This class does not own |observer|. When |observer| is about to
-  // be destroyed, |RemoveDefaultPresentationRequestObserver| must be called.
-  void AddDefaultPresentationRequestObserver(
-      DefaultPresentationRequestObserver* observer);
-  void RemoveDefaultPresentationRequestObserver(
-      DefaultPresentationRequestObserver* observer);
-
-  // Gets the default presentation request for the owning WebContents. It
-  // is an error to call this method if the default presentation request does
-  // not exist.
-  const content::PresentationRequest& GetDefaultPresentationRequest() const;
-
-  // Returns true if there is a default presentation request for the owning tab
-  // WebContents.
-  bool HasDefaultPresentationRequest() const;
+  // WebContentsPresentationManager implementation.
+  void AddObserver(WebContentsPresentationManager::Observer* observer) override;
+  void RemoveObserver(
+      WebContentsPresentationManager::Observer* observer) override;
+  bool HasDefaultPresentationRequest() const override;
+  const content::PresentationRequest& GetDefaultPresentationRequest()
+      const override;
+  void OnPresentationResponse(const content::PresentationRequest& request,
+                              mojom::RoutePresentationConnectionPtr connection,
+                              const RouteRequestResult& result) override;
+
+  base::WeakPtr<PresentationServiceDelegateImpl> GetWeakPtr();
 
   // Returns the WebContents that owns this instance.
   content::WebContents* web_contents() const { return web_contents_; }
 
-  base::WeakPtr<PresentationServiceDelegateImpl> GetWeakPtr();
-
   bool HasScreenAvailabilityListenerForTest(
       int render_process_id,
       int render_frame_id,
@@ -276,6 +252,9 @@ class PresentationServiceDelegateImpl
       const blink::mojom::PresentationInfo& presentation_info,
       mojom::RoutePresentationConnectionPtr* connection);
 
+  void NotifyDefaultPresentationChanged(
+      const content::PresentationRequest* request);
+
   // References to the WebContents that owns this instance, and associated
   // browser profile's MediaRouter instance.
   content::WebContents* const web_contents_;
@@ -283,8 +262,8 @@ class PresentationServiceDelegateImpl
 
   // References to the observers listening for changes to the default
   // presentation of the associated WebContents.
-  base::ObserverList<DefaultPresentationRequestObserver>::Unchecked
-      default_presentation_request_observers_;
+  base::ObserverList<WebContentsPresentationManager::Observer>
+      presentation_observers_;
 
   // Default presentation request for the owning WebContents.
   base::Optional<content::PresentationRequest> default_presentation_request_;

chrome/browser/media/router/presentation/presentation_service_delegate_impl_unittest.cc

@@ -10,6 +10,7 @@
 #include "chrome/browser/media/router/media_router_factory.h"
 #include "chrome/browser/media/router/presentation/local_presentation_manager.h"
 #include "chrome/browser/media/router/presentation/local_presentation_manager_factory.h"
+#include "chrome/browser/media/router/presentation/web_contents_presentation_manager.h"
 #include "chrome/browser/media/router/test/mock_media_router.h"
 #include "chrome/browser/media/router/test/mock_screen_availability_listener.h"
 #include "chrome/browser/media/router/test/test_helper.h"
@@ -67,13 +68,27 @@ class MockDelegateObserver
   MOCK_METHOD1(OnDefaultPresentationStarted, void(const PresentationInfo&));
 };
 
-class MockDefaultPresentationRequestObserver
-    : public PresentationServiceDelegateImpl::
-          DefaultPresentationRequestObserver {
+class MockWebContentsPresentationObserver
+    : public WebContentsPresentationManager::Observer {
  public:
+  explicit MockWebContentsPresentationObserver(
+      content::WebContents* web_contents) {
+    presentation_manager_ = WebContentsPresentationManager::Get(web_contents);
+    presentation_manager_->AddObserver(this);
+  }
+
+  ~MockWebContentsPresentationObserver() override {
+    if (presentation_manager_)
+      presentation_manager_->RemoveObserver(this);
+  }
+
+  MOCK_METHOD1(OnMediaRoutesChanged,
+               void(const std::vector<MediaRoute>& routes));
   MOCK_METHOD1(OnDefaultPresentationChanged,
-               void(const content::PresentationRequest&));
-  MOCK_METHOD0(OnDefaultPresentationRemoved, void());
+               void(const content::PresentationRequest* presentation_request));
+
+ private:
+  base::WeakPtr<WebContentsPresentationManager> presentation_manager_;
 };
 
 class MockCreatePresentationConnnectionCallbacks {
@@ -178,8 +193,8 @@ class PresentationServiceDelegateImplTest
     // Should not trigger callback since route response is error.
     std::unique_ptr<RouteRequestResult> result = RouteRequestResult::FromError(
         "Error", RouteRequestResult::UNKNOWN_ERROR);
-    delegate_impl_->OnRouteResponse(request, /** connection */ nullptr,
-                                    *result);
+    delegate_impl_->OnPresentationResponse(request, /** connection */ nullptr,
+                                           *result);
     EXPECT_TRUE(Mock::VerifyAndClearExpectations(this));
 
     // Should not trigger callback since request doesn't match.
@@ -191,8 +206,8 @@ class PresentationServiceDelegateImplTest
     media_route.set_incognito(incognito);
     result =
         RouteRequestResult::FromSuccess(media_route, "differentPresentationId");
-    delegate_impl_->OnRouteResponse(different_request,
-                                    /** connection */ nullptr, *result);
+    delegate_impl_->OnPresentationResponse(different_request,
+                                           /** connection */ nullptr, *result);
     EXPECT_TRUE(Mock::VerifyAndClearExpectations(this));
 
     // Should trigger callback since request matches.
@@ -200,8 +215,8 @@ class PresentationServiceDelegateImplTest
     MediaRoute media_route2("routeId", source1_, "mediaSinkId", "", true, true);
     media_route2.set_incognito(incognito);
     result = RouteRequestResult::FromSuccess(media_route2, "presentationId");
-    delegate_impl_->OnRouteResponse(request, /** connection */ nullptr,
-                                    *result);
+    delegate_impl_->OnPresentationResponse(request, /** connection */ nullptr,
+                                           *result);
   }
 
   void SetMainFrame() {
@@ -379,13 +394,12 @@ TEST_F(PresentationServiceDelegateImplIncognitoTest,
 }
 
 TEST_F(PresentationServiceDelegateImplTest,
-       DefaultPresentationRequestObserver) {
+       NotifyWebContentsPresentationObservers) {
   auto callback = base::BindRepeating(
       &PresentationServiceDelegateImplTest::OnDefaultPresentationStarted,
       base::Unretained(this));
 
-  StrictMock<MockDefaultPresentationRequestObserver> observer;
-  delegate_impl_->AddDefaultPresentationRequestObserver(&observer);
+  StrictMock<MockWebContentsPresentationObserver> observer(GetWebContents());
 
   content::WebContentsTester::For(GetWebContents())
       ->NavigateAndCommit(frame_url_);
@@ -409,7 +423,7 @@ TEST_F(PresentationServiceDelegateImplTest,
   EXPECT_TRUE(Mock::VerifyAndClearExpectations(&observer));
 
   // Remove default presentation URL.
-  EXPECT_CALL(observer, OnDefaultPresentationRemoved()).Times(1);
+  EXPECT_CALL(observer, OnDefaultPresentationChanged(nullptr)).Times(1);
   content::PresentationRequest empty_request(
       {main_frame_process_id_, main_frame_routing_id_}, std::vector<GURL>(),
       frame_origin_);

chrome/browser/media/router/presentation/web_contents_presentation_manager.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 "chrome/browser/media/router/presentation/web_contents_presentation_manager.h"
+
+#include "chrome/browser/media/router/presentation/presentation_service_delegate_impl.h"
+
+namespace media_router {
+
+// static
+base::WeakPtr<WebContentsPresentationManager>
+WebContentsPresentationManager::Get(content::WebContents* web_contents) {
+  return PresentationServiceDelegateImpl::GetOrCreateForWebContents(
+             web_contents)
+      ->GetWeakPtr();
+}
+
+WebContentsPresentationManager::~WebContentsPresentationManager() = default;
+
+}  // namespace media_router

chrome/browser/media/router/presentation/web_contents_presentation_manager.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_MEDIA_ROUTER_PRESENTATION_WEB_CONTENTS_PRESENTATION_MANAGER_H_
+#define CHROME_BROWSER_MEDIA_ROUTER_PRESENTATION_WEB_CONTENTS_PRESENTATION_MANAGER_H_
+
+#include <vector>
+
+#include "base/containers/flat_map.h"
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
+#include "chrome/common/media_router/mojom/media_router.mojom.h"
+
+namespace content {
+struct PresentationRequest;
+class WebContents;
+}  // namespace content
+
+namespace media_router {
+
+class MediaRoute;
+class RouteRequestResult;
+
+// Keeps track of the default PresentationRequest and presentation MediaRoutes
+// associated with a WebContents. Its lifetime is tied to that of the
+// WebContents.
+class WebContentsPresentationManager {
+ public:
+  class Observer : public base::CheckedObserver {
+   public:
+    // Called whenever presentation MediaRoutes associated with the WebContents
+    // are added, removed, or have their attributes changed.
+    virtual void OnMediaRoutesChanged(const std::vector<MediaRoute>& routes) {}
+
+    // |presentation_request| is a nullptr if the default PresentationRequest
+    // has been removed.
+    virtual void OnDefaultPresentationChanged(
+        const content::PresentationRequest* presentation_request) {}
+
+   protected:
+    Observer() = default;
+  };
+
+  static base::WeakPtr<WebContentsPresentationManager> Get(
+      content::WebContents* web_contents);
+
+  virtual ~WebContentsPresentationManager() = 0;
+
+  virtual void AddObserver(Observer* observer) = 0;
+  virtual void RemoveObserver(Observer* observer) = 0;
+
+  // Returns true if there is a default presentation request for the
+  // WebContents.
+  virtual bool HasDefaultPresentationRequest() const = 0;
+
+  // Gets the default presentation request for the WebContents. It is an error
+  // to call this method if the default presentation request does not exist.
+  virtual const content::PresentationRequest& GetDefaultPresentationRequest()
+      const = 0;
+
+  // Invoked by Media Router when a PresentationRequest is started from a
+  // browser-initiated dialog. Updates the internal state and propagates the
+  // request result to the presentation controller.
+  virtual void OnPresentationResponse(
+      const content::PresentationRequest& presentation_request,
+      mojom::RoutePresentationConnectionPtr connection,
+      const RouteRequestResult& result) = 0;
+
+ protected:
+  WebContentsPresentationManager() = default;
+};
+
+}  // namespace media_router
+
+#endif  // CHROME_BROWSER_MEDIA_ROUTER_PRESENTATION_WEB_CONTENTS_PRESENTATION_MANAGER_H_

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

@@ -121,14 +121,12 @@ void MediaRouterDialogControllerViews::OnServiceDisabled() {
 }
 
 void MediaRouterDialogControllerViews::InitializeMediaRouterUI() {
-  ui_ = std::make_unique<MediaRouterViewsUI>();
-  PresentationServiceDelegateImpl* delegate =
-      PresentationServiceDelegateImpl::FromWebContents(initiator());
-  if (!start_presentation_context_) {
-    ui_->InitWithDefaultMediaSource(initiator(), delegate);
-  } else {
+  ui_ = std::make_unique<MediaRouterViewsUI>(initiator());
+  if (start_presentation_context_) {
     ui_->InitWithStartPresentationContext(
-        initiator(), delegate, std::move(start_presentation_context_));
+        std::move(start_presentation_context_));
+  } else {
+    ui_->InitWithDefaultMediaSource();
   }
 }
 

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

@@ -26,7 +26,6 @@
 #include "chrome/browser/media/router/media_router_factory.h"
 #include "chrome/browser/media/router/media_router_metrics.h"
 #include "chrome/browser/media/router/media_routes_observer.h"
-#include "chrome/browser/media/router/presentation/presentation_service_delegate_impl.h"
 #include "chrome/browser/media/router/providers/wired_display/wired_display_media_route_provider.h"
 #include "chrome/browser/media/webrtc/desktop_media_picker_controller.h"
 #include "chrome/browser/profiles/profile.h"
@@ -198,7 +197,13 @@ class MediaRouterViewsUI::WebContentsFullscreenOnLoadedObserver final
   base::OneShotTimer capture_poll_timer_;
 };
 
-MediaRouterViewsUI::MediaRouterViewsUI() = default;
+MediaRouterViewsUI::MediaRouterViewsUI(content::WebContents* initiator)
+    : presentation_manager_(WebContentsPresentationManager::Get(initiator)),
+      initiator_(initiator) {
+  CHECK(initiator_);
+  if (presentation_manager_)
+    presentation_manager_->AddObserver(this);
+}
 
 MediaRouterViewsUI::~MediaRouterViewsUI() {
   for (CastDialogController::Observer& observer : observers_)
@@ -206,9 +211,9 @@ MediaRouterViewsUI::~MediaRouterViewsUI() {
 
   if (query_result_manager_.get())
     query_result_manager_->RemoveObserver(this);
-  if (presentation_service_delegate_.get())
-    presentation_service_delegate_->RemoveDefaultPresentationRequestObserver(
-        this);
+  if (presentation_manager_)
+    presentation_manager_->RemoveObserver(this);
+
   // If |start_presentation_context_| still exists, then it means presentation
   // route request was never attempted.
   if (start_presentation_context_) {
@@ -264,21 +269,14 @@ void MediaRouterViewsUI::ClearIssue(const Issue::Id& issue_id) {
   RemoveIssue(issue_id);
 }
 
-void MediaRouterViewsUI::InitWithDefaultMediaSource(
-    content::WebContents* initiator,
-    PresentationServiceDelegateImpl* delegate) {
-  DCHECK(initiator);
-  DCHECK(!presentation_service_delegate_);
+void MediaRouterViewsUI::InitWithDefaultMediaSource() {
   DCHECK(!query_result_manager_);
+  InitCommon();
 
-  InitCommon(initiator);
-  if (delegate) {
-    presentation_service_delegate_ = delegate->GetWeakPtr();
-    presentation_service_delegate_->AddDefaultPresentationRequestObserver(this);
-  }
-
-  if (delegate && delegate->HasDefaultPresentationRequest()) {
-    OnDefaultPresentationChanged(delegate->GetDefaultPresentationRequest());
+  if (presentation_manager_ &&
+      presentation_manager_->HasDefaultPresentationRequest()) {
+    OnDefaultPresentationChanged(
+        &presentation_manager_->GetDefaultPresentationRequest());
   } else {
     // Register for MediaRoute updates without a media source.
     routes_observer_ = std::make_unique<UIMediaRoutesObserver>(
@@ -289,21 +287,16 @@ void MediaRouterViewsUI::InitWithDefaultMediaSource(
 }
 
 void MediaRouterViewsUI::InitWithStartPresentationContext(
-    content::WebContents* initiator,
-    PresentationServiceDelegateImpl* delegate,
     std::unique_ptr<StartPresentationContext> context) {
-  DCHECK(initiator);
-  DCHECK(delegate);
   DCHECK(context);
   DCHECK(!start_presentation_context_);
   DCHECK(!query_result_manager_);
 
   start_presentation_context_ = std::move(context);
-  presentation_service_delegate_ = delegate->GetWeakPtr();
 
-  InitCommon(initiator);
+  InitCommon();
   OnDefaultPresentationChanged(
-      start_presentation_context_->presentation_request());
+      &start_presentation_context_->presentation_request());
 }
 
 bool MediaRouterViewsUI::CreateRoute(const MediaSink::Id& sink_id,
@@ -379,7 +372,7 @@ std::vector<MediaSinkWithCastModes> MediaRouterViewsUI::GetEnabledSinks()
   // the best place to do this, but the Media Router browser service and
   // extension process are shared between normal and incognito, so incognito
   // behaviors around sink availability have to be handled at the UI layer.
-  if (initiator()->GetBrowserContext()->IsOffTheRecord()) {
+  if (initiator_->GetBrowserContext()->IsOffTheRecord()) {
     base::EraseIf(enabled_sinks, [](const MediaSinkWithCastModes& sink) {
       return sink.sink.IsMaybeCloudSink();
     });
@@ -390,7 +383,6 @@ std::vector<MediaSinkWithCastModes> MediaRouterViewsUI::GetEnabledSinks()
 
 base::string16 MediaRouterViewsUI::GetPresentationRequestSourceName() const {
   GURL gurl = GetFrameURL();
-  CHECK(initiator_);
   // Presentation URLs are only possible on https: and other secure contexts,
   // so we can omit http/https schemes here.
   return gurl.SchemeIs(extensions::kExtensionScheme)
@@ -482,10 +474,7 @@ void MediaRouterViewsUI::HandleCreateSessionRequestRouteResponse(
   // TODO(crbug.com/868186): Close the dialog.
 }
 
-void MediaRouterViewsUI::InitCommon(content::WebContents* initiator) {
-  DCHECK(initiator);
-  initiator_ = initiator;
-
+void MediaRouterViewsUI::InitCommon() {
   GetMediaRouter()->OnUserGesture();
 
   // Create |collator_| before |query_result_manager_| so that |collator_| is
@@ -514,7 +503,7 @@ void MediaRouterViewsUI::InitCommon(content::WebContents* initiator) {
   query_result_manager_->SetSourcesForCastMode(
       MediaCastMode::LOCAL_FILE, {MediaSource::ForTab(0)}, origin);
 
-  SessionID::id_type tab_id = SessionTabHelper::IdForTab(initiator).id();
+  SessionID::id_type tab_id = SessionTabHelper::IdForTab(initiator_).id();
   if (tab_id != -1) {
     MediaSource mirroring_source(MediaSource::ForTab(tab_id));
     query_result_manager_->SetSourcesForCastMode(MediaCastMode::TAB_MIRROR,
@@ -533,12 +522,17 @@ void MediaRouterViewsUI::InitCommon(content::WebContents* initiator) {
 }
 
 void MediaRouterViewsUI::OnDefaultPresentationChanged(
-    const content::PresentationRequest& presentation_request) {
+    const content::PresentationRequest* presentation_request) {
+  if (!presentation_request) {
+    OnDefaultPresentationRemoved();
+    return;
+  }
+
   std::vector<MediaSource> sources;
-  for (const auto& url : presentation_request.presentation_urls) {
+  for (const auto& url : presentation_request->presentation_urls) {
     sources.push_back(MediaSource::ForPresentationUrl(url));
   }
-  presentation_request_ = presentation_request;
+  presentation_request_ = *presentation_request;
   query_result_manager_->SetSourcesForCastMode(
       MediaCastMode::PRESENTATION, sources,
       presentation_request_->frame_origin);
@@ -589,8 +583,6 @@ base::Optional<RouteParameters> MediaRouterViewsUI::GetRouteParameters(
     const MediaSink::Id& sink_id,
     MediaCastMode cast_mode) {
   DCHECK(query_result_manager_);
-  DCHECK(initiator_);
-
   RouteParameters params;
 
   // Note that there is a rarely-encountered bug, where the MediaCastMode to
@@ -636,7 +628,7 @@ base::Optional<RouteParameters> MediaRouterViewsUI::GetRouteParameters(
   //     The StartPresentationContext will need to be answered with the route
   //     response.
   // (3) Browser-initiated presentation route request. If successful,
-  //     PresentationServiceDelegateImpl will have to be notified. Note that we
+  //     WebContentsPresentationManager will have to be notified. Note that we
   //     treat subsequent route requests from a Presentation API-initiated
   //     dialogs as browser-initiated.
   // TODO(https://crbug.com/868186): Close the Views dialog in case (2).
@@ -654,15 +646,14 @@ base::Optional<RouteParameters> MediaRouterViewsUI::GetRouteParameters(
       params.route_result_callbacks.push_back(base::BindOnce(
           &MediaRouterViewsUI::HandleCreateSessionRequestRouteResponse,
           weak_factory_.GetWeakPtr()));
-    } else if (presentation_service_delegate_) {
+    } else if (presentation_manager_) {
       params.presentation_callback = base::BindOnce(
-          &PresentationServiceDelegateImpl::OnRouteResponse,
-          presentation_service_delegate_, *presentation_request_);
+          &WebContentsPresentationManager::OnPresentationResponse,
+          presentation_manager_, *presentation_request_);
     }
   }
 
   params.timeout = GetRouteRequestTimeout(cast_mode);
-  CHECK(initiator_);
   params.incognito = initiator_->GetBrowserContext()->IsOffTheRecord();
 
   return base::make_optional(std::move(params));
@@ -948,13 +939,11 @@ content::WebContents* MediaRouterViewsUI::OpenTabWithUrl(const GURL& url) {
 }
 
 MediaRouter* MediaRouterViewsUI::GetMediaRouter() const {
-  CHECK(initiator_);
   return MediaRouterFactory::GetApiForBrowserContext(
       initiator_->GetBrowserContext());
 }
 
 Browser* MediaRouterViewsUI::GetBrowser() {
-  CHECK(initiator_);
   return chrome::FindBrowserWithWebContents(initiator_);
 }
 

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

@@ -19,7 +19,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/media/router/issues_observer.h"
 #include "chrome/browser/media/router/media_router_dialog_controller.h"
-#include "chrome/browser/media/router/presentation/presentation_service_delegate_impl.h"
+#include "chrome/browser/media/router/presentation/web_contents_presentation_manager.h"
 #include "chrome/browser/ui/media_router/cast_dialog_controller.h"
 #include "chrome/browser/ui/media_router/cast_dialog_model.h"
 #include "chrome/browser/ui/media_router/media_cast_mode.h"
@@ -53,11 +53,10 @@ class RouteRequestResult;
 class MediaRouterViewsUI
     : public CastDialogController,
       public QueryResultManager::Observer,
-      public PresentationServiceDelegateImpl::
-          DefaultPresentationRequestObserver,
+      public WebContentsPresentationManager::Observer,
       public MediaRouterFileDialog::MediaRouterFileDialogDelegate {
  public:
-  MediaRouterViewsUI();
+  explicit MediaRouterViewsUI(content::WebContents* initiator);
   ~MediaRouterViewsUI() override;
 
   // CastDialogController:
@@ -71,36 +70,22 @@ class MediaRouterViewsUI
   void ClearIssue(const Issue::Id& issue_id) override;
 
   // Initializes internal state (e.g. starts listening for MediaSinks) for
-  // targeting the default MediaSource (if any) of the initiator tab that owns
-  // |delegate|, as well as mirroring sources of that tab.
-  // The contents of the UI will change as the default MediaSource changes.
-  // If there is a default MediaSource, then PRESENTATION MediaCastMode will be
-  // added to |cast_modes_|.
-  // Init* methods can only be called once.
-  // |initiator|: Reference to the WebContents that initiated the dialog.
-  //              Must not be null.
-  // |delegate|: PresentationServiceDelegateImpl of the initiator tab.
-  //             Must not be null.
-  // TODO(imcheng): Replace use of impl with an intermediate abstract
-  // interface.
-  void InitWithDefaultMediaSource(content::WebContents* initiator,
-                                  PresentationServiceDelegateImpl* delegate);
+  // targeting the default MediaSource (if any) of |initiator_|, as well as
+  // mirroring sources of that tab. The contents of the UI will change as the
+  // default MediaSource changes. If there is a default MediaSource, then
+  // PRESENTATION MediaCastMode will be added to |cast_modes_|. Init* methods
+  // can only be called once.
+  void InitWithDefaultMediaSource();
 
   // Initializes internal state targeting the presentation specified in
-  // |context|. Also sets up mirroring sources based on |initiator|.
+  // |context|. Also sets up mirroring sources based on |initiator_|.
   // This is different from InitWithDefaultMediaSource() in that it does not
   // listen for default media source changes, as the UI is fixed to the source
-  // in |request|.
+  // in |context|.
   // Init* methods can only be called once.
-  // |initiator|: Reference to the WebContents that initiated the dialog.
-  //              Must not be null.
-  // |delegate|: PresentationServiceDelegateImpl of the initiator tab.
-  //             Must not be null.
   // |context|: Context object for the PresentationRequest. This instance will
   //            take ownership of it. Must not be null.
   void InitWithStartPresentationContext(
-      content::WebContents* initiator,
-      PresentationServiceDelegateImpl* delegate,
       std::unique_ptr<StartPresentationContext> context);
 
   // Requests a route be created from the source mapped to
@@ -225,13 +210,14 @@ class MediaRouterViewsUI
   virtual void HandleCreateSessionRequestRouteResponse(
       const RouteRequestResult&);
 
-  // Initializes the dialog with mirroring sources derived from |initiator|.
-  virtual void InitCommon(content::WebContents* initiator);
+  // Initializes the dialog with mirroring sources derived from |initiator_|.
+  virtual void InitCommon();
 
-  // PresentationServiceDelegateImpl::DefaultPresentationObserver
+  // WebContentsPresentationManager::Observer
   void OnDefaultPresentationChanged(
-      const content::PresentationRequest& presentation_request) override;
-  void OnDefaultPresentationRemoved() override;
+      const content::PresentationRequest* presentation_request) override;
+
+  void OnDefaultPresentationRemoved();
 
   // Called to update the dialog with the current list of of enabled sinks.
   void UpdateSinks();
@@ -392,17 +378,12 @@ class MediaRouterViewsUI
   // mode, if supported. Otherwise set to nullopt.
   base::Optional<content::PresentationRequest> presentation_request_;
 
-  // It's possible for PresentationServiceDelegateImpl to be destroyed before
-  // this class.
-  // (e.g. if a tab with the UI open is closed, then the tab WebContents will
-  // be destroyed first momentarily before the UI WebContents).
-  // Holding a WeakPtr to PresentationServiceDelegateImpl is the cleanest way to
-  // handle this.
-  // TODO(imcheng): hold a weak ptr to an abstract type instead.
-  base::WeakPtr<PresentationServiceDelegateImpl> presentation_service_delegate_;
+  // |presentation_manager_| notifies |this| whenever there is an update to the
+  // default PresentationRequest or MediaRoutes associated with |initiator_|.
+  base::WeakPtr<WebContentsPresentationManager> presentation_manager_;
 
   // WebContents for the tab for which the Cast dialog is shown.
-  content::WebContents* initiator_ = nullptr;
+  content::WebContents* const initiator_;
 
   // The dialog that handles opening the file dialog and validating and
   // returning the results.

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

@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/media/router/media_router_factory.h"
 #include "chrome/browser/media/router/media_sinks_observer.h"
 #include "chrome/browser/media/router/providers/wired_display/wired_display_media_route_provider.h"
 #include "chrome/browser/media/router/test/mock_media_router.h"
@@ -107,19 +108,6 @@ class PresentationRequestCallbacks {
   blink::mojom::PresentationError expected_error_;
 };
 
-// Injects a MediaRouter instance into MediaRouterViewsUI.
-class TestMediaRouterViewsUI : public MediaRouterViewsUI {
- public:
-  explicit TestMediaRouterViewsUI(MediaRouter* router) : router_(router) {}
-  ~TestMediaRouterViewsUI() override = default;
-
-  MediaRouter* GetMediaRouter() const override { return router_; }
-
- private:
-  MediaRouter* router_;
-  DISALLOW_COPY_AND_ASSIGN(TestMediaRouterViewsUI);
-};
-
 class TestWebContentsDisplayObserver : public WebContentsDisplayObserver {
  public:
   explicit TestWebContentsDisplayObserver(const display::Display& display)
@@ -143,16 +131,20 @@ class MediaRouterViewsUITest : public ChromeRenderViewHostTestHarness {
   void SetUp() override {
     ChromeRenderViewHostTestHarness::SetUp();
 
+    SetMediaRouterFactory();
+    mock_router_ = static_cast<MockMediaRouter*>(
+        MediaRouterFactory::GetApiForBrowserContext(GetBrowserContext()));
+
     // Store sink observers so that they can be notified in tests.
-    ON_CALL(mock_router_, RegisterMediaSinksObserver(_))
-        .WillByDefault(Invoke([this](MediaSinksObserver* observer) {
+    ON_CALL(*mock_router_, RegisterMediaSinksObserver(_))
+        .WillByDefault([this](MediaSinksObserver* observer) {
           media_sinks_observers_.push_back(observer);
           return true;
-        }));
+        });
 
     SessionTabHelper::CreateForWebContents(web_contents());
-    ui_ = std::make_unique<TestMediaRouterViewsUI>(&mock_router_);
-    ui_->InitWithDefaultMediaSource(web_contents(), nullptr);
+    ui_ = std::make_unique<MediaRouterViewsUI>(web_contents());
+    ui_->InitWithDefaultMediaSource();
   }
 
   void TearDown() override {
@@ -160,14 +152,19 @@ class MediaRouterViewsUITest : public ChromeRenderViewHostTestHarness {
     ChromeRenderViewHostTestHarness::TearDown();
   }
 
+  virtual void SetMediaRouterFactory() {
+    MediaRouterFactory::GetInstance()->SetTestingFactory(
+        GetBrowserContext(), base::BindRepeating(&MockMediaRouter::Create));
+  }
+
   void CreateMediaRouterUIForURL(const GURL& url) {
     web_contents()->GetController().LoadURL(url, content::Referrer(),
                                             ui::PAGE_TRANSITION_LINK, "");
     content::RenderFrameHostTester::CommitPendingLoad(
         &web_contents()->GetController());
     SessionTabHelper::CreateForWebContents(web_contents());
-    ui_ = std::make_unique<TestMediaRouterViewsUI>(&mock_router_);
-    ui_->InitWithDefaultMediaSource(web_contents(), nullptr);
+    ui_ = std::make_unique<MediaRouterViewsUI>(web_contents());
+    ui_->InitWithDefaultMediaSource();
   }
 
   // These methods are used so that we don't have to friend each test case that
@@ -186,7 +183,7 @@ class MediaRouterViewsUITest : public ChromeRenderViewHostTestHarness {
     MediaSource media_source =
         MediaSource::ForTab(SessionTabHelper::IdForTab(web_contents()).id());
     EXPECT_CALL(
-        mock_router_,
+        *mock_router_,
         CreateRouteInternal(media_source.id(), kSinkId, _, web_contents(), _,
                             base::TimeDelta::FromSeconds(60), is_incognito));
     MediaSink sink(kSinkId, kSinkName, SinkIconType::GENERIC);
@@ -202,7 +199,7 @@ class MediaRouterViewsUITest : public ChromeRenderViewHostTestHarness {
     MediaSink sink(kSinkId, kSinkName, SinkIconType::CAST);
     ui_->OnResultsUpdated({{sink, {cast_mode}}});
     MediaRouteResponseCallback callback;
-    EXPECT_CALL(mock_router_,
+    EXPECT_CALL(*mock_router_,
                 CreateRouteInternal(
                     _, _, _, _, _,
                     base::TimeDelta::FromSeconds(timeout_seconds), false))
@@ -210,7 +207,7 @@ class MediaRouterViewsUITest : public ChromeRenderViewHostTestHarness {
     for (MediaSinksObserver* sinks_observer : media_sinks_observers_)
       sinks_observer->OnSinksUpdated({sink}, std::vector<url::Origin>());
     ui_->StartCasting(kSinkId, cast_mode);
-    Mock::VerifyAndClearExpectations(&mock_router_);
+    Mock::VerifyAndClearExpectations(mock_router_);
 
     EXPECT_CALL(observer, OnModelUpdated(_))
         .WillOnce(WithArg<0>([&](const CastDialogModel& model) {
@@ -238,13 +235,13 @@ class MediaRouterViewsUITest : public ChromeRenderViewHostTestHarness {
                    base::Unretained(request_callbacks.get())));
     StartPresentationContext* context_ptr = context.get();
     ui_->set_start_presentation_context_for_test(std::move(context));
-    ui_->OnDefaultPresentationChanged(context_ptr->presentation_request());
+    ui_->OnDefaultPresentationChanged(&context_ptr->presentation_request());
     return request_callbacks;
   }
 
  protected:
   std::vector<MediaSinksObserver*> media_sinks_observers_;
-  MockMediaRouter mock_router_;
+  MockMediaRouter* mock_router_ = nullptr;
   std::unique_ptr<MediaRouterViewsUI> ui_;
   std::unique_ptr<StartPresentationContext> start_presentation_context_;
   content::PresentationRequest presentation_request_{
@@ -326,8 +323,9 @@ TEST_F(MediaRouterViewsUITest, SetDialogHeader) {
 
   GURL gurl("https://example.com");
   url::Origin origin = url::Origin::Create(gurl);
-  ui_->OnDefaultPresentationChanged(content::PresentationRequest(
-      content::GlobalFrameRoutingId(), {gurl}, origin));
+  content::PresentationRequest presentation_request(
+      content::GlobalFrameRoutingId(), {gurl}, origin);
+  ui_->OnDefaultPresentationChanged(&presentation_request);
 
   // Now that the presentation request has been set, the dialog header contains
   // its origin.
@@ -347,7 +345,7 @@ TEST_F(MediaRouterViewsUITest, StartCasting) {
 }
 
 TEST_F(MediaRouterViewsUITest, StopCasting) {
-  EXPECT_CALL(mock_router_, TerminateRoute(kRouteId));
+  EXPECT_CALL(*mock_router_, TerminateRoute(kRouteId));
   ui_->StopCasting(kRouteId);
 }
 
@@ -430,7 +428,7 @@ TEST_F(MediaRouterViewsUITest, AddAndRemoveIssue) {
                             {sink2, {MediaCastMode::TAB_MIRROR}}});
 
   MockControllerObserver observer(ui_.get());
-  MockIssuesObserver issues_observer(mock_router_.GetIssueManager());
+  MockIssuesObserver issues_observer(mock_router_->GetIssueManager());
   issues_observer.Init();
   const std::string issue_title("Issue 1");
   IssueInfo issue(issue_title, IssueInfo::Action::DISMISS,
@@ -450,7 +448,7 @@ TEST_F(MediaRouterViewsUITest, AddAndRemoveIssue) {
             EXPECT_EQ(model.media_sinks()[1].id, sink2.id());
             EXPECT_EQ(model.media_sinks()[1].issue->info().title, issue_title);
           })));
-  mock_router_.GetIssueManager()->AddIssue(issue);
+  mock_router_->GetIssueManager()->AddIssue(issue);
 
   EXPECT_CALL(observer, OnModelUpdated(_))
       .WillOnce(WithArg<0>(Invoke([&sink2](const CastDialogModel& model) {
@@ -458,7 +456,7 @@ TEST_F(MediaRouterViewsUITest, AddAndRemoveIssue) {
         EXPECT_EQ(model.media_sinks()[1].id, sink2.id());
         EXPECT_FALSE(model.media_sinks()[1].issue.has_value());
       })));
-  mock_router_.GetIssueManager()->ClearIssue(issue_id);
+  mock_router_->GetIssueManager()->ClearIssue(issue_id);
 }
 
 TEST_F(MediaRouterViewsUITest, ShowDomainForHangouts) {
@@ -514,7 +512,7 @@ TEST_F(MediaRouterViewsUITest, RouteCreationTimeoutForPresentation) {
   content::PresentationRequest presentation_request(
       {0, 0}, {GURL("https://presentationurl.com")},
       url::Origin::Create(GURL("https://frameurl.fakeurl")));
-  ui_->OnDefaultPresentationChanged(presentation_request);
+  ui_->OnDefaultPresentationChanged(&presentation_request);
   StartCastingAndExpectTimeout(
       MediaCastMode::PRESENTATION,
       l10n_util::GetStringFUTF8(IDS_MEDIA_ROUTER_ISSUE_CREATE_ROUTE_TIMEOUT,
@@ -535,7 +533,7 @@ TEST_F(MediaRouterViewsUITest, RouteCreationLocalFileModeInTab) {
   EXPECT_CALL(*file_dialog_ptr, GetLastSelectedFileUrl())
       .WillOnce(Return(GURL(file_url)));
   content::WebContents* location_file_opened = nullptr;
-  EXPECT_CALL(mock_router_, CreateRouteInternal(_, _, _, _, _, _, _))
+  EXPECT_CALL(*mock_router_, CreateRouteInternal(_, _, _, _, _, _, _))
       .WillOnce(SaveArgWithMove<3>(&location_file_opened));
   ui_->CreateRoute(kSinkId, MediaCastMode::LOCAL_FILE);
 
@@ -695,6 +693,13 @@ TEST_F(MediaRouterViewsUITest, UpdateSinksWhenDialogMovesToAnotherDisplay) {
 
 class MediaRouterViewsUIIncognitoTest : public MediaRouterViewsUITest {
  protected:
+  void SetMediaRouterFactory() override {
+    // We must set the factory on the non-incognito browser context.
+    MediaRouterFactory::GetInstance()->SetTestingFactory(
+        MediaRouterViewsUITest::GetBrowserContext(),
+        base::BindRepeating(&MockMediaRouter::Create));
+  }
+
   content::BrowserContext* GetBrowserContext() override {
     return static_cast<Profile*>(MediaRouterViewsUITest::GetBrowserContext())
         ->GetOffTheRecordProfile();