[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..810bf3de5dcdbfbf2f7a78a9ca82ef7103e5addf 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_->UpdateWithSinksQueryResult(cast_mode_, source_, result);
     result_manager_->NotifyOnResultsUpdated();
   }
 
@@ -50,6 +53,7 @@ class QueryResultManager::CastModeMediaSinksObserver
 
  private:
   const MediaCastMode cast_mode_;

[mark a. foltz, 2016/08/23 20:46:47]

This assumes that a source is associated with one cast mode.  This likely
remains true for now as the default mode will have presentation URLs and the
desktop/tab modes will have capture sources.

+  const MediaSource source_;
   std::vector<MediaSink::Id> latest_sink_ids_;
   QueryResultManager* const result_manager_;
 };
@@ -74,106 +78,156 @@ void QueryResultManager::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
-void QueryResultManager::StartSinksQuery(MediaCastMode cast_mode,
-                                         const MediaSource& source,
-                                         const GURL& origin) {
+void QueryResultManager::StartSinksQuery(

[mark a. foltz, 2016/08/23 20:46:46]

This might be better named UpdateSourcesForCastMode?

[takumif, 2016/08/29 23:05:15]

Isn't it nicer to have StartSinksQuery() and StopSinksQuery() have matching
names?

+    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>());
+  SetSourcesForCastMode(cast_mode, sources);
+  RemoveObserversForCastMode(cast_mode);

[mark a. foltz, 2016/08/23 20:46:46]

Shouldn't this be computing the difference between the old set of sources and
the new ones and removing/creating observers accordingly?

[takumif, 2016/08/23 22:13:09]

That would be more complex, but would make sense if we want to reduce
the number of queries we make to the MRMojoImpl.

+  ResetSinkCompatibilityForCastMode(cast_mode);
 
-  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);
+  for (const MediaSource& source : sources) {
+    std::unique_ptr<CastModeMediaSinksObserver> observer(
+        new CastModeMediaSinksObserver(
+            cast_mode, source, origin, router_, this));
+    observer->Init();
+    sinks_observers_[cast_mode].push_back(std::move(observer));

[mark a. foltz, 2016/08/23 20:46:46]

It would seem more logical to have MediaSource::Id as the key for
sinks_observers_, then no need to keep a vector in the value.

[takumif, 2016/08/23 22:13:09]

Keeping them in a vector made it simpler to get rid of all the observers for a
cast mode in RemoveObserversForCastMode(), but if we'll be removing observers
source-by-source, that'd make more sense.

[mark a. foltz, 2016/08/29 23:00:29]

If that's the common case (adding/removing by cast mode) then this data
structure seems ok.

+  }
   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>());
+  RemoveObserversForCastMode(cast_mode);
+  SetSourcesForCastMode(cast_mode, std::vector<MediaSource>());
+  ResetSinkCompatibilityForCastMode(cast_mode);
   NotifyOnResultsUpdated();
 }
 
-void QueryResultManager::SetSourceForCastMode(
-      MediaCastMode cast_mode, const MediaSource& source) {
+CastModeSet QueryResultManager::GetSupportedCastModes() const {
+  CastModeSet modes;
+  for (const auto& observer_pair : sinks_observers_)
+    modes.insert(observer_pair.first);
+  return modes;
+}
+
+MediaSource QueryResultManager::GetSourceForCastModeAndSink(
+    MediaCastMode cast_mode, MediaSink::Id sink_id) const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

[mark a. foltz, 2016/08/23 20:46:46]

Please call DCHECK_CURRENTLY_ON consistently for public methods in this API.

[takumif, 2016/08/29 23:05:14]

Done.

-  cast_mode_sources_[cast_mode] = source;
+  auto source_it = cast_mode_sources_.find(cast_mode);
+  return source_it == cast_mode_sources_.end() ?
+    MediaSource() : GetFirstSourceSupportedBySink(source_it->second, sink_id);
 }
 
