Add Media Router Mojo impl code.

chrome/browser/media/router/media_router_mojo_impl.cc

+// Copyright 2015 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/media_router_mojo_impl.h"
+
+#include "base/bind.h"
+#include "base/guid.h"
+#include "base/logging.h"
+#include "base/observer_list.h"
+#include "chrome/browser/media/router/media_router_mojo_impl_factory.h"
+#include "chrome/browser/media/router/media_router_type_converters.h"
+#include "chrome/browser/media/router/media_routes_observer.h"
+#include "chrome/browser/media/router/media_sinks_observer.h"
+#include "extensions/browser/process_manager.h"
+
+#define DVLOG_WITH_INSTANCE(level) \
+  DVLOG(level) << "MR #" << instance_id_ << ": "
+
+#define DLOG_WITH_INSTANCE(level) DLOG(level) << "MR #" << instance_id_ << ": "
+
+namespace media_router {
+namespace {
+
+// Converts the callback result of calling Mojo CreateRoute() into a local
+// callback.
+void CreateRouteFinished(const MediaSinkId& sink_id,
+                         const MediaRouteResponseCallback& callback,
+                         interfaces::MediaRoutePtr media_route,
+                         const mojo::String& error_text) {
+  if (media_route.is_null()) {
+    // An error occurred.
+    DCHECK(!error_text.is_null());
+    callback.Run(nullptr, !error_text.get().empty() ? error_text.get()
+                                                    : "Unknown error.");
+    return;
+  }
+  callback.Run(make_scoped_ptr(new MediaRoute(media_route.To<MediaRoute>())),
+               "");
+}
+
+void EventPageWakeComplete(bool success) {
+  if (!success)
+    LOG(ERROR) << "An error encountered while waking the event page.";

[Wez, 2015/05/20 23:37:48]

Shouldn't we do fun stuff like dispatch errors for any pending_requests_, and
make sure no new requests are queued, in this case?

Do we have a test-case for wake failure? :)

[imcheng (use chromium acct), 2015/05/21 19:28:50]

Filed a tracking bug for handling failure.

+}
+
+}  // namespace
+
+MediaRouterMojoImpl::MediaRouterMojoImpl()
+    : event_page_tracker_(nullptr), instance_id_(base::GenerateGUID()) {
+}
+
+MediaRouterMojoImpl::MediaRouterMojoImpl(
+    const std::string& mrpm_extension_id,
+    extensions::EventPageTracker* event_page_tracker_for_test)
+    : MediaRouterMojoImpl() {
+  DCHECK(!mrpm_extension_id.empty());
+  DCHECK(event_page_tracker_for_test);

[Wez, 2015/05/20 23:37:48]

nit: Suggest a blank line between pre-condition DCHECKs and the rest of the
function body, here and below - helps break up the code to be more readable at a
glance.

[imcheng (use chromium acct), 2015/05/21 19:28:50]

Done.

+  mojo_media_router_extension_id_ = mrpm_extension_id;
+  event_page_tracker_ = event_page_tracker_for_test;
+}
+
+MediaRouterMojoImpl::~MediaRouterMojoImpl() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+}
+
+// static
+void MediaRouterMojoImpl::BindToRequest(
+    const std::string& extension_id,
+    content::BrowserContext* context,
+    mojo::InterfaceRequest<interfaces::MediaRouterObserver> request) {
+  MediaRouterMojoImpl* impl =
+      MediaRouterMojoImplFactory::GetApiForBrowserContext(context);
+  DCHECK(impl);
+  impl->Bind(request.Pass());
+  impl->MonitorExtension(extension_id, context);
+}
+
+void MediaRouterMojoImpl::Bind(
+    mojo::InterfaceRequest<interfaces::MediaRouterObserver> request) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  binding_.reset(
+      new mojo::Binding<interfaces::MediaRouterObserver>(this, request.Pass()));
+  binding_->set_error_handler(this);
+}
+
+void MediaRouterMojoImpl::OnConnectionError() {
+  DCHECK(thread_checker_.CalledOnValidThread());

[Wez, 2015/05/20 23:37:48]

What happens to pendign requests at this point?

[imcheng (use chromium acct), 2015/05/21 19:28:50]

There's an edge case here where this can happen between Bind() and
ProvideMediaRouter(), in that case we should probably flush the pending requests
with failure. Added a tracking bug for it.

+  mojo_media_router_.reset();
+  binding_.reset();
+}
+
+void MediaRouterMojoImpl::ProvideMediaRouter(
+    interfaces::MediaRouterPtr media_router_ptr,
+    const interfaces::MediaRouterObserver::ProvideMediaRouterCallback&
+        callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  mojo_media_router_ = media_router_ptr.Pass();
+  mojo_media_router_.set_error_handler(this);
+  callback.Run(instance_id_);
+  ExecutePendingRequests();
+}
+
+void MediaRouterMojoImpl::OnMessage(const mojo::String& route_id,
+                                    const mojo::String& message) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  NOTIMPLEMENTED();

