[Media Router] Handle route creation where resolved route is not for …

…display.

This change handles route creation where the initially resolved route is not marked for display. This keeps track of the route id and checks if the route is updated to be ready for display.

In the case where the route is never ready for display, route creation times out and fails. The MRP will send an issue to the Media Router if this happens.

Updated reporting for relevant UMA metric.

BUG=579138

Review URL: https://codereview.chromium.org/1605133002

Cr-Commit-Position: refs/heads/master@{#371385}

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

@@ -213,6 +213,17 @@ Polymer({
       value: false,
     },
 
+    /**
+     * The ID of the route that is currently being created. This is set when
+     * route creation is resolved but not ready for its controls to be
+     * displayed.
+     * @private {string}
+     */
+    pendingCreatedRouteId_: {
+      type: String,
+      value: '',
+    },
+
     /**
      * The time the sink list was shown and populated with at least one sink.
      * This is reset whenever the user switches views or there are no sinks
@@ -885,26 +896,21 @@ Polymer({
    *
    * @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.
+   * @param {string} routeId The ID of the newly created route for the sink if
+   *     succeeded; empty otherwise.
    */
-  onCreateRouteResponseReceived: function(sinkId, route) {
-    this.currentLaunchingSinkId_ = '';
-    // The provider will handle sending an issue for a failed route request.
-    if (!route)
+  onCreateRouteResponseReceived: function(sinkId, routeId) {
+    // Check that |sinkId| exists and corresponds to |currentLaunchingSinkId_|.
+    if (!this.sinkMap_[sinkId] || this.currentLaunchingSinkId_ != sinkId)
       return;
 
-    // Check that |sinkId| exists.
-    if (!this.sinkMap_[sinkId])
+    // The provider will handle sending an issue for a failed route request.
+    if (this.isEmptyOrWhitespace_(routeId)) {
+      this.resetRouteCreationProperties_(false);
       return;
+    }
 
-    // If there is an existing route associated with the same sink, its
-    // |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);
-
-    this.startTapTimer_();
+    this.pendingCreatedRouteId_ = routeId;
   },
 
   /**
@@ -926,12 +932,10 @@ Polymer({
   },
 
   /**
-   * Handles timeout of previous create route attempt. Clearing
-   * |currentLaunchingSinkId_| hides the spinner indicating there is a route
-   * creation in progress and show the device icon instead.
+   * Handles timeout of previous create route attempt.
    */
   onNotifyRouteCreationTimeout: function() {
-    this.currentLaunchingSinkId_ = '';
+    this.resetRouteCreationProperties_(false);
   },
 
   /**
@@ -964,6 +968,19 @@ Polymer({
       tempSinkToRouteMap[route.sinkId] = route;
     }, this);
 
+    // If there is route creation in progress, check if any of the route ids
+    // correspond to |pendingCreatedRouteId_|. If so, the newly created route
+    // is ready to be displayed; switch to route details view.
+    if (this.currentLaunchingSinkId_ != '' &&
+        this.pendingCreatedRouteId_ != '') {
+      var route = tempSinkToRouteMap[this.currentLaunchingSinkId_];
+      if (route && this.pendingCreatedRouteId_ == route.id) {
+        this.showRouteDetails_(route);
+        this.startTapTimer_();
+        this.resetRouteCreationProperties_(true);
+      }
+    }
+
     // If |currentRoute_| is no longer active, clear |currentRoute_|. Also
     // switch back to the SINK_PICKER view if the user is currently in the
     // ROUTE_DETAILS view.
@@ -1029,6 +1046,21 @@ Polymer({
     this.rebuildSinksToShow_();
   },
 
+  /**
+   * Resets the properties relevant to creating a new route. Fires an event
+   * indicating whether or not route creation was successful.
+   * Clearing |currentLaunchingSinkId_| hides the spinner indicating there is
+   * a route creation in progress and show the device icon instead.
+   *
+   * @private
+   */
+  resetRouteCreationProperties_: function(creationSuccess) {
+    this.currentLaunchingSinkId_ = '';
+    this.pendingCreatedRouteId_ = '';
+
+    this.fire('report-route-creation', {success: creationSuccess});
+  },
+
   /**
    * Updates the shown cast mode, and updates the header text fields
    * according to the cast mode. If |castMode| type is AUTO, then set

chrome/browser/resources/media_router/media_router.js

@@ -47,6 +47,7 @@ cr.define('media_router', function() {
     container.addEventListener('report-initial-action', onInitialAction);
     container.addEventListener('report-initial-action-close',
                                onInitialActionClose);
+    container.addEventListener('report-route-creation', onReportRouteCreation);
     container.addEventListener('report-sink-click-time',
                                onSinkClickTimeReported);
     container.addEventListener('report-sink-count', onSinkCountReported);
@@ -218,6 +219,18 @@ cr.define('media_router', function() {
         media_router.MediaRouterView.SINK_LIST);
   }
 
+  /**
+   * Reports whether or not the route creation was successful.
+   *
+   * @param {!Event} event
+   * Parameters in |event|.detail:
+   *   success - whether or not the route creation was successful.
+   */
+  function onReportRouteCreation(event) {
+    var detail = event.detail;
+    media_router.browserApi.reportRouteCreation(detail.success);
+  }
+
   /**
    * Reports the initial state of the dialog after it is opened.
    * Called after initial data is populated.

chrome/browser/resources/media_router/media_router_ui_interface.js

@@ -22,11 +22,11 @@ cr.define('media_router.ui', function() {
    *
    * @param {string} sinkId The ID of the sink to which the Media Route was
    *     creating a route.
-   * @param {?media_router.Route} route The newly create route to the sink
-   *     if route creation succeeded; null otherwise
+   * @param {string} routeId The ID of the newly created route that corresponds
+   *     to the sink if route creation succeeded; empty otherwise.
    */
-  function onCreateRouteResponseReceived(sinkId, route) {
-    container.onCreateRouteResponseReceived(sinkId, route);
+  function onCreateRouteResponseReceived(sinkId, routeId) {
+    container.onCreateRouteResponseReceived(sinkId, routeId);
   }
 
   /**
@@ -214,6 +214,15 @@ cr.define('media_router.browserApi', function() {
     chrome.send('reportNavigateToView', [view]);
   }
 
+  /**
+   * Reports whether or not a route was created successfully.
+   *
+   * @param {boolean} success
+   */
+  function reportRouteCreation(success) {
+    chrome.send('reportRouteCreation', [success]);
+  }
+
   /**
    * Reports the cast mode that the user selected.
    *
@@ -284,6 +293,7 @@ cr.define('media_router.browserApi', function() {
     reportInitialState: reportInitialState,
     reportNavigateToView: reportNavigateToView,
     reportSelectedCastMode: reportSelectedCastMode,
+    reportRouteCreation: reportRouteCreation,
     reportSinkCount: reportSinkCount,
     reportTimeToClickSink: reportTimeToClickSink,
     reportTimeToInitialActionClose: reportTimeToInitialActionClose,

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

@@ -465,7 +465,8 @@ void MediaRouterUI::OnRouteResponseReceived(const int route_request_id,
     DVLOG(0) << "MediaRouteResponse returned error: " << error;
   }
 
-  handler_->OnCreateRouteResponseReceived(sink_id, route);
+  std::string route_id = route ? route->media_route_id() : std::string();
+  handler_->OnCreateRouteResponseReceived(sink_id, route_id);
   current_route_request_id_ = -1;
   route_creation_timer_.Stop();
 }

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

@@ -41,6 +41,7 @@ const char kReportClickedSinkIndex[] = "reportClickedSinkIndex";
 const char kReportInitialAction[] = "reportInitialAction";
 const char kReportInitialState[] = "reportInitialState";
 const char kReportNavigateToView[] = "reportNavigateToView";
+const char kReportRouteCreation[] = "reportRouteCreation";
 const char kReportSelectedCastMode[] = "reportSelectedCastMode";
 const char kReportSinkCount[] = "reportSinkCount";
 const char kReportTimeToClickSink[] = "reportTimeToClickSink";
@@ -222,22 +223,11 @@ void MediaRouterWebUIMessageHandler::UpdateCastModes(
 
 void MediaRouterWebUIMessageHandler::OnCreateRouteResponseReceived(
     const MediaSink::Id& sink_id,
-    const MediaRoute* route) {
+    const MediaRoute::Id& route_id) {
   DVLOG(2) << "OnCreateRouteResponseReceived";
-  if (route) {
-    scoped_ptr<base::DictionaryValue> route_value(RouteToValue(*route, false,
-        media_router_ui_->GetRouteProviderExtensionId()));
-    web_ui()->CallJavascriptFunction(kOnCreateRouteResponseReceived,
-                                     base::StringValue(sink_id), *route_value);
-    UMA_HISTOGRAM_BOOLEAN("MediaRouter.Ui.Action.StartLocalSessionSuccessful",
-                          true);
-  } else {
-    web_ui()->CallJavascriptFunction(kOnCreateRouteResponseReceived,
-                                     base::StringValue(sink_id),
-                                     *base::Value::CreateNullValue());
-    UMA_HISTOGRAM_BOOLEAN("MediaRouter.Ui.Action.StartLocalSessionSuccessful",
-                          false);
-  }
+  web_ui()->CallJavascriptFunction(kOnCreateRouteResponseReceived,
+                                   base::StringValue(sink_id),
+                                   base::StringValue(route_id));
 }
 
 void MediaRouterWebUIMessageHandler::UpdateIssue(const Issue* issue) {
@@ -298,6 +288,10 @@ void MediaRouterWebUIMessageHandler::RegisterMessages() {
       kReportInitialAction,
       base::Bind(&MediaRouterWebUIMessageHandler::OnReportInitialAction,
                  base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
+      kReportRouteCreation,
+      base::Bind(&MediaRouterWebUIMessageHandler::OnReportRouteCreation,
+                 base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
       kReportSelectedCastMode,
       base::Bind(&MediaRouterWebUIMessageHandler::OnReportSelectedCastMode,
@@ -567,6 +561,19 @@ void MediaRouterWebUIMessageHandler::OnReportNavigateToView(
   }
 }
 
+void MediaRouterWebUIMessageHandler::OnReportRouteCreation(
+    const base::ListValue* args) {
+  DVLOG(1) << "OnReportRouteCreation";
+  bool route_created_successfully;
+  if (!args->GetBoolean(0, &route_created_successfully)) {
+    DVLOG(1) << "Unable to extract args.";
+    return;
+  }
+
+  UMA_HISTOGRAM_BOOLEAN("MediaRouter.Ui.Action.StartLocalSessionSuccessful",
+                        route_created_successfully);
+}
+
 void MediaRouterWebUIMessageHandler::OnReportSelectedCastMode(
     const base::ListValue* args) {
   DVLOG(1) << "OnReportSelectedCastMode";

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

@@ -38,7 +38,7 @@ class MediaRouterWebUIMessageHandler : public content::WebUIMessageHandler {
   void UpdateCastModes(const CastModeSet& cast_modes,
                        const std::string& source_host);
   void OnCreateRouteResponseReceived(const MediaSink::Id& sink_id,
-                                     const MediaRoute* route);
+                                     const MediaRoute::Id& route_id);
 
   // Does not take ownership of |issue|. Note that |issue| can be nullptr, when
   // there are no more issues.
@@ -68,6 +68,7 @@ class MediaRouterWebUIMessageHandler : public content::WebUIMessageHandler {
   void OnReportInitialAction(const base::ListValue* args);
   void OnReportInitialState(const base::ListValue* args);
   void OnReportNavigateToView(const base::ListValue* args);
+  void OnReportRouteCreation(const base::ListValue* args);
   void OnReportSelectedCastMode(const base::ListValue* args);
   void OnReportSinkCount(const base::ListValue* args);
   void OnReportTimeToClickSink(const base::ListValue* args);

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

@@ -174,9 +174,7 @@ TEST_F(MediaRouterWebUIMessageHandlerTest, OnCreateRouteResponseReceived) {
   MediaRoute route(route_id, MediaSource("mediaSource"), sink_id, description,
                    is_local, "", true);
 
-  EXPECT_CALL(*mock_media_router_ui_, GetRouteProviderExtensionId()).WillOnce(
-      ReturnRef(provider_extension_id()));
-  handler_->OnCreateRouteResponseReceived(sink_id, &route);
+  handler_->OnCreateRouteResponseReceived(sink_id, route.media_route_id());
   EXPECT_EQ(1u, web_ui_->call_data().size());
   const content::TestWebUI::CallData& call_data = *web_ui_->call_data()[0];
   EXPECT_EQ("media_router.ui.onCreateRouteResponseReceived",
@@ -187,20 +185,9 @@ TEST_F(MediaRouterWebUIMessageHandlerTest, OnCreateRouteResponseReceived) {
   EXPECT_EQ(sink_id, sink_id_value->GetString());
 
   const base::Value* arg2 = call_data.arg2();
-  const base::DictionaryValue* route_value = nullptr;
-  ASSERT_TRUE(arg2->GetAsDictionary(&route_value));
-
-  std::string value;
-  EXPECT_TRUE(route_value->GetString("id", &value));
-  EXPECT_EQ(route_id, value);
-  EXPECT_TRUE(route_value->GetString("sinkId", &value));
-  EXPECT_EQ(sink_id, value);
-  EXPECT_TRUE(route_value->GetString("description", &value));
-  EXPECT_EQ(description, value);
-
-  bool actual_is_local = false;
-  EXPECT_TRUE(route_value->GetBoolean("isLocal", &actual_is_local));
-  EXPECT_EQ(is_local, actual_is_local);
+  const base::StringValue* route_id_value = nullptr;
+  ASSERT_TRUE(arg2->GetAsString(&route_id_value));
+  EXPECT_EQ(route_id, route_id_value->GetString());
 }
 
 TEST_F(MediaRouterWebUIMessageHandlerTest, UpdateIssue) {