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

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

Index: chrome/browser/media/router/mojo/media_route_controller.cc
diff --git a/chrome/browser/media/router/mojo/media_route_controller.cc b/chrome/browser/media/router/mojo/media_route_controller.cc
index 372d33a7ca859377dd76c959e81767e8139fea1c..e7ef189c5d3a227c89cf5e122cb5442f3df0a102 100644
--- a/chrome/browser/media/router/mojo/media_route_controller.cc
+++ b/chrome/browser/media/router/mojo/media_route_controller.cc
@@ -6,6 +6,8 @@
 
 #include <utility>
 
+#include "chrome/browser/media/router/media_router.h"
+
 namespace media_router {
 
 MediaRouteController::Observer::Observer(
@@ -29,8 +31,12 @@ void MediaRouteController::Observer::OnControllerInvalidated() {}
 
 MediaRouteController::MediaRouteController(
     const MediaRoute::Id& route_id,
-    mojom::MediaControllerPtr media_controller)
-    : route_id_(route_id), media_controller_(std::move(media_controller)) {
+    mojom::MediaControllerPtr media_controller,
+    MediaRouter* media_router)
+    : route_id_(route_id),
+      media_controller_(std::move(media_controller)),
+      media_router_(media_router),

[dcheng, 2017/03/27 22:15:26]

Shall we DCHECK(media_router)?

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

Done.

+      binding_(this) {
   DCHECK(media_controller_.is_bound());
   media_controller_.set_connection_error_handler(
       base::Bind(&MediaRouteController::Invalidate, base::Unretained(this)));

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

Suggest adding a OnMojoConnectionError method that would call into MR to detach
itself and call Invalidate().

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

I'm not sure if we need OnMojoConnectionError() and Invalidate() separately, if
the point is to keep DetachRouteController() out of Invalidate(). We can just
call DetachRouteController() from Invalidate(), since Invalidate() can call the
dtor and the dtor has to call DetachRouteController() anyways (to cover the case
in which the controller is deleted by deleting all the scoped_refptrs to it).

[imcheng, 2017/03/31 01:30:24]

Right now we have MR::RemoveInvalidRouteControllers -> MRC::Invalidate ->
MR::DetachRouteController, which is redundant. By separating the Mojo error case
from the route terminated case, it's clearer who is supposed to perform the map
entry cleanup.

@@ -67,7 +73,15 @@ void MediaRouteController::Invalidate() {
   // |this| is deleted here!
 }
 
-MediaRouteController::~MediaRouteController() {}
+mojom::MediaStatusObserverPtr MediaRouteController::BindObserverPtr() {
+  return binding_.CreateInterfacePtrAndBind();

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

Did you mean to store it in a variable and return it last?

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

Yes. I'm surprised that this even compiled.

+  binding_.set_connection_error_handler(
+      base::Bind(&MediaRouteController::Invalidate, base::Unretained(this)));
+}
+
+MediaRouteController::~MediaRouteController() {
+  media_router_->OnRouteControllerDestroyed(route_id_);
+}
 
 void MediaRouteController::AddObserver(Observer* observer) {
   observers_.AddObserver(observer);