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

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

Index: chrome/browser/ui/webui/media_router/query_result_manager.h
diff --git a/chrome/browser/ui/webui/media_router/query_result_manager.h b/chrome/browser/ui/webui/media_router/query_result_manager.h
index ecffaec94c339f576f76942035983278fcca9dea..d6b5d22ac2335e7e5c3f764c9be2174de06f9f77 100644
--- a/chrome/browser/ui/webui/media_router/query_result_manager.h
+++ b/chrome/browser/ui/webui/media_router/query_result_manager.h
@@ -8,6 +8,7 @@
 #include <map>
 #include <memory>
 #include <set>
+#include <unordered_set>
 #include <vector>
 
 #include "base/gtest_prod_util.h"
@@ -16,6 +17,7 @@
 #include "chrome/browser/media/router/media_routes_observer.h"
 #include "chrome/browser/media/router/media_sink.h"
 #include "chrome/browser/media/router/media_source.h"
+#include "chrome/browser/ui/webui/media_router/cast_modes_with_media_sources.h"
 #include "chrome/browser/ui/webui/media_router/media_cast_mode.h"
 #include "chrome/browser/ui/webui/media_router/media_sink_with_cast_modes.h"
 
@@ -28,9 +30,9 @@ struct SinksQueryResult;
 
 // The Media Router dialog allows the user to initiate casting using one of
 // several actions (each represented by a cast mode).  Each cast mode is
-// associated with a media source.  This class allows the dialog to receive
-// lists of MediaSinks compatible with the cast modes available through the
-// dialog.
+// associated with a vector of media sources.  This class allows the dialog to
+// receive lists of MediaSinks compatible with the cast modes available through
+// the dialog.
 //
 // Typical use:
 //
@@ -39,18 +41,19 @@ struct SinksQueryResult;
 //   QueryResultManager result_manager(router);
 //   result_manager.AddObserver(observer);
 //   result_manager.StartSinksQuery(MediaCastMode::DEFAULT,
-//       MediaSourceForPresentationUrl("http://google.com"), origin);
+//       {MediaSourceForPresentationUrl("http://google.com")}, origin);
 //   result_manager.StartSinksQuery(MediaCastMode::TAB_MIRROR,
-//       MediaSourceForTab(123), origin);
+//       {MediaSourceForTab(123)}, origin);
 //   ...
 //   [Updates will be received by observer via OnResultsUpdated()]
 //   ...
 //   [When info on MediaSource is needed, i.e. when requesting route for a mode]
 //   CastModeSet cast_modes = result_manager.GetSupportedCastModes();
 //   [Logic to select a MediaCastMode from the set]
-//   MediaSource source = result_manager.GetSourceForCastMode(
-//       MediaCastMode::TAB_MIRROR);
-//   if (!source.Empty()) {
+//   std::unique_ptr<MediaSource> source =
+//       result_manager.GetSourceForCastModeAndSink(
+//           MediaCastMode::TAB_MIRROR, sink_of_interest);
+//   if (source) {
 //     ...
 //   }
 //
