[Media Router] Wiring for searching route providers for new sinks.

chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js

Index: chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js
diff --git a/chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js b/chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js
index f22eb6060b7c1917e965499945928b544fe1b7b3..96dd77fac66edad51d93173df21e999b3b8bb30f 100644
--- a/chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js
+++ b/chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js
@@ -298,6 +298,25 @@ Polymer({
     },
 
     /**
+     * Pseudo sinks from MRPs that represent their ability to accept sink search
+     * requests.
+     * @private {!Array<!media_router.Sink>}
+     */
+    pseudoSinks_: {
+      type: Array,
+      value: [],
+    },
+
+    /**
+     * Helps manage the state of creating a sink and a route from a pseudo sink.
+     * @private {PseudoSinkSearchState}

[imcheng, 2016/04/18 23:15:47]

should this be ?PseudoSinkSearchState ?

[btolsch, 2016/04/19 01:39:43]

As a proper object, using its name alone implies '?'.

+     */
+    pseudoSinkSearchState_: {
+      type: Object,
+      value: null,
+    },
+
+    /**
      * Whether the next character input should cause a filter action metric to
      * be sent.
      * @type {boolean}
@@ -806,6 +825,7 @@ Polymer({
    */
   computeIssueBannerShown_: function(view, issue) {
     return !!issue && (view == media_router.MediaRouterView.SINK_LIST ||
+                       view == media_router.MediaRouterView.FILTER ||
                        view == media_router.MediaRouterView.ISSUE);
   },
 
@@ -1135,6 +1155,25 @@ Polymer({
                                 substrings: matchSubstrings});
     }
     searchResultsToShow.sort(this.compareSearchMatches_);
+
+    var pendingPseudoSink = (this.pseudoSinkSearchState_) ?
+        this.pseudoSinkSearchState_.getPseudoSink() :
+        null;
+    // Now we need to put any pseudo sinks that should not be overridden by real
+    // sinks at the top of the filter results. First we find the pseudo sinks
+    // that should not be hidden, then we convert them to the match object
+    // format, and then we append the rest of the search matches.
+    searchResultsToShow = this.pseudoSinks_.filter(function(pseudoSink) {
+      return (!pendingPseudoSink || pseudoSink.id != pendingPseudoSink.id) &&
+          !searchResultsToShow.find(function(searchResult) {
+            return searchResult.sinkItem.name == searchInputText &&
+                   searchResult.sinkItem.iconType == pseudoSink.iconType;
+          });
+    }).map(function(pseudoSink) {
+      pseudoSink.name = searchInputText;
+      return {sinkItem: pseudoSink,
+              substrings: [[0, searchInputText.length - 1]]};
+    }).concat(searchResultsToShow);
     this.searchResultsToShow_ = searchResultsToShow;
   },
 
