[Presentation API] Add support for multiple URLs in PresentationRequest on Media Router UI side

chrome/browser/ui/webui/media_router/query_result_manager.cc

Index: chrome/browser/ui/webui/media_router/query_result_manager.cc
diff --git a/chrome/browser/ui/webui/media_router/query_result_manager.cc b/chrome/browser/ui/webui/media_router/query_result_manager.cc
index 132bd925c77bc8112298f6a1e27164516fffcd38..b6d24584dc4b630f13ffd57e2ddced85def99a02 100644
--- a/chrome/browser/ui/webui/media_router/query_result_manager.cc
+++ b/chrome/browser/ui/webui/media_router/query_result_manager.cc
@@ -4,6 +4,8 @@
 
 #include "chrome/browser/ui/webui/media_router/query_result_manager.h"
 
+#include <utility>
+
 #include "base/containers/hash_tables.h"
 #include "base/stl_util.h"
 #include "chrome/browser/media/router/media_router.h"
@@ -13,7 +15,7 @@
 namespace media_router {
 
 // MediaSinkObserver that propagates results back to |result_manager|.
-// An instance of this class is associated with each registered MediaCastMode.
+// An instance of this class is associated with each registered MediaSource.
 class QueryResultManager::CastModeMediaSinksObserver
     : public MediaSinksObserver {
  public:
@@ -24,6 +26,7 @@ class QueryResultManager::CastModeMediaSinksObserver
                              QueryResultManager* result_manager)
       : MediaSinksObserver(router, source, origin),
         cast_mode_(cast_mode),
+        source_(source),
         result_manager_(result_manager) {
     DCHECK(result_manager);
   }
@@ -36,7 +39,7 @@ class QueryResultManager::CastModeMediaSinksObserver
     for (const MediaSink& sink : result) {
       latest_sink_ids_.push_back(sink.id());
     }
-    result_manager_->UpdateWithSinksQueryResult(cast_mode_, result);
+    result_manager_->SetSinksCompatibleWithSource(cast_mode_, source_, result);
     result_manager_->NotifyOnResultsUpdated();
   }
 
@@ -50,6 +53,7 @@ class QueryResultManager::CastModeMediaSinksObserver
 
  private:
   const MediaCastMode cast_mode_;
+  const MediaSource source_;
   std::vector<MediaSink::Id> latest_sink_ids_;
   QueryResultManager* const result_manager_;
 };
@@ -74,106 +78,138 @@ void QueryResultManager::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
-void QueryResultManager::StartSinksQuery(MediaCastMode cast_mode,
-                                         const MediaSource& source,
-                                         const GURL& origin) {
+void QueryResultManager::StartSinksQuery(
+    MediaCastMode cast_mode,
+    const std::vector<MediaSource>& sources,
+    const GURL& origin) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  if (source.Empty()) {
+  if (sources.empty()) {
     LOG(WARNING) << "StartSinksQuery called with empty source for "
                  << cast_mode;
     return;
   }
 
-  SetSourceForCastMode(cast_mode, source);
-  RemoveObserverForCastMode(cast_mode);
-  UpdateWithSinksQueryResult(cast_mode, std::vector<MediaSink>());
-
-  std::unique_ptr<CastModeMediaSinksObserver> observer(
-      new CastModeMediaSinksObserver(cast_mode, source, origin, router_, this));
-  observer->Init();
-  auto result =
-      sinks_observers_.insert(std::make_pair(cast_mode, std::move(observer)));
-  DCHECK(result.second);
+  RemoveOldSourcesForCastMode(cast_mode, sources);
+  AddObserversForCastMode(cast_mode, sources, origin);
+  cast_mode_sources_[cast_mode] = sources;
   NotifyOnResultsUpdated();
 }
 
 void QueryResultManager::StopSinksQuery(MediaCastMode cast_mode) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  RemoveObserverForCastMode(cast_mode);
-  SetSourceForCastMode(cast_mode, MediaSource());
-  UpdateWithSinksQueryResult(cast_mode, std::vector<MediaSink>());
+  RemoveOldSourcesForCastMode(cast_mode, std::vector<MediaSource>());
+  cast_mode_sources_.erase(cast_mode);
   NotifyOnResultsUpdated();
 }
 