[Wez, 2015/05/20 23:37:47]

is this something that will be implemented, ever? If so then add a comment and
bug #? If not then should this be NOTREACHED() - i.e. getting here is really bad
and indicates an error somewhere else?

[imcheng (use chromium acct), 2015/05/21 19:28:51]

Yes, this will be implemented eventually.

+}
+
+void MediaRouterMojoImpl::OnIssue(const interfaces::IssuePtr issue) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  NOTIMPLEMENTED();

[Wez, 2015/05/20 23:37:48]

See above.

[imcheng (use chromium acct), 2015/05/21 19:28:50]

Ditto.

+}
+
+void MediaRouterMojoImpl::OnSinksReceived(
+    const mojo::String& media_source,
+    mojo::Array<interfaces::MediaSinkPtr> sinks) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DVLOG_WITH_INSTANCE(1) << "OnSinksReceived";
+  std::vector<MediaSink> sinks_converted;
+  sinks_converted.reserve(sinks.size());
+
+  for (size_t i = 0; i < sinks.size(); ++i) {
+    sinks_converted.push_back(sinks[i].To<MediaSink>());
+  }
+
+  auto it = sinks_observers_.find(media_source);
+  if (it == sinks_observers_.end()) {
+    DVLOG_WITH_INSTANCE(1)
+        << "Received sink list without any active observers: " << media_source;
+  } else {
+    FOR_EACH_OBSERVER(MediaSinksObserver, *it->second,
+                      OnSinksReceived(sinks_converted));
+  }
+}
+
+void MediaRouterMojoImpl::OnRoutesUpdated(
+    mojo::Array<interfaces::MediaRoutePtr> routes) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DVLOG_WITH_INSTANCE(1) << "OnRoutesUpdated";
+
+  std::vector<MediaRoute> routes_converted;
+  routes_converted.reserve(routes.size());
+
+  for (size_t i = 0; i < routes.size(); ++i) {
+    routes_converted.push_back(routes[i].To<MediaRoute>());
+  }
+
+  FOR_EACH_OBSERVER(MediaRoutesObserver, routes_observers_,
+                    OnRoutesUpdated(routes_converted));
+}
+
+// ----------------------------------------------------------------------------
+// MediaRouter methods.

[Wez, 2015/05/20 23:37:48]

nit: Remove this comment? It doesn't really add anything - I can see what this
method is about by looking at the header.

[imcheng (use chromium acct), 2015/05/21 19:28:50]

Done.

+
+void MediaRouterMojoImpl::CreateRoute(
+    const MediaSourceId& source_id,
+    const MediaSinkId& sink_id,
+    const MediaRouteResponseCallback& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoCreateRoute,
+                        base::Unretained(this), source_id, sink_id, callback));
+}
+
+void MediaRouterMojoImpl::CloseRoute(const MediaRouteId& route_id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoCloseRoute,
+                        base::Unretained(this), route_id));
+}
+
+void MediaRouterMojoImpl::PostMessage(const MediaRouteId& route_id,
+                                      const std::string& message) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoPostMessage,
+                        base::Unretained(this), route_id, message));
+}
+
+void MediaRouterMojoImpl::ClearIssue(const Issue::IssueId& issue_id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoClearIssue,
+                        base::Unretained(this), issue_id));
+}
+
+bool MediaRouterMojoImpl::RegisterMediaSinksObserver(
+    MediaSinksObserver* observer) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  // Create an observer list for the media source and add |observer|
+  // to it. Fail if |observer| is already registered.
+  const std::string& source_id = observer->source().id();
+  ObserverList<MediaSinksObserver>* observer_list =
+      sinks_observers_.get(source_id);
+  if (!observer_list) {
+    observer_list = new ObserverList<MediaSinksObserver>;
+    sinks_observers_.add(source_id, make_scoped_ptr(observer_list));
+  } else {
+    if (observer_list->HasObserver(observer)) {
+      DLOG_WITH_INSTANCE(FATAL)
+          << "Redundant RegisterMediaSinksObserver call detected.";

[Wez, 2015/05/20 23:37:48]

s/Redundant/Duplicate

Does this indicate a coding error in Chrome? If so then consider NOTREACHED(),
and get rid of the return-code from this function.

[imcheng (use chromium acct), 2015/05/21 19:28:50]

How about a DCHECK? That is what we used to have in MediaRouterImpl before it is
removed.

+      return false;
+    }
+  }
+
+  RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoStartObservingMediaSinks,
+                        base::Unretained(this), source_id));
+  observer_list->AddObserver(observer);

