Shows a launch spinner next to a sink once it is selected to create a route. The launch spinner sur…

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 0ae3e15187235c0be7a294917f08e92d4302045d..56cf535f784d89cb9263d9a3fed75e23c8e3131e 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
@@ -84,6 +84,15 @@ Polymer({
     },
 
     /**
+     * True if the Media Router is creating a route. False otherwise.
+     * @private {boolean}
+     */
+    isLaunching_: {
+      type: Boolean,
+      value: false,
+    },
+
+    /**
      * The issue to show.
      * @type {?media_router.Issue}
      */
@@ -167,7 +176,7 @@ Polymer({
     sinkList: {
       type: Array,
       value: [],
-      observer: 'rebuildSinkMap_',
+      observer: 'onSinkListUpdated_',
     },
 
     /**
@@ -191,7 +200,7 @@ Polymer({
 
   ready: function() {
     this.addEventListener('close-route-click', this.removeRoute);
-    this.currentView_ = this.CONTAINER_VIEW_.SINK_LIST;
+    this.showSinkList_();
   },
 
   attached: function() {
@@ -202,11 +211,21 @@ Polymer({
   },
 
   /**
-   * Adds |route| to |routeList|.
+   * Handles response of previous create route attempt.
    *
-   * @param {!media_router.Route} route The route to add.
+   * @param {string} sinkId The ID of the sink to which the Media Route was
+   *     creating a route.
+   * @param {?media_router.Route} route The newly created route to the sink
+   *     if succeeded; null otherwise.
    */
-  addRoute: function(route) {
+  onCreateRouteResponseReceived: function(sinkId, route) {
+    this.isLaunching_ = false;
+    this.updateLaunchSpinner_(sinkId, false);
+    if (!route) {
+      // TODO(apacible) Show launch failure.
+      return;
+    }
+
     // Check if |route| already exists or if its associated sink
     // does not exist.
     if (this.routeMap_[route.id] || !this.sinkMap_[route.sinkId])
@@ -216,6 +235,7 @@ Polymer({
     // |sinkToRouteMap_| entry will be overwritten with that of the new route,
     // which results in the correct sink to route mapping.
     this.routeList.push(route);
+    this.showRouteDetails_(route);
   },
 
   /**
@@ -317,7 +337,8 @@ Polymer({
    * @private
    */
   computeSinkIconClass_: function(sinkId, sinkToRouteMap) {
-    return sinkToRouteMap[sinkId] ? 'active-sink' : '';
+    return sinkToRouteMap[sinkId]?

[apacible, 2015/08/11 18:29:58]

nit: space before the ?

[haibinlu, 2015/08/12 20:32:53]

Done.

+        'sink-icon active-sink' : 'sink-icon';

[apacible, 2015/08/11 18:29:58]

Does this fit on the previous line?

[haibinlu, 2015/08/12 20:32:53]

Done.

   },
 
   /**
@@ -357,24 +378,24 @@ Polymer({
    * @private
    */
   maybeShowRouteDetailsOnOpen_: function(route) {
-    if (this.localRouteCount_ == 1 && this.justOpened_ && route) {
-      this.currentRoute_ = route;
-      this.currentView_ = this.CONTAINER_VIEW_.ROUTE_DETAILS;
-    }
+    if (this.localRouteCount_ == 1 && this.justOpened_ && route)
+      this.showRouteDetails_(route);
   },
 
   /**
-   * Creates a new route if |route| is null. Otherwise, shows the route
-   * details.
+   * Creates a new route if there is no route to the |sink| . Otherwise,
+   * shows the route details.
    *
    * @param {!media_router.Sink} sink The sink to use.
-   * @param {?media_router.Route} route The current route tied to |sink|.
    * @private
    */
-  showOrCreateRoute_: function(sink, route) {
+  showOrCreateRoute_: function(sink) {
+    var route = this.sinkToRouteMap_[sink.id];
     if (route) {
-      this.showRouteDetails_();
+      this.showRouteDetails_(route);
     } else {
+      this.isLaunching_ = true;

[Kevin M, 2015/08/11 21:28:47]

This boolean seems redundant. Can't we wait for the extension to send over a new
sinkList with the is_launching bit set correctly?

[haibinlu, 2015/08/12 20:32:53]

Done.

+      this.updateLaunchSpinner_(sink.id, true);
       this.fire('create-route', {
         sinkId: sink.id,
         selectedCastModeValue: this.selectedCastModeValue_
@@ -383,6 +404,23 @@ Polymer({
   },
 
   /**
+   * Shows or hides the launch spinner next to a sink.

[Kevin M, 2015/08/11 21:28:47]

Temporarily overrides the "isLaunching" bit for a sink.

[haibinlu, 2015/08/12 20:32:53]

Done.

+   *
+   * @param {string} sinkId The ID of the sink.
+   * @param {boolean} isLaunching Whether or not the media router is creating
+   *    a route to the sink.
+   * @private
+   */
+  updateLaunchSpinner_: function(sinkId, isLaunching) {

[Kevin M, 2015/08/11 21:28:47]

I think "set" makes more sense than "update" here, since the semantics of the
call mutate a sink's record, rather than persisting or transmitting an existing
sink.

[haibinlu, 2015/08/12 20:32:53]

Done.

+    for (var index = 0; index < this.sinkList.length; index++) {
+      if (this.sinkList[index].id == sinkId) {
+        this.set(['sinkList', index, 'isLaunching'], isLaunching);
+        return;
+      }
+    }
+  },
+
+  /**
    * Handles a cast mode selection. Updates |headerText|, |headerTextTooltip|,
    * and |selectedCastModeValue_|.
    *
@@ -406,15 +444,25 @@ Polymer({
   },
 
   /**
-   * Called when a sink is clicked. Updates |currentRoute_|.
+   * Called when a sink is clicked.
    *
    * @param {!Event} event The event object.
    * @private
    */
   onSinkClick_: function(event) {
-    var clickedSink = this.$.sinkList.itemForElement(event.target);
-    this.currentRoute_ = this.sinkToRouteMap_[clickedSink.id];
-    this.showOrCreateRoute_(clickedSink, this.currentRoute_);
+    if (this.isLaunching_)
+      return;
+    this.showOrCreateRoute_(this.$.sinkList.itemForElement(event.target));
+  },
+
+  /**
+   * Called when |sinkList| is updated.
+   *
+   * @private
+   */
+  onSinkListUpdated_: function() {
+    this.rebuildSinkMap_();
+    this.updateLaunchState_();
   },
 
   /**
@@ -471,15 +519,18 @@ Polymer({
    * @private
    */
   showCastModeList_: function() {
+    this.currentRoute_ = null;
     this.currentView_ = this.CONTAINER_VIEW_.CAST_MODE_LIST;
   },
 
   /**
    * Shows the route details.
    *
+   * @param {?media_router.Route} route The route to show.
    * @private
    */
-  showRouteDetails_: function() {
+  showRouteDetails_: function(route) {
+    this.currentRoute_ = route;
     this.currentView_ = this.CONTAINER_VIEW_.ROUTE_DETAILS;
   },
 
@@ -489,6 +540,7 @@ Polymer({
    * @private
    */
   showSinkList_: function() {
+    this.currentRoute_ = null;
     this.currentView_ = this.CONTAINER_VIEW_.SINK_LIST;
   },
 
@@ -503,4 +555,15 @@ Polymer({
     else
       this.showCastModeList_();
   },
+
+  /**
+   * Called when |sinkList| is updated. Updates |isLaunching_|.
+   *
+   * @private
+   */
+  updateLaunchState_: function() {
+    this.isLaunching_ = this.sinkList.some(function(sink) {
+      return sink.isLaunching;
+    }, this);

[apacible, 2015/08/11 18:29:58]

Do we need |this|?

[haibinlu, 2015/08/12 20:32:53]

removed

+  },
 });