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

chrome/browser/media/router/mojo/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/mojo/media_router_mojo_impl.h"
 
 #include <stddef.h>
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/guid.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/scoped_vector.h"
 #include "base/observer_list.h"
 #include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/media/router/issues_observer.h"
 #include "chrome/browser/media/router/media_router_factory.h"
 #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"
 #include "chrome/browser/media/router/route_message_observer.h"
 #include "chrome/browser/sessions/session_tab_helper.h"
 #include "content/public/browser/browser_thread.h"
 #include "extensions/browser/process_manager.h"
 
 #define DVLOG_WITH_INSTANCE(level) \
   DVLOG(level) << "MR #" << instance_id_ << ": "
@@ skipping 190 matching lines @@
   routes_query->joinable_route_ids = joinable_route_ids;
 
   if (routes_query->observers.might_have_observers()) {
     for (auto& observer : routes_query->observers)
       observer.OnRoutesUpdated(routes, joinable_route_ids);
   } else {
     DVLOG_WITH_INSTANCE(1)
         << "Received routes update without any active observers: "
         << media_source;
   }
+  RemoveInvalidRouteControllers(routes);
 }
 
 void MediaRouterMojoImpl::RouteResponseReceived(
     const std::string& presentation_id,
     bool is_incognito,
     const std::vector<MediaRouteResponseCallback>& callbacks,
     bool is_join,
     const base::Optional<MediaRoute>& media_route,
     const base::Optional<std::string>& error_text,
     RouteRequestResult::ResultCode result_code) {
@@ skipping 148 matching lines @@
     const std::string& domain,
     const MediaSinkSearchResponseCallback& sink_callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   SetWakeReason(MediaRouteProviderWakeReason::SEARCH_SINKS);
   RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoSearchSinks,
                         base::Unretained(this), sink_id, source_id,
                         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;

[mark a. foltz, 2017/03/30 23:03:28]

Can you prefix Mojo variables with mojo_ for readability?

[takumif, 2017/04/04 03:19:50]

Done.

+  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(

[mark a. foltz, 2017/03/30 23:03:28]

What guarantees that the event page is alive?  Most calls through to the MRPM
will queues requests if necessary until the event page is ready to receive them.

[takumif, 2017/04/04 03:19:51]

These requests should be queued as well. I can't std::move arguments into
base::Bind() *, so I'm giving MediaRouteController a method for getting
MediaControllerRequest, and calling that in DoCreateMediaRouteController().

* I tried using base::BindOnce() instead, but was having issues with queuing
requests with std::deque::push_back(). It was calling the copy ctor of
base::OnceCallback rather than the move ctor, even with the use of std::move().

+      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;

[mark a. foltz, 2017/03/30 23:03:28]

What state is this in before OnMediaControllerCreated is invoked?

[takumif, 2017/04/04 03:19:50]

Before DoCreateMediaRouteController() is called, route_controller won't be
accepting any media commands, which is fine because the media controls in WebUI
will only be shown after the first media status update. 

+}
+
 bool MediaRouterMojoImpl::RegisterMediaSinksObserver(
     MediaSinksObserver* observer) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   // 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();
   std::unique_ptr<MediaSinksQuery>& sinks_query = sinks_queries_[source_id];
   bool is_new_query = false;
   if (!sinks_query) {
@@ skipping 173 matching lines @@
   it->second->RemoveObserver(observer);
   if (!it->second->might_have_observers()) {
     message_observers_.erase(route_id);
     SetWakeReason(
         MediaRouteProviderWakeReason::STOP_LISTENING_FOR_ROUTE_MESSAGES);
     RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoStopListeningForRouteMessages,
                           base::Unretained(this), route_id));
   }
 }
 
+void MediaRouterMojoImpl::DetachRouteController(
+    const MediaRoute::Id& route_id,
+    MediaRouteController* controller) {
+  auto it = route_controllers_.find(route_id);
+  if (it != route_controllers_.end() && it->second == controller)
+    route_controllers_.erase(it);
+}
+
 void MediaRouterMojoImpl::DoCreateRoute(
     const MediaSource::Id& source_id,
     const MediaSink::Id& sink_id,
     const url::Origin& origin,
     int tab_id,
     const std::vector<MediaRouteResponseCallback>& callbacks,
     base::TimeDelta timeout,
     bool incognito) {
   std::string presentation_id = MediaRouterBase::CreatePresentationId();
   DVLOG_WITH_INSTANCE(1) << "DoCreateRoute " << source_id << "=>" << sink_id
@@ skipping 368 matching lines @@
   RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoUpdateMediaSinks,
                         base::Unretained(this), source_id));
 }
 
 void MediaRouterMojoImpl::DoUpdateMediaSinks(
     const MediaSource::Id& source_id) {
   DVLOG_WITH_INSTANCE(1) << "DoUpdateMediaSinks" << source_id;
   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();

[mark a. foltz, 2017/03/30 23:03:28]

Doesn't this call back into DetachRouteController() above (which will be a no-op
since you just erase()d controller)?

Can you add a specific method like controller->OnRouteInvalid() that will drop
observers without  re-entrancy here?

[takumif, 2017/04/04 03:19:50]

Yes. Added OnRouteInvalid().

+    } else {
+      ++it;
+    }
+  }
+}
+
+void MediaRouterMojoImpl::OnMediaControllerCreated(

[mark a. foltz, 2017/03/30 23:03:28]

Shouldn't this return a boolean per the mojom?

[takumif, 2017/04/04 03:19:51]

No, this is the callback that gets called, so it doesn't need to return
anything.

+    const MediaRoute::Id& route_id,
+    bool success) {
+  if (success)
+    return;
+
+  auto it = route_controllers_.find(route_id);
+  if (it != route_controllers_.end())
+    it->second->Invalidate();
+}
+
 }  // namespace media_router