-void QueryResultManager::RemoveObserverForCastMode(MediaCastMode cast_mode) {
+std::vector<MediaSource> QueryResultManager::GetSourcesForCastMode(
+    MediaCastMode cast_mode) const {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  return cast_mode_sources_.find(cast_mode) == cast_mode_sources_.end() ?
+      std::vector<MediaSource>() : cast_mode_sources_.at(cast_mode);
+}
+
+void QueryResultManager::SetSourcesForCastMode(
+      MediaCastMode cast_mode, const std::vector<MediaSource>& sources) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  cast_mode_sources_[cast_mode] = sources;
+}
+
+void QueryResultManager::RemoveObserversForCastMode(MediaCastMode cast_mode) {
   auto observers_it = sinks_observers_.find(cast_mode);
   if (observers_it != sinks_observers_.end())
     sinks_observers_.erase(observers_it);
 }
 
-bool QueryResultManager::IsValid(const MediaSinkWithCastModes& entry) const {
-  return !entry.cast_modes.empty();
+void QueryResultManager::ResetSinkCompatibilityForCastMode(
+    MediaCastMode cast_mode) {
+  for (auto it = all_sinks_.begin(); it != all_sinks_.end(); /*no-op*/) {
+    it->second.erase(cast_mode);
+    if (it->second.empty())
+      all_sinks_.erase(it++);
+    else
+      ++it;
+  }
 }
 
 void QueryResultManager::UpdateWithSinksQueryResult(
     MediaCastMode cast_mode,
-    const std::vector<MediaSink>& result) {
+    MediaSource source,
+    const std::vector<MediaSink>& result_sinks) {
   base::hash_set<MediaSink::Id> result_sink_ids;
-  for (const MediaSink& sink : result)
+  for (const MediaSink& sink : result_sinks)
     result_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);
+    const MediaSink& sink = it->first;
+    if (!base::ContainsKey(result_sink_ids, sink.id())) {
+      all_sinks_[sink][cast_mode].erase(source.id());
+      if (all_sinks_[sink][cast_mode].empty())
+        all_sinks_[sink].erase(cast_mode);
     }
-    if (!IsValid(it->second)) {
+    if (all_sinks_[sink].empty())
       all_sinks_.erase(it++);
-    } else {
+    else
       ++it;
-    }
   }
 
+  if (source.Empty())
+    return;
+
   // (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 : result_sinks)
+    all_sinks_[sink][cast_mode].insert(source.id());
 }
 
-CastModeSet QueryResultManager::GetSupportedCastModes() const {
-  CastModeSet modes;
-  for (const auto& observer_pair : sinks_observers_)
-    modes.insert(observer_pair.first);
-  return modes;
+void QueryResultManager::NotifyOnResultsUpdated() {
+  std::vector<MediaSinkWithCastModes> sinks;
+  for (const auto& sink_pair : all_sinks_)
+    sinks.push_back(GetMediaSinkWithCastModes(sink_pair.first));
+  FOR_EACH_OBSERVER(QueryResultManager::Observer, observers_,
+      OnResultsUpdated(sinks));
 }
 
-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;
+MediaSource QueryResultManager::GetFirstSourceSupportedBySink(
+    std::vector<MediaSource> sources, MediaSink::Id sink_id) const {
+  for (const auto& sink_pair : all_sinks_) {
+    if (sink_pair.first.id() == sink_id) {
+      for (const MediaSource source : sources) {
+        if (SinkSupportsSource(sink_pair.first, source))
+          return source;
+      }
+      break;
+    }
+  }
+  return MediaSource();
 }
 
-void QueryResultManager::NotifyOnResultsUpdated() {
-  std::vector<MediaSinkWithCastModes> sinks;
-  for (const auto& sink_pair : all_sinks_) {
-    sinks.push_back(sink_pair.second);
+bool QueryResultManager::SinkSupportsSource(
+    MediaSink sink, MediaSource source) const {
+  if (all_sinks_.find(sink) == all_sinks_.end())
+    return false;
+
+  auto mode_sources = all_sinks_.at(sink);
+  for (const auto& mode_pair : mode_sources) {
+    if (mode_pair.second.find(source.id()) != mode_pair.second.end())
+      return true;
   }
-  FOR_EACH_OBSERVER(QueryResultManager::Observer, observers_,
-      OnResultsUpdated(sinks));
+  return false;
+}
+
+MediaSinkWithCastModes QueryResultManager::GetMediaSinkWithCastModes(
+    const MediaSink& sink) {
+  MediaSinkWithCastModes sink_with_cast_modes(sink);
+  for (const auto& mode_pair : all_sinks_[sink])
+    sink_with_cast_modes.cast_modes.insert(mode_pair.first);
+  return sink_with_cast_modes;
 }
 
 }  // namespace media_router