MediaRouterAction: Only observe Media Routes when there is a local route.

chrome/browser/media/router/media_router_mojo_impl.cc

Index: chrome/browser/media/router/media_router_mojo_impl.cc
diff --git a/chrome/browser/media/router/media_router_mojo_impl.cc b/chrome/browser/media/router/media_router_mojo_impl.cc
index cfdd2b8d355f2b346a4fab1c63550b3c26781c9b..f20a8bf1c5aa7a4e2bad00f92bec0cfdc1b9e002 100644
--- a/chrome/browser/media/router/media_router_mojo_impl.cc
+++ b/chrome/browser/media/router/media_router_mojo_impl.cc
@@ -11,6 +11,7 @@
 #include "base/observer_list.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/media/router/issues_observer.h"
+#include "chrome/browser/media/router/local_media_routes_observer.h"
 #include "chrome/browser/media/router/media_router_factory.h"
 #include "chrome/browser/media/router/media_router_type_converters.h"
 #include "chrome/browser/media/router/media_routes_observer.h"
@@ -26,29 +27,6 @@
 namespace media_router {
 namespace {
 
-// Converts the callback result of calling Mojo CreateRoute()/JoinRoute()
-// into a local callback.
-void RouteResponseReceived(
-    const std::string& presentation_id,
-    const std::vector<MediaRouteResponseCallback>& callbacks,
-    interfaces::MediaRoutePtr media_route,
-    const mojo::String& error_text) {
-  scoped_ptr<MediaRoute> route;
-  std::string actual_presentation_id;
-  std::string error;
-  if (media_route.is_null()) {
-    // An error occurred.
-    DCHECK(!error_text.is_null());
-    error = !error_text.get().empty() ? error_text.get() : "Unknown error.";
-  } else {
-    route = media_route.To<scoped_ptr<MediaRoute>>();
-    actual_presentation_id = presentation_id;
-  }
-
-  for (const MediaRouteResponseCallback& callback : callbacks)
-    callback.Run(route.get(), actual_presentation_id, error);
-}
-
 // TODO(imcheng): We should handle failure in this case. One way is to invoke
 // all pending requests with failure. (crbug.com/490787)
 void EventPageWakeComplete(bool success) {
@@ -84,12 +62,42 @@ ConvertToPresentationSessionMessage(interfaces::RouteMessagePtr input) {
   return output.Pass();
 }
 
+class MediaRouterMediaRoutesObserver :
+    public media_router::MediaRoutesObserver {
+ public:
+  explicit MediaRouterMediaRoutesObserver(MediaRouterMojoImpl* router)
+      : MediaRoutesObserver(router),
+        router_(router) {
+    DCHECK(router);
+  }
+  ~MediaRouterMediaRoutesObserver() override {}
+
+  // media_router::MediaRoutesObserver:
+  void OnRoutesUpdated(
+      const std::vector<media_router::MediaRoute>& routes) override {
+    bool has_local_route =
+        std::find_if(routes.begin(), routes.end(),
+                     [](const media_router::MediaRoute& route) {
+                        return route.is_local(); }) !=
+        routes.end();
+
+    // |this| will be deleted beyond this point if |has_local_route| is false.
+    router_->UpdateHasLocalRoute(has_local_route);
+  }
+
+ private:
+  MediaRouterMojoImpl* const router_;
+
+  DISALLOW_COPY_AND_ASSIGN(MediaRouterMediaRoutesObserver);
+};
+
 }  // namespace
 
 MediaRouterMojoImpl::MediaRouterMojoImpl(
     extensions::EventPageTracker* event_page_tracker)
     : event_page_tracker_(event_page_tracker),
-      instance_id_(base::GenerateGUID()) {
+      instance_id_(base::GenerateGUID()),
+      has_local_route_(false) {
   DCHECK(event_page_tracker_);
 }
 
@@ -191,6 +199,32 @@ void MediaRouterMojoImpl::OnRoutesUpdated(
                     OnRoutesUpdated(routes_converted));
 }
 
+void MediaRouterMojoImpl::RouteResponseReceived(
+    const std::string& presentation_id,
+    const std::vector<MediaRouteResponseCallback>& callbacks,
+    interfaces::MediaRoutePtr media_route,
+    const mojo::String& error_text) {
+  scoped_ptr<MediaRoute> route;
+  std::string actual_presentation_id;
+  std::string error;
+  if (media_route.is_null()) {
+    // An error occurred.
+    DCHECK(!error_text.is_null());
+    error = !error_text.get().empty() ? error_text.get() : "Unknown error.";
+  } else {
+    route = media_route.To<scoped_ptr<MediaRoute>>();
+    actual_presentation_id = presentation_id;
+
+    FOR_EACH_OBSERVER(LocalMediaRoutesObserver, local_routes_observers_,

[imcheng, 2015/10/02 19:54:43]

Replace this with UpdateHasLocalRoute(true)

[apacible, 2015/10/02 20:45:48]

Done.

+                      OnHasLocalRouteUpdated(true));
+    if (!routes_observer_)
+      routes_observer_.reset(new MediaRouterMediaRoutesObserver(this));
+  }
+
+  for (const MediaRouteResponseCallback& callback : callbacks)
+    callback.Run(route.get(), actual_presentation_id, error);
+}
+
 void MediaRouterMojoImpl::CreateRoute(
     const MediaSource::Id& source_id,
     const MediaSink::Id& sink_id,
@@ -392,6 +426,25 @@ void MediaRouterMojoImpl::UnregisterPresentationSessionMessagesObserver(
   }
 }
 
+void MediaRouterMojoImpl::RegisterLocalMediaRoutesObserver(

[imcheng, 2015/10/02 19:54:43]

we should add a test for this. Maybe in a separate patch.

[apacible, 2015/10/02 20:45:48]

Acknowledged.

+    LocalMediaRoutesObserver* observer) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(observer);
+
+  DCHECK(!local_routes_observers_.HasObserver(observer));
+  local_routes_observers_.AddObserver(observer);
+}
+
+void MediaRouterMojoImpl::UnregisterLocalMediaRoutesObserver(
+    LocalMediaRoutesObserver* observer) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(observer);
+
+  if (!local_routes_observers_.HasObserver(observer))
+    return;
+  local_routes_observers_.RemoveObserver(observer);
+}
+
 void MediaRouterMojoImpl::DoCreateRoute(
     const MediaSource::Id& source_id,
     const MediaSink::Id& sink_id,
@@ -404,7 +457,8 @@ void MediaRouterMojoImpl::DoCreateRoute(
                          << ", presentation ID: " << presentation_id;
   media_route_provider_->CreateRoute(
       source_id, sink_id, presentation_id, origin, tab_id,
-      base::Bind(&RouteResponseReceived, presentation_id, callbacks));
+      base::Bind(&MediaRouterMojoImpl::RouteResponseReceived,
+          base::Unretained(this), presentation_id, callbacks));
 }
 
 void MediaRouterMojoImpl::DoJoinRoute(
@@ -417,7 +471,8 @@ void MediaRouterMojoImpl::DoJoinRoute(
                          << ", presentation ID: " << presentation_id;
   media_route_provider_->JoinRoute(
       source_id, presentation_id, origin, tab_id,
-      base::Bind(&RouteResponseReceived, presentation_id, callbacks));
+      base::Bind(&MediaRouterMojoImpl::RouteResponseReceived,
+          base::Unretained(this), presentation_id, callbacks));
 }
 
 void MediaRouterMojoImpl::DoCloseRoute(const MediaRoute::Id& route_id) {
@@ -517,6 +572,22 @@ void MediaRouterMojoImpl::DoOnPresentationSessionDetached(
   media_route_provider_->OnPresentationSessionDetached(route_id);
 }
 
+bool MediaRouterMojoImpl::HasLocalRoute() const {
+  return has_local_route_;
+}
+
+void MediaRouterMojoImpl::UpdateHasLocalRoute(bool has_local_route) {
+  if (has_local_route_ != has_local_route) {
+    if (!has_local_route)
+      routes_observer_.reset();
+
+    FOR_EACH_OBSERVER(LocalMediaRoutesObserver, local_routes_observers_,
+                      OnHasLocalRouteUpdated(has_local_route_));

[imcheng, 2015/10/02 19:54:43]

you would need to have updated has_local_route_ before you use it here.

[apacible, 2015/10/02 20:45:48]

Done.

+  }
+
+  has_local_route_ = has_local_route;
+}
+
 void MediaRouterMojoImpl::DoStartObservingMediaSinks(
     const MediaSource::Id& source_id) {
   DVLOG_WITH_INSTANCE(1) << "DoStartObservingMediaSinks: " << source_id;