[Media Router] Custom Controls 2 - add MediaRouter::GetRouteController()

chrome/browser/media/router/mojo/media_router_mojo_impl.cc

Index: chrome/browser/media/router/mojo/media_router_mojo_impl.cc
diff --git a/chrome/browser/media/router/mojo/media_router_mojo_impl.cc b/chrome/browser/media/router/mojo/media_router_mojo_impl.cc
index ae0ce93801a7f0296b1c1e681699ed6d8169747f..73981d0c32ec5c0a511908eb55e957b250620792 100644
--- a/chrome/browser/media/router/mojo/media_router_mojo_impl.cc
+++ b/chrome/browser/media/router/mojo/media_router_mojo_impl.cc
@@ -21,6 +21,7 @@
 #include "chrome/browser/media/router/media_routes_observer.h"
 #include "chrome/browser/media/router/media_sinks_observer.h"
 #include "chrome/browser/media/router/media_source_helper.h"
+#include "chrome/browser/media/router/mojo/media_route_controller.h"
 #include "chrome/browser/media/router/mojo/media_route_provider_util_win.h"
 #include "chrome/browser/media/router/mojo/media_router_mojo_metrics.h"
 #include "chrome/browser/media/router/route_message.h"
@@ -231,6 +232,7 @@ void MediaRouterMojoImpl::OnRoutesUpdated(
         << "Received routes update without any active observers: "
         << media_source;
   }
+  RemoveInvalidRouteControllers(routes);
 }
 
 void MediaRouterMojoImpl::RouteResponseReceived(
@@ -399,6 +401,33 @@ void MediaRouterMojoImpl::SearchSinks(
                         search_input, domain, sink_callback));
 }
 
+scoped_refptr<MediaRouteController> MediaRouterMojoImpl::GetRouteController(
+    const MediaRoute::Id& route_id) {
+  auto it = route_controllers_.find(route_id);
+  if (it != route_controllers_.end())
+    return scoped_refptr<MediaRouteController>(it->second);
+
+  mojom::MediaControllerPtr media_controller;
+  mojom::MediaControllerRequest media_controller_request =
+      mojo::MakeRequest(&media_controller);
+  scoped_refptr<MediaRouteController> route_controller =
+      new MediaRouteController(route_id, std::move(media_controller), this);
+
+  media_route_provider_->CreateMediaRouteController(
+      route_id, std::move(media_controller_request),
+      base::Bind(&MediaRouterMojoImpl::OnMediaControllerCreated,
+                 base::Unretained(this), route_id));
+  media_route_provider_->SetMediaRouteStatusObserver(
+      route_id, route_controller->BindObserverPtr());
+  route_controllers_.insert({route_id, route_controller.get()});
+  return route_controller;
+}
+
+void MediaRouterMojoImpl::OnRouteControllerDestroyed(
+    const MediaRoute::Id& route_id) {

[imcheng, 2017/03/28 01:18:38]

Consider passing in the controller pointer into this method and use it to check
against the value in the map. It can help prevent scenarios like the following
(suppose controllers 1 and 2 are of the same route):

Controller 1 created
Controller 1 invalidated, removed from map
Controller 2 created
Controller 1 destroyed -> MRMojoImpl removes controller, which is not correct

[takumif, 2017/03/29 02:34:49]

Done.

+  route_controllers_.erase(route_id);
+}
+
 bool MediaRouterMojoImpl::RegisterMediaSinksObserver(
     MediaSinksObserver* observer) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -980,4 +1009,30 @@ void MediaRouterMojoImpl::DoUpdateMediaSinks(
   media_route_provider_->UpdateMediaSinks(source_id);
 }
 
+void MediaRouterMojoImpl::RemoveInvalidRouteControllers(
+    const std::vector<MediaRoute>& routes) {
+  auto it = route_controllers_.begin();
+  while (it != route_controllers_.end()) {
+    const MediaRoute::Id& route_id = it->first;
+    if (std::find_if(routes.begin(), routes.end(),
+                     [&route_id](const MediaRoute& route) {
+                       return route.media_route_id() == route_id;
+                     }) == routes.end()) {
+      MediaRouteController* controller = it->second;
+      it = route_controllers_.erase(it);
+      // Invalidate() will result in another erase() call, which will be no-op.
+      controller->Invalidate();
+    } else {
+      ++it;
+    }
+  }
+}
+
+void MediaRouterMojoImpl::OnMediaControllerCreated(
+    const MediaRoute::Id& route_id,
+    bool success) {
+  if (base::ContainsKey(route_controllers_, route_id) && !success)

[imcheng, 2017/03/28 01:18:38]

Check !success first. Also you can combine the two calls into route_controllers_
with a find().

[takumif, 2017/03/29 02:34:49]

Done.

+    route_controllers_.at(route_id)->Invalidate();

[imcheng, 2017/03/28 01:18:38]

Also remove the entry from the map to be safe? We should detach the controller
from MR as soon as we find out that it is invalid.

[takumif, 2017/03/29 02:34:49]

Calling DetachRouteController() from Invalidate().

+}
+
 }  // namespace media_router