[Wez, 2015/05/20 23:37:47]

nit: In principle you should add the observer _before_ you start observing media
sinks, so that any notifications about them don't appear between you starting
observing, and registering the observer. I realise this is single-threaded, so
that's not an issue here, but I'd order things in what is _logically_ the right
order here, in case the code changes!

Do you still need to start observing if you are already observing for a
particular source_id? If so then add a comment to clarify why.

[imcheng (use chromium acct), 2015/05/21 19:28:51]

Good point. Swapped the order.

+
+  return true;
+}
+
+void MediaRouterMojoImpl::UnregisterMediaSinksObserver(
+    MediaSinksObserver* observer) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  const std::string& source_id = observer->source().id();
+  auto* observer_list = sinks_observers_.get(source_id);
+  if (!observer_list || !observer_list->HasObserver(observer)) {
+    return;

[Wez, 2015/05/20 23:37:47]

Does reaching here indicate a coding error? If so then NOTREACHED()! Or you
could just not do these checks, and let RemoveObserver() trap the problem and
crash us.

[imcheng (use chromium acct), 2015/05/21 19:28:51]

Done.

+  }
+  observer_list->RemoveObserver(observer);
+
+  // If we are removing the final observer for the source, then stop
+  // observing sinks for it.

[Wez, 2015/05/20 23:37:48]

nit: Suggest clarifying here that might_have_observers() is reliable here (i.e.
won't return true even though there are still observers) because we are
definitely not iterating over it right now - I'm assuming it's guaranteed that
we never UnregisterMediaSinksObserver from within an observer callback, though.
;)

[imcheng (use chromium acct), 2015/05/21 19:28:51]

Done.

+  if (!observer_list->might_have_observers()) {
+    RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoStopObservingMediaSinks,
+                          base::Unretained(this), source_id));
+    sinks_observers_.erase(source_id);
+  }
+}
+
+bool MediaRouterMojoImpl::RegisterMediaRoutesObserver(
+    MediaRoutesObserver* observer) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (routes_observers_.HasObserver(observer)) {
+    DLOG(FATAL) << "Redundant RegisterMediaRoutesObserver() call detected.";

[Wez, 2015/05/20 23:37:48]

See above re these never-reached error conditions.

[imcheng (use chromium acct), 2015/05/21 19:28:50]

Done.

+    return true;

[Wez, 2015/05/20 23:37:48]

Why return true here?

[imcheng (use chromium acct), 2015/05/21 19:28:50]

removed return value.

+  }
+
+  RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoStartObservingMediaRoutes,
+                        base::Unretained(this)));
+  routes_observers_.AddObserver(observer);
+  return true;
+}
+
+void MediaRouterMojoImpl::UnregisterMediaRoutesObserver(
+    MediaRoutesObserver* observer) {
+  if (!routes_observers_.HasObserver(observer))
+    return;
+  routes_observers_.RemoveObserver(observer);
+  if (!routes_observers_.might_have_observers()) {
+    RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoStopObservingMediaRoutes,
+                          base::Unretained(this)));
+  }
+}
+
+void MediaRouterMojoImpl::AddIssuesObserver(IssuesObserver* observer) {
+  NOTIMPLEMENTED();

[Wez, 2015/05/20 23:37:48]

Is there  abug # for adding these?

[imcheng (use chromium acct), 2015/05/21 19:28:50]

Done.

+}
+
+void MediaRouterMojoImpl::RemoveIssuesObserver(IssuesObserver* observer) {
+  NOTIMPLEMENTED();
+}
+
+void MediaRouterMojoImpl::DoCreateRoute(
+    const MediaSourceId& source_id,
+    const MediaSinkId& sink_id,
+    const MediaRouteResponseCallback& callback) {
+  DVLOG_WITH_INSTANCE(1) << "CreateRoute " << source_id << "=>" << sink_id;
+  mojo_media_router_->CreateRoute(
+      source_id, sink_id, base::Bind(&CreateRouteFinished, sink_id, callback));
+}
+
+void MediaRouterMojoImpl::DoCloseRoute(const MediaRouteId& route_id) {
+  DVLOG_WITH_INSTANCE(1) << "CloseRoute " << route_id;
+  mojo_media_router_->CloseRoute(route_id);
+}
+
+void MediaRouterMojoImpl::DoPostMessage(const MediaRouteId& route_id,
+                                        const std::string& message) {
+  DVLOG_WITH_INSTANCE(1) << "PostMessage " << route_id;
+  mojo_media_router_->PostMessage(route_id, message);
+}
+
+void MediaRouterMojoImpl::DoClearIssue(const Issue::IssueId& issue_id) {
+  DVLOG_WITH_INSTANCE(1) << "ClearIssue " << issue_id;
+  mojo_media_router_->ClearIssue(issue_id);
+}
+
+void MediaRouterMojoImpl::DoStartObservingMediaSinks(
+    const std::string& source_id) {
+  DVLOG_WITH_INSTANCE(1) << "StartObservingMediaSinks: " << source_id;
+  mojo_media_router_->StartObservingMediaSinks(source_id);
+}
+
+void MediaRouterMojoImpl::DoStopObservingMediaSinks(
+    const std::string& source_id) {
+  DVLOG_WITH_INSTANCE(1) << "StopObservingMediaSinks: " << source_id;
+  mojo_media_router_->StopObservingMediaSinks(source_id);
+}
+
+void MediaRouterMojoImpl::DoStartObservingMediaRoutes() {
+  DVLOG_WITH_INSTANCE(1) << "StartObservingMediaRoutes";
+  mojo_media_router_->StartObservingMediaRoutes();
+}
+
+void MediaRouterMojoImpl::DoStopObservingMediaRoutes() {
+  DVLOG_WITH_INSTANCE(1) << "StopObservingMediaRoutes";
+  mojo_media_router_->StopObservingMediaRoutes();
+}
+
+void MediaRouterMojoImpl::EnqueueTask(const base::Closure& closure) {
+  DVLOG_WITH_INSTANCE(2) << "EnqueueTask (size=" << pending_requests_.size()
+                         << ")";

[Wez, 2015/05/20 23:37:48]

Does this get used anywhere...?

[imcheng (use chromium acct), 2015/05/21 19:28:50]

Changed below to use this function.

+  pending_requests_.push_back(closure);
+}
+
+void MediaRouterMojoImpl::MonitorExtension(const std::string& extension_id,
+                                           content::BrowserContext* context) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(context);
+  mojo_media_router_extension_id_ = extension_id;
+  event_page_tracker_ = extensions::ProcessManager::Get(context);

