[Media Router] Custom Controls 1 - Add MediaStatus, MediaRouteController, and mojo interfaces

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
new file mode 100644
index 0000000000000000000000000000000000000000..ad1595abe4a3de9b9c840d7438415ea24b2c91ff
--- /dev/null
+++ b/chrome/browser/media/router/mojo/media_route_controller.cc
@@ -0,0 +1,81 @@
+// Copyright 2017 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/mojo/media_route_controller.h"
+
+#include <utility>
+
+namespace media_router {
+
+MediaRouteController::Observer::Observer(
+    scoped_refptr<MediaRouteController> controller)
+    : controller_(std::move(controller)) {
+  controller_->AddObserver(this);
+}
+
+MediaRouteController::Observer::~Observer() {
+  if (controller_)
+    controller_->RemoveObserver(this);
+}
+
+void MediaRouteController::Observer::OnControllerInvalidated() {
+  controller_ = nullptr;
+}
+
+MediaRouteController::MediaRouteController(
+    const MediaRoute::Id& route_id,
+    mojom::MediaControllerPtr media_controller,
+    const base::Closure& destructor_callback)
+    : route_id_(route_id),
+      media_controller_(std::move(media_controller)),
+      destructor_callback_(destructor_callback) {
+  DCHECK(media_controller_.is_bound());
+  media_controller_.set_connection_error_handler(base::Bind(
+      &MediaRouteController::OnControllerInvalidated, base::Unretained(this)));
+}
+
+void MediaRouteController::Play() {
+  media_controller_->Play();
+}
+
+void MediaRouteController::Pause() {
+  media_controller_->Pause();
+}
+
+void MediaRouteController::Seek(base::TimeDelta time) {
+  media_controller_->Seek(time);
+}
+
+void MediaRouteController::SetMute(bool mute) {
+  media_controller_->SetMute(mute);
+}
+
+void MediaRouteController::SetVolume(float volume) {
+  media_controller_->SetVolume(volume);
+}
+
+void MediaRouteController::OnMediaStatusUpdated(const MediaStatus& status) {
+  for (Observer& observer : observers_)
+    observer.OnMediaStatusUpdated(status);
+}
+
+void MediaRouteController::OnControllerInvalidated() {
+  for (Observer& observer : observers_)
+    observer.OnControllerInvalidated();
+  // |this| is deleted here!
+}
+
+MediaRouteController::~MediaRouteController() {
+  destructor_callback_.Run();

[mark a. foltz, 2017/03/17 23:55:04]

What does this do?  If there is some cleanup that needs to happen outside the
MediaController when destroyed, at first glance, I would prefer the caller do it
then destroy this vs. passing a callback to be run later.

It's also dangerous:  if the callback attempts to add a new scoped_refptr to
this, it will result in a use-after-free.

[takumif, 2017/03/20 22:10:45]

Replacing this line with
router_->OnRouteControllerDestroyed();
in the next CL. MRController needs to notify MediaRouter because MediaRouter
does not necessarily decide when the MRController is destroyed.

[mark a. foltz, 2017/03/21 00:05:21]

The design based on notifying observers before the dtor is invoked is better. 
Thanks.

+}
+
+void MediaRouteController::AddObserver(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void MediaRouteController::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+}  // namespace media_router