@@ -74,28 +77,33 @@ class QueryResultManager {
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
-  // Requests a list of MediaSinks compatible with |source| for |cast_mode|
-  // from |origin|.
+  // Requests a list of MediaSinks compatible with |sources| for |cast_mode|
+  // from |origin|. |sources| should be in descending order of priority.
   // Results are sent to all observers registered with AddObserver().
   //
-  // May start a new query in the Media Router for the registered source if
-  // there is no existing query for it.  If there is an existing query for
-  // |cast_mode|, it is stopped.
+  // Starts new queries in the Media Router for sources that we have no existing
+  // queries for, and stops queries for sources that no longer exist.

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

Maybe:

stops queries for sources no longer associated with any cast mode.

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

Done.

   //
-  // If |source| is empty, no new queries are begun.
+  // If |sources| is empty, no new queries are begun.
   void StartSinksQuery(MediaCastMode cast_mode,

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

IIUC, the set of sources and set of cast modes are now distinct, so this might
not start a sinks query if the sources were already associated with a different
cast mode.

It might be slightly better to rename this API
SetSourcesForCastMode()/RemoveSourcesForCastMode(), but I don't feel strongly.

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

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

Renamed the two methods.

-                       const MediaSource& source,
+                       const std::vector<MediaSource>& sources,
                        const GURL& origin);
 
-  // Stops notifying observers for |cast_mode|.
+  // Stops notifying observers for |cast_mode|, and removes it from the set of
+  // supported cast modes.
   void StopSinksQuery(MediaCastMode cast_mode);
 
   // Gets the set of cast modes that are being actively queried.
   CastModeSet GetSupportedCastModes() const;
 
-  // Returns the MediaSource registered for |cast_mode|.  Returns an empty
-  // MediaSource if there is none.
-  MediaSource GetSourceForCastMode(MediaCastMode cast_mode) const;
+  // Gets the highest-priority source for the cast mode that is supported by
+  // the sink. Returns an empty unique_ptr if there isn't any.
+  std::unique_ptr<MediaSource> GetSourceForCastModeAndSink(
+      MediaCastMode cast_mode, MediaSink::Id sink_id) const;
+
+  // Returns all the sources registered for |cast_mode|.  Returns an empty
+  // vector if there is none.
+  std::vector<MediaSource> GetSourcesForCastMode(MediaCastMode cast_mode) const;
 
  private:
   class CastModeMediaSinksObserver;
@@ -103,35 +111,48 @@ class QueryResultManager {
   FRIEND_TEST_ALL_PREFIXES(QueryResultManagerTest, Observers);
   FRIEND_TEST_ALL_PREFIXES(QueryResultManagerTest, StartRoutesDiscovery);
   FRIEND_TEST_ALL_PREFIXES(QueryResultManagerTest, MultipleQueries);
-
-  // Sets the media source for |cast_mode|.
-  void SetSourceForCastMode(MediaCastMode cast_mode, const MediaSource& source);
-
-  // Stops and destroys the MediaSinksObserver for |cast_mode|.
-  void RemoveObserverForCastMode(MediaCastMode cast_mode);
-
-  // Returns true if the |entry|'s sink is compatible with at least one cast
-  // mode.
-  bool IsValid(const MediaSinkWithCastModes& entry) const;
-
-  // Modifies the current set of results with |result| associated with
-  // |cast_mode|.
-  void UpdateWithSinksQueryResult(MediaCastMode cast_mode,
-                                  const std::vector<MediaSink>& result);
+  FRIEND_TEST_ALL_PREFIXES(QueryResultManagerTest, MultipleUrls);
+
+  // Stops and destroys the MediaSinksObservers for media sources that
+  // |cast_mode| used to support, but isn't in |new_sources|.

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

What if the sources were registered for a different cast mode?

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

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

+  void RemoveOldSourcesForCastMode(

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

RemoveObserversForCastMode sounds better to me, but I don't feel strongly.

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

Not only do we remove observers, but also disassociate old sources from sinks in
this method. Reflected that in the comment.

+      MediaCastMode cast_mode, const std::vector<MediaSource>& new_sources);
+
+  // Creates observers and starts queries for each source in |sources| that
+  // doesn't already have an associated observer.
+  void AddObserversForCastMode(MediaCastMode cast_mode,
+                               const std::vector<MediaSource>& sources,

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

|new_sources| ?

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

Not all sources are necessarily new.

+                               const GURL& origin);
+
+  // Modifies the set of sinks compatible with |cast_mode| and |source|
+  // to |new_sinks|.
+  void SetSinksCompatibleWithSource(MediaCastMode cast_mode,
+                                    const MediaSource source,

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

const MediaSource&

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

Done.

+                                    const std::vector<MediaSink>& new_sinks);
+
+  // Returns the highest-priority source for |cast_mode| supported by |sink|.
+  // Returns an empty unique_ptr if none exists.
+  std::unique_ptr<MediaSource> GetHighestPrioritySourceForCastModeAndSink(
+      MediaCastMode cast_mode, const MediaSink& sink) const;
 
   // Notifies observers that results have been updated.
   void NotifyOnResultsUpdated();
 
-  // MediaSinksObservers that listens for compatible MediaSink updates.
-  // Each observer is associated with a MediaCastMode. Results received by
+  // Creates a MediaSinkWithCastModes that contains the sink and the cast modes
+  // it supports.
+  MediaSinkWithCastModes GetMediaSinkWithCastModes(const MediaSink& sink);
+
+  // MediaSinksObservers that listen for compatible MediaSink updates.
+  // Each observer is associated with a MediaSource. Results received by
   // observers are propagated back to this class.
-  std::map<MediaCastMode, std::unique_ptr<MediaSinksObserver>> sinks_observers_;
+  std::map<MediaSource, std::unique_ptr<MediaSinksObserver>,

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

Is there any reason we need to maintain order for these sources/observers? It
looks like we added the Compare struct specifically for this purpose but I am
not sure if we actually need it.

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

Replaced with unordered_map.

+      MediaSource::Compare> sinks_observers_;
 
   // Holds registrations of MediaSources for cast modes.
-  std::map<MediaCastMode, MediaSource> cast_mode_sources_;
+  std::map<MediaCastMode, std::vector<MediaSource>> cast_mode_sources_;
 
-  // Holds all known sinks and their associated cast modes.
-  std::map<MediaSink::Id, MediaSinkWithCastModes> all_sinks_;
+  // Holds all known sinks along with the cast modes and sources they support.
+  std::map<MediaSink, CastModesWithMediaSources, MediaSink::Compare> all_sinks_;

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

Ditto on ordering/Compare struct.

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

Using map rather than unordered_map here allows us to compare the parameters to
OnResultsUpdated() with a vector in tests.

[imcheng, 2016/09/14 01:51:23]

It isn't ideal to add the Compare solely because of tests. Since we don't expect
QRM to be returning results in a specific order, maybe the test could use the
UnorderedElementsAreArray matcher instead? Though it may be a bit tricky to use
without MediaSink::operator== (which the style guideline prefers over Equals()
method). Maybe we can do that in a separate patch.

[takumif, 2016/09/14 18:49:22]

Acknowledged.

 
   // Registered observers.
   base::ObserverList<Observer> observers_;