-void QueryResultManager::SetSourceForCastMode(
-      MediaCastMode cast_mode, const MediaSource& source) {
+CastModeSet QueryResultManager::GetSupportedCastModes() const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  cast_mode_sources_[cast_mode] = source;
+  CastModeSet modes;
+  for (const auto& observer_pair : cast_mode_sources_)
+    modes.insert(observer_pair.first);

[mark a. foltz, 2016/09/09 22:22:27]

Blank line after for statement

[takumif, 2016/09/13 03:48:21]

Done.

+  return modes;
 }
 
-void QueryResultManager::RemoveObserverForCastMode(MediaCastMode cast_mode) {
-  auto observers_it = sinks_observers_.find(cast_mode);
-  if (observers_it != sinks_observers_.end())
-    sinks_observers_.erase(observers_it);
+std::unique_ptr<MediaSource> QueryResultManager::GetSourceForCastModeAndSink(
+    MediaCastMode cast_mode, MediaSink::Id sink_id) const {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  for (const auto& sink_pair : all_sinks_) {
+    if (sink_pair.first.id() == sink_id) {

[mark a. foltz, 2016/09/09 22:22:27]

std::find_if ?

[takumif, 2016/09/13 03:48:21]

find_if would look messier, since we can't use auto for the parameter for the
lambda function passed into it.

+      return GetHighestPrioritySourceForCastModeAndSink(
+          cast_mode, sink_pair.first);
+    }
+  }
+  return std::unique_ptr<MediaSource>();
 }
 
-bool QueryResultManager::IsValid(const MediaSinkWithCastModes& entry) const {
-  return !entry.cast_modes.empty();
+std::vector<MediaSource> QueryResultManager::GetSourcesForCastMode(
+    MediaCastMode cast_mode) const {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  return base::ContainsKey(cast_mode_sources_, cast_mode) ?

[mark a. foltz, 2016/09/09 22:22:27]

cast_mode_sources_.find() to avoid double lookup

[takumif, 2016/09/13 03:48:21]

Done.

+      cast_mode_sources_.at(cast_mode) : std::vector<MediaSource>();
 }
 
-void QueryResultManager::UpdateWithSinksQueryResult(
+void QueryResultManager::RemoveOldSourcesForCastMode(
+    MediaCastMode cast_mode, const std::vector<MediaSource>& new_sources) {
+  if (!base::ContainsKey(cast_mode_sources_, cast_mode))
+    return;
+
+  for (const MediaSource& source : cast_mode_sources_.at(cast_mode)) {
+    if (!base::ContainsValue(new_sources, source)) {
+      sinks_observers_.erase(source);

[mark a. foltz, 2016/09/09 22:22:27]

This assumes that two different modes can't add the same source.  That should be
documented (and enforced).

[takumif, 2016/09/13 03:48:22]

Added a method AreSourcesValidForCastMode() for checking that a source doesn't
get associated with two cast modes.

+      SetSinksCompatibleWithSource(cast_mode, source, std::vector<MediaSink>());
+    }
+  }
+}
+
+void QueryResultManager::AddObserversForCastMode(
     MediaCastMode cast_mode,
-    const std::vector<MediaSink>& result) {
-  base::hash_set<MediaSink::Id> result_sink_ids;
-  for (const MediaSink& sink : result)
-    result_sink_ids.insert(sink.id());
+    const std::vector<MediaSource>& sources,
+    const GURL& origin) {
+  for (const MediaSource& source : sources) {
+    if (!base::ContainsKey(sinks_observers_, source)) {
+      std::unique_ptr<CastModeMediaSinksObserver> observer(
+          new CastModeMediaSinksObserver(
+              cast_mode, source, origin, router_, this));
+      observer->Init();
+      sinks_observers_[source] = std::move(observer);
+    }
+  }
+}
+
+void QueryResultManager::SetSinksCompatibleWithSource(
+    MediaCastMode cast_mode,
+    MediaSource source,
+    const std::vector<MediaSink>& new_sinks) {
+  base::hash_set<MediaSink::Id> new_sink_ids;
+  for (const MediaSink& sink : new_sinks)
+    new_sink_ids.insert(sink.id());
 
   // (1) Iterate through current sink set, remove cast mode from those that
   // do not appear in latest result.
   for (auto it = all_sinks_.begin(); it != all_sinks_.end(); /*no-op*/) {
-    if (!base::ContainsKey(result_sink_ids, it->first)) {
-      it->second.cast_modes.erase(cast_mode);
-    }
-    if (!IsValid(it->second)) {
+    const MediaSink& sink = it->first;
+    CastModesWithMediaSources& sources_for_sink = it->second;
+    if (!base::ContainsKey(new_sink_ids, sink.id()))
+      sources_for_sink.RemoveSource(cast_mode, source);
+    if (sources_for_sink.IsEmpty())
       all_sinks_.erase(it++);
-    } else {
+    else
       ++it;
-    }
   }
 
   // (2) Add / update sinks with latest result.
-  for (const MediaSink& sink : result) {
-    auto result =
-      all_sinks_.insert(std::make_pair(sink.id(),
-                                       MediaSinkWithCastModes(sink)));
-    if (!result.second)
-      result.first->second.sink = sink;
-    result.first->second.cast_modes.insert(cast_mode);
-  }
+  for (const MediaSink& sink : new_sinks)
+    all_sinks_[sink].AddSource(cast_mode, source);
 }
 
-CastModeSet QueryResultManager::GetSupportedCastModes() const {
-  CastModeSet modes;
-  for (const auto& observer_pair : sinks_observers_)
-    modes.insert(observer_pair.first);
-  return modes;
-}
+std::unique_ptr<MediaSource>
+QueryResultManager::GetHighestPrioritySourceForCastModeAndSink(
+    MediaCastMode cast_mode, const MediaSink& sink) const {
+  if (!base::ContainsKey(cast_mode_sources_, cast_mode))
+    return std::unique_ptr<MediaSource>();
 
-MediaSource QueryResultManager::GetSourceForCastMode(
-      MediaCastMode cast_mode) const {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  auto source_it = cast_mode_sources_.find(cast_mode);
-  return source_it == cast_mode_sources_.end() ?
-    MediaSource() : source_it->second;
+  const CastModesWithMediaSources& sources_for_sink = all_sinks_.at(sink);

[imcheng, 2016/09/12 19:26:28]

I don't think we should be using the at() methods on associative containers
since they throw exceptions if the value is not found, and exceptions are banned
in Chromium.

In this case in particular we are also doing a double lookup in |all_sinks_|.
Also, it doesn't seem necessary to break this into a separate function. I think
we should just inline this into GetSourceForCastModeAndSink() and see if we can
get rid of the double lookup.

[takumif, 2016/09/13 03:48:21]

I'd like to keep this a separate method to avoid having too many nested for/ifs.
I can avoid the double lookup by passing in CastModesWithMediaSources instead of
MediaSink to this method.

Got rid of at() to avoid double lookup, but at() should be safe as long as we
check for membership beforehand. 

+  for (const MediaSource& source : cast_mode_sources_.at(cast_mode)) {
+    if (sources_for_sink.HasSource(cast_mode, source))
+      return std::unique_ptr<MediaSource>(new MediaSource(source.id()));
+  }
+  return std::unique_ptr<MediaSource>();
 }
 
 void QueryResultManager::NotifyOnResultsUpdated() {
   std::vector<MediaSinkWithCastModes> sinks;
-  for (const auto& sink_pair : all_sinks_) {
-    sinks.push_back(sink_pair.second);
-  }
+  for (const auto& sink_pair : all_sinks_)
+    sinks.push_back(GetMediaSinkWithCastModes(sink_pair.first));
   FOR_EACH_OBSERVER(QueryResultManager::Observer, observers_,
       OnResultsUpdated(sinks));
 }
 
+MediaSinkWithCastModes QueryResultManager::GetMediaSinkWithCastModes(
+    const MediaSink& sink) {
+  MediaSinkWithCastModes sink_with_cast_modes(sink);
+  sink_with_cast_modes.cast_modes = all_sinks_[sink].GetCastModes();
+  return sink_with_cast_modes;
+}
+
 }  // namespace media_router