[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..2a34cf81022d55f3cba09a1eb6a9fc92bdc99f8a 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(
@@ -20,60 +22,105 @@ MediaRouteController::Observer::~Observer() {
 }
 
 void MediaRouteController::Observer::InvalidateController() {
-  controller_->RemoveObserver(this);
   controller_ = nullptr;
   OnControllerInvalidated();
 }
 
 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)) {
-  DCHECK(media_controller_.is_bound());
-  media_controller_.set_connection_error_handler(
-      base::Bind(&MediaRouteController::Invalidate, base::Unretained(this)));
+MediaRouteController::MediaRouteController(const MediaRoute::Id& route_id,
+                                           MediaRouter* media_router)
+    : route_id_(route_id), media_router_(media_router), binding_(this) {
+  DCHECK(media_router);
 }
 
 void MediaRouteController::Play() {
-  media_controller_->Play();
+  DCHECK(is_valid_);
+  DCHECK(mojo_media_controller_.is_bound());
+  mojo_media_controller_->Play();
 }
 
 void MediaRouteController::Pause() {
-  media_controller_->Pause();
+  DCHECK(is_valid_);
+  DCHECK(mojo_media_controller_.is_bound());
+  mojo_media_controller_->Pause();
 }
 
 void MediaRouteController::Seek(base::TimeDelta time) {
-  media_controller_->Seek(time);
+  DCHECK(is_valid_);
+  DCHECK(mojo_media_controller_.is_bound());
+  mojo_media_controller_->Seek(time);
 }
 
 void MediaRouteController::SetMute(bool mute) {
-  media_controller_->SetMute(mute);
+  DCHECK(is_valid_);
+  DCHECK(mojo_media_controller_.is_bound());
+  mojo_media_controller_->SetMute(mute);
 }
 
 void MediaRouteController::SetVolume(float volume) {
-  media_controller_->SetVolume(volume);
+  DCHECK(is_valid_);
+  DCHECK(mojo_media_controller_.is_bound());
+  mojo_media_controller_->SetVolume(volume);
 }
 
 void MediaRouteController::OnMediaStatusUpdated(const MediaStatus& status) {
+  if (!is_valid_)
+    return;
+
   for (Observer& observer : observers_)
     observer.OnMediaStatusUpdated(status);
 }
 
 void MediaRouteController::Invalidate() {
+  is_valid_ = false;

[imcheng, 2017/04/05 07:18:02]

You could just call OnRouteInvalid here. Actually I would suggest you name this
OnMojoConnectionError() and make it private, and rename OnRouteInvalid() to
Invalidate(). Then it is clear that a cleanup sequence is:

1) OnMojoConnectionError() / MediaRouter detects route is terminated and removes
controller from mapping 
2) Invalidate()
3) Observers drop their controller refrerences
4) Controller gets deleted.

[takumif, 2017/04/06 19:38:32]

Done.

+  media_router_->DetachRouteController(route_id_, this);
   for (Observer& observer : observers_)
     observer.InvalidateController();
   // |this| is deleted here!
 }
 
-MediaRouteController::~MediaRouteController() {}
+void MediaRouteController::OnRouteInvalid() {
+  is_valid_ = false;
+  for (Observer& observer : observers_)
+    observer.InvalidateController();
+  // |this| is deleted here!
+}
+
+mojom::MediaStatusObserverPtr MediaRouteController::BindObserverPtr() {
+  DCHECK(is_valid_);
+  DCHECK(!binding_.is_bound());

[imcheng, 2017/04/06 21:14:04]

Did you mean to revert this?

[takumif, 2017/04/12 23:11:36]

No, putting back.

+  mojom::MediaStatusObserverPtr observer_ptr =
+      binding_.CreateInterfacePtrAndBind();
+  binding_.set_connection_error_handler(
+      base::Bind(&MediaRouteController::Invalidate, base::Unretained(this)));
+
+  return observer_ptr;
+}
+
+mojom::MediaControllerRequest
+MediaRouteController::GetMediaControllerRequest() {
+  DCHECK(is_valid_ && !mojo_media_controller_.is_bound());
+  mojom::MediaControllerRequest mojo_media_controller_request =
+      mojo::MakeRequest(&mojo_media_controller_);
+  mojo_media_controller_.set_connection_error_handler(
+      base::Bind(&MediaRouteController::Invalidate, base::Unretained(this)));
+
+  return mojo_media_controller_request;
+}
+
+MediaRouteController::~MediaRouteController() {
+  if (is_valid_)
+    media_router_->DetachRouteController(route_id_, this);
+}
 
 void MediaRouteController::AddObserver(Observer* observer) {
+  DCHECK(is_valid_);
   observers_.AddObserver(observer);
 }
 
 void MediaRouteController::RemoveObserver(Observer* observer) {
+  DCHECK(is_valid_);
   observers_.RemoveObserver(observer);
 }