[Wez, 2015/05/20 23:37:48]

This doesn't seem to be doing anything to actually request extension
state-change notifications?

[imcheng (use chromium acct), 2015/05/21 19:28:50]

Yeah, we changed from a push model to a pull model at some point, so the name is
no longer accurate. How about moving these into Bind() and renaming it
BindAndInitExtensionInfo()?

[Wez, 2015/05/21 23:32:47]

Bind could certainly use a re-name to something more helpful - I'm drawing a
blank on how to describe it better, though.

[imcheng (use chromium acct), 2015/05/22 00:54:07]

I ended up keeping the Bind() and extension logic separate since
MediaRouterMojoTest setup code only requires the former. How about calling it
BindToMojoRequest?

+}
+
+void MediaRouterMojoImpl::RunOrDefer(const base::Closure& request_cb) {

[Wez, 2015/05/20 23:37:48]

This isn't the request callback, it's the request to run - suggest just call it
|request|

[imcheng (use chromium acct), 2015/05/21 19:28:51]

Done.

+  DCHECK(event_page_tracker_);
+  if (event_page_tracker_->IsEventPageSuspended(
+          mojo_media_router_extension_id_)) {
+    DVLOG_WITH_INSTANCE(1) << "Waking event page.";
+    pending_requests_.push_back(request_cb);
+    if (!event_page_tracker_->WakeEventPage(
+            mojo_media_router_extension_id_,
+            base::Bind(&EventPageWakeComplete))) {

[Wez, 2015/05/20 23:37:48]

Seems strange that we don't care about the wake compeltion notification; I'd
expect that we at least trap it and handle failure, and potentially start a
timer to ensure that we cope with the extension starting but then failing to
connect via Mojo.

[imcheng (use chromium acct), 2015/05/21 19:28:50]

Yes, I agree. But these adds complexity to this class, so I will file a TODO for
now. We can work on it after the upstreaming is complete.

[Wez, 2015/05/21 23:32:47]

Acknowledged.

+      LOG(ERROR) << "An error encountered while waking the event page.";
+    }
+    mojo_media_router_.reset();
+  } else if (!mojo_media_router_) {
+    DVLOG_WITH_INSTANCE(1) << "Extension is awake, awaiting MRPM connection.";
+    pending_requests_.push_back(request_cb);
+  } else {
+    request_cb.Run();
+  }
+}
+
+void MediaRouterMojoImpl::ExecutePendingRequests() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(mojo_media_router_);
+  DCHECK(event_page_tracker_);
+  if (event_page_tracker_->IsEventPageSuspended(
+          mojo_media_router_extension_id_)) {
+    DVLOG_WITH_INSTANCE(1)
+        << "ExecutePendingRequests was called while extension is suspended.";
+    return;
+  }
+
+  for (const auto& next_request : pending_requests_)
+    next_request.Run();
+
+  pending_requests_.clear();
+}
+
+}  // namespace media_router