@@ -1329,6 +1368,10 @@ Polymer({
       return;
     }
 
+    if (this.pseudoSinkSearchState_) {
+      sinkId = this.pseudoSinkSearchState_.mapRouteSinkId(sinkId);
+    }
+
     // Check that |sinkId| exists and corresponds to |currentLaunchingSinkId_|.
     if (!this.sinkMap_[sinkId] || this.currentLaunchingSinkId_ != sinkId) {
       this.fire('report-resolved-route', {
@@ -1421,6 +1464,29 @@ Polymer({
   },
 
   /**
+   * Called when a search has completed up to route creation. |sinkId|
+   * identifies the sink that should be in |allSinks|, if a sink was found.
+   *
+   * @param {string} sinkId The ID of the sink that is the result of the
+   *     currently pending search.
+   */
+  onReceiveSearchResult: function(sinkId) {
+    // TODO(btolsch): Consider showing the pseudo sink in the sink list, as well
+    // as freezing its name, when it is launching in case there's ever a delay
+    // in reaching this point.
+    this.pseudoSinkSearchState_.receiveSinkResponse(sinkId);

[imcheng, 2016/04/18 23:15:47]

can receiveSinkResponse and checkForRealSink be made into one call? In the one
place where they are not called together (L1514), the pseudo sink id is always
used.

[btolsch, 2016/04/19 01:39:43]

I don't think I understand this comment. I assume you're referring to L1620
(L1547 in patchset 7) and not L1514 since that has nothing to do with
checkForRealSink(). But at L1620, I'm not sure what you mean by "the pseudo sink
id is always used." Could you explain?

It would be possible to make them one function if L1620 uses |null| to avoid
resetting an existing sink id, though I think that's slightly less readable.

[imcheng, 2016/04/19 19:52:36]

I meant L1514 in this patchset, where we are calling getCurrentLaunchingSinkId()
on a new PseudoSinkSearchState, which always return clickedSink.id. With the
exception of L1514, invocations of getCurrentLaunchingSinkId() is always
preceded with checkForRealSink(), so I asked if it's possible to merge the two
calls.

[btolsch, 2016/04/20 04:20:23]

That makes sense, and I did merge them since you also asked that in another
comment. But here you mentioned receiveSinkResponse() and checkForRealSink()
which was confusing.

+    this.pseudoSinkSearchState_.checkForRealSink(this.allSinks);
+    this.currentLaunchingSinkId_ =
+        this.pseudoSinkSearchState_.getCurrentLaunchingSinkId();
+    this.rebuildSinksToShow_();
+    // If we're in filter view, make sure the |sinksToShow_| change is picked
+    // up.
+    if (this.isUserSearching_) {
+      this.filterSinks_(this.searchInputText_);
+    }
+  },
+
+  /**
    * Called when a sink is clicked.
    *
    * @param {!Event} event The event object.
@@ -1430,7 +1496,27 @@ Polymer({
     var clickedSink = (this.isUserSearching_) ?
         this.$$('#searchResults').itemForElement(event.target).sinkItem :
         this.$.sinkList.itemForElement(event.target);
-    this.showOrCreateRoute_(clickedSink);
+    if (clickedSink.isPseudoSink) {
+      if (this.currentLaunchingSinkId_ == '') {
+        this.pseudoSinkSearchState_ = new PseudoSinkSearchState(clickedSink);
+        this.fire('search-sinks-and-create-route', {

[imcheng, 2016/04/18 23:15:47]

could you please refactor the logic here with the create route logic in
showOrCreateRoute_?

[btolsch, 2016/04/19 01:39:43]

Done.

+          id: clickedSink.id,
+          name: clickedSink.name,
+          domain: clickedSink.domain,
+          selectedCastMode:
+              this.shownCastModeValue_ == media_router.CastModeType.AUTO ?
+                  clickedSink.castModes & -clickedSink.castModes :
+                  this.shownCastModeValue_
+        });
+        this.maybeReportUserFirstAction(
+            media_router.MediaRouterUserAction.START_LOCAL);
+        this.currentLaunchingSinkId_ =
+            this.pseudoSinkSearchState_.getCurrentLaunchingSinkId();
+        this.rebuildSinksToShow_();
+      }
+    } else {
+      this.showOrCreateRoute_(clickedSink);
+    }
     this.fire('sink-click', {index: event['model'].index});
   },
 
@@ -1488,11 +1574,19 @@ Polymer({
    * name.
    */
   rebuildSinksToShow_: function() {
-    var sinksToShow = [];
+    var sinksToShow = this.allSinks.filter(function(sink) {
+      return !sink.isPseudoSink;
+    }, this);
+    if (this.pseudoSinkSearchState_) {
+      var pendingPseudoSink = this.pseudoSinkSearchState_.getPseudoSink();
+      if (pendingPseudoSink.id == this.currentLaunchingSinkId_) {
+        sinksToShow.unshift(pendingPseudoSink);
+      }
+    }
     if (this.userHasSelectedCastMode_) {
       // If user explicitly selected a cast mode, then we show only sinks that
       // are compatible with current cast mode or sinks that are active.
-      sinksToShow = this.allSinks.filter(function(element) {
+      sinksToShow = sinksToShow.filter(function(element) {
         return (element.castModes & this.shownCastModeValue_) ||
                this.sinkToRouteMap_[element.id];
       }, this);
@@ -1503,7 +1597,6 @@ Polymer({
       // - Otherwise, the cast mode becomes auto mode.
       // Either way, all sinks will be shown.
       this.setShownCastMode_(this.computeCastMode_());
-      sinksToShow = this.allSinks;
     }
 
     this.sinksToShow_ = sinksToShow;
@@ -1518,9 +1611,18 @@ Polymer({
     this.sinkMap_ = {};
 
     this.allSinks.forEach(function(sink) {
-      this.sinkMap_[sink.id] = sink;
+      if (!sink.isPseudoSink) {
+        this.sinkMap_[sink.id] = sink;
+      }
     }, this);
 
+    if (this.pseudoSinkSearchState_) {
+      this.pseudoSinkSearchState_.checkForRealSink(this.allSinks);

[imcheng, 2016/04/18 23:15:46]

Can checkForRealSink and getCurrentLaunchingSinkId be combined into one call?

[btolsch, 2016/04/19 01:39:43]

If the call at L1513 is removed and instead sets the launching sink id manually
to the pseudo sink id. I merged them for now so you can tell me what you think
about that line in the showOrCreateRoute_() refactor.

+      this.currentLaunchingSinkId_ =
+          this.pseudoSinkSearchState_.getCurrentLaunchingSinkId();
+    }
+    this.pseudoSinks_ =
+        this.allSinks.filter(function(sink) { return sink.isPseudoSink; });
     this.rebuildSinksToShow_();
     if (this.isUserSearching_) {
       this.filterSinks_(this.searchInputText_);
@@ -1536,6 +1638,7 @@ Polymer({
    * @private
    */
   resetRouteCreationProperties_: function(creationSuccess) {
+    this.pseudoSinkSearchState_ = null;
     this.currentLaunchingSinkId_ = '';
     this.pendingCreatedRouteId_ = '';