MediaRouter: Cache MediaRoutesObserver queries to prevent ext wake-ups

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

Index: chrome/browser/media/router/mojo/media_router_mojo_impl.cc
diff --git a/chrome/browser/media/router/mojo/media_router_mojo_impl.cc b/chrome/browser/media/router/mojo/media_router_mojo_impl.cc
index 82bf5d61afcd190d2b1eff7d8f50206112bf1dd8..dad48b51daffb46174de5147b2f08f73a977a8a4 100644
--- a/chrome/browser/media/router/mojo/media_router_mojo_impl.cc
+++ b/chrome/browser/media/router/mojo/media_router_mojo_impl.cc
@@ -186,17 +186,17 @@ void MediaRouterMojoImpl::OnSinksReceived(
 
   auto* sinks_query = it->second.get();
   sinks_query->has_cached_result = true;
-  sinks_query->origins = origins;
   sinks_query->cached_sink_list = sinks;
+  sinks_query->origins = origins;
 
-  if (!sinks_query->observers.might_have_observers()) {
-    DVLOG_WITH_INSTANCE(1)
-        << "Received sink list without any active observers: " << media_source;
-  } else {
+  if (sinks_query->observers.might_have_observers()) {
     for (auto& observer : sinks_query->observers) {
       observer.OnSinksUpdated(sinks_query->cached_sink_list,
                               sinks_query->origins);
     }
+  } else {
+    DVLOG_WITH_INSTANCE(1)
+        << "Received sink list without any active observers: " << media_source;
   }
 }
 
@@ -215,8 +215,19 @@ void MediaRouterMojoImpl::OnRoutesUpdated(
     return;
   }
 
-  for (auto& observer : it->second->observers)
-    observer.OnRoutesUpdated(routes, joinable_route_ids);
+  auto* routes_query = it->second.get();
+  routes_query->has_cached_result = true;
+  routes_query->cached_route_list = routes;
+  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;
+  }
 }
 
 void MediaRouterMojoImpl::RouteResponseReceived(
@@ -459,16 +470,48 @@ void MediaRouterMojoImpl::RegisterMediaRoutesObserver(
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   const MediaSource::Id source_id = observer->source_id();
   auto& routes_query = routes_queries_[source_id];
+  bool new_query = false;

[Wez, 2017/03/04 02:07:32]

nit: is_new_query

[miu, 2017/03/04 02:24:39]

Done.

   if (!routes_query) {
+    new_query = true;
     routes_query = base::MakeUnique<MediaRoutesQuery>();
   } else {
     DCHECK(!routes_query->observers.HasObserver(observer));
   }
 
   routes_query->observers.AddObserver(observer);
-  SetWakeReason(MediaRouteProviderWakeReason::START_OBSERVING_MEDIA_ROUTES);
-  RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoStartObservingMediaRoutes,
-                        base::Unretained(this), source_id));
+  if (new_query) {
+    SetWakeReason(MediaRouteProviderWakeReason::START_OBSERVING_MEDIA_ROUTES);
+    RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoStartObservingMediaRoutes,
+                          base::Unretained(this), source_id));
+    // The MRPM will call MediaRouterMojoImpl::OnRoutesUpdated() soon, if there
+    // are any existing routes the new observer should be aware of.
+  } else if (routes_query->has_cached_result) {
+    // Return to the event loop before notifying of a cached route list because
+    // MediaRoutesObserver is calling this method from its constructor, and that
+    // must complete before invoking its virtual OnRoutesUpdated() method.
+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI, FROM_HERE,
+        base::Bind(
+            &MediaRouterMojoImpl::NotifyOfExistingRoutesAfterRegistration,
+            weak_factory_.GetWeakPtr(), source_id, observer));
+  }
+}
+
+void MediaRouterMojoImpl::NotifyOfExistingRoutesAfterRegistration(
+    const MediaSource::Id& source_id,
+    MediaRoutesObserver* observer) const {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // Check that the route query still exists with a cached result, and that the
+  // observer is still registered. Otherwise, punt.

[Wez, 2017/03/04 02:07:32]

nit: What does "punt" mean here? Perhaps rephrase to something like "If the
query has no cached result, or the observer is gone, then there is nothing to do
here."

[miu, 2017/03/04 02:24:39]

Done.

+  const auto it = routes_queries_.find(source_id);
+  if (it == routes_queries_.end() || !it->second->has_cached_result ||
+      !it->second->observers.HasObserver(observer)) {
+    return;
+  }
+
+  observer->OnRoutesUpdated(it->second->cached_route_list,
+                            it->second->joinable_route_ids);
 }
 
 void MediaRouterMojoImpl::UnregisterMediaRoutesObserver(