[Media Router UI] Simplify rendering of route descriptions in the MR dialog.

We want to consolidate how route descriptions passed to the UX by using
MediaRoute.description instead of MediaStatus.description.  This adds comments
explaining what the route description should be used for, and notes that
MediaStatus.description will be deprecated once MRPs are updated to use
MediaRoute.description instead.

It also removes the "Casting" prefix which was sometimes inserted before
MediaRoute.description, as we want to customize the description to say
"Casting," "Remoting," or "Mirroring."

The template string remains so we can use it in a future change
for proper localization of the description.  This change does not make the UX
any worse than it is right now (and will be followed soon by an MRP changes to
add "Casting" back where it makes sense).

The Media Router elements are refactored to use "description" consistently and
to better share CSS, which makes rendering more consistent between
MediaStatus.description and MediaRoute.description.

TBR=estark@chromium.org for comment-only .mojom changes

Bug: 786208
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I9356fa917f0d77eeac5b7b32d6175aa03a943e5c
Reviewed-on: https://chromium-review.googlesource.com/776130
Commit-Queue: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517656}

chrome/browser/resources/media_router/elements/route_controls/route_controls.css

@@ -16,13 +16,6 @@
   right: 20px;
 }
 
-.ellipsis {
-  padding: 0 1%;
-  text-overflow: ellipsis;
-  white-space: nowrap;
-  width: 90%;
-}
-
 :host-context([dir='rtl']) #play-pause-volume-hangouts-controls {
   transform: scaleX(-1);
 }
@@ -48,7 +41,7 @@
 
 #route-description {
   margin: 15px 8px 3px 8px;
-  overflow: hidden;
+  width: 90%;
 }
 
 #route-time-controls {

chrome/browser/resources/media_router/elements/route_controls/route_controls.html

@@ -8,9 +8,14 @@
   <link rel="import" type="css" href="route_controls.css">
   <template>
     <div id="media-controls">
+      <!--
+        TODO(crbug.com/786208): Remove the div below and always render the
+        description in the details element.  And, possibly combine details and
+        controls elements.
+      -->
       <div class="ellipsis" id="route-description"
-           title="[[displayedDescription_]]">
-        [[displayedDescription_]]
+           title="[[routeDescription_]]">
+        [[routeDescription_]]
       </div>
       <div class="ellipsis" id="route-title" title="[[routeStatus.title]]">
         [[routeStatus.title]]

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

@@ -21,11 +21,11 @@ Polymer({
     },
 
     /**
-     * The media description to display. Uses route description if none is
-     * provided by the route status object.
+     * The route description to display. Uses the media route description if
+     * none is provided by the media route status object.
      * @private {string}
      */
-    displayedDescription_: {
+    routeDescription_: {
       type: String,
       value: '',
     },
@@ -357,7 +357,7 @@ Polymer({
       this.displayedVolume_ = Math.round(newRouteStatus.volume * 100) / 100;
     }
     if (newRouteStatus.description !== '') {
-      this.displayedDescription_ = newRouteStatus.description;
+      this.routeDescription_ = newRouteStatus.description;
     }
     if (!this.initialLoadTime_) {
       this.initialLoadTime_ = Date.now();
@@ -386,8 +386,7 @@ Polymer({
       this.stopIncrementingCurrentTime_();
     }
     if (route && this.routeStatus.description === '') {
-      this.displayedDescription_ =
-          loadTimeData.getStringF('castingActivityStatus', route.description);
+      this.routeDescription_ = route.description;
     }
   },
 

chrome/browser/resources/media_router/elements/route_details/route_details.css

@@ -17,12 +17,10 @@
   text-align: end;
 }
 
-#route-information {
+#route-description {
   -webkit-padding-end: var(--dialog-padding-end);
   -webkit-padding-start: 44px;
-  background-color: white;
   font-size: 1.2em;
   line-height: 1.5em;
   margin-top: 16px;
-  overflow: hidden;
 }

chrome/browser/resources/media_router/elements/route_details/route_details.html

@@ -7,9 +7,9 @@
   <link rel="import" type="css" href="../../media_router_common.css">
   <link rel="import" type="css" href="route_details.css">
   <template>
-    <div id="route-information"
-        hidden$="[[!shouldShowRouteInfoOnly_(controllerType_)]]">
-      <span>[[activityStatus_]]</span>
+    <div class="ellipsis" id="route-description" title="[[routeDescription_]]"
+        hidden$="[[!shouldShowRouteDescription_(controllerType_)]]">
+      [[routeDescription_]]
     </div>
     <template is="dom-if" if="[[shouldAttemptLoadingExtensionView_(route)]]">
       <extension-view-wrapper id="extension-view-wrapper" route="[[route]]"

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

@@ -9,10 +9,10 @@ Polymer({
 
   properties: {
     /**
-     * The text for the current casting activity status.
+     * Description of the current casting activity, e.g. "Casting YouTube".
      * @private {string|undefined}
      */
-    activityStatus_: {
+    routeDescription_: {
       type: String,
     },
 
@@ -191,15 +191,12 @@ Polymer({
   },
 
   /**
-   * Updates |activityStatus_| for the default view.
+   * Updates |routeDescription_| for the default view.
    *
    * @private
    */
-  updateActivityStatus_: function() {
-    this.activityStatus_ = this.route ?
-        loadTimeData.getStringF(
-            'castingActivityStatus', this.route.description) :
-        '';
+  updateRouteDescription_: function() {
+    this.routeDescription_ = this.route ? this.route.description : '';
   },
 
   /**
@@ -231,7 +228,7 @@ Polymer({
    */
   onRouteChange_: function(newRoute) {
     if (this.controllerType_ !== media_router.ControllerType.WEBUI) {
-      this.updateActivityStatus_();
+      this.updateRouteDescription_();
     }
   },
 
@@ -260,7 +257,7 @@ Polymer({
    *     the extensionview or the WebUI route controller.
    * @private
    */
-  shouldShowRouteInfoOnly_: function(controllerType) {
+  shouldShowRouteDescription_: function(controllerType) {
     return controllerType === media_router.ControllerType.NONE;
   },
 

chrome/browser/resources/media_router/media_router_common.css

@@ -19,3 +19,14 @@
 [hidden] {
   display: none !important;
 }
+
+.ellipsis {
+  padding: 0 1%;
+  text-overflow: ellipsis;
+  white-space: nowrap;
+}
+
+#route-description {
+  background-color: white;
+  overflow: hidden;
+}

chrome/common/media_router/media_route.h

@@ -32,7 +32,7 @@ class MediaRoute {
   // |media_route_id|: ID of the route.
   // |media_source|: Description of source of the route.
   // |media_sink|: The sink that is receiving the media.
-  // |description|: Description of the route to be displayed.
+  // |description|: Human readable description of the casting activity.
   // |is_local|: true if the route was created from this browser.
   // |custom_controller_path|: custom controller path if it is given by route
   //     provider. empty otherwise.
@@ -126,8 +126,8 @@ class MediaRoute {
   // The ID of sink being routed to.
   MediaSink::Id media_sink_id_;
 
-  // The description of the media route activity, for example
-  // "Playing Foo Bar Music All Access."
+  // Human readable description of the casting activity.  Examples:
+  // "Mirroring tab (www.example.com)", "Casting media", "Casting YouTube"
   std::string description_;
 
   // |true| if the route is created locally (versus discovered by a media route

chrome/common/media_router/media_status.h

@@ -36,6 +36,8 @@ struct MediaStatus {
 
   // Text describing the media, or a secondary title. For example, in a
   // MediaStatus representing a YouTube Cast session, this could be "YouTube".
+  //
+  // DEPRECATED.  TODO(crbug.com/786208): Remove this when no longer used.
   std::string description;
 
   // If this is true, the media can be played and paused.

chrome/common/media_router/mojo/media_router.mojom

@@ -30,6 +30,11 @@ struct MediaSink {
   // The human-readable name, e.g. "Janet's Chromecast".
   string name;
   // Optional description of the sink.
+  //
+  // DEPRECATED.  This is currently used in the Media Router UX to relay a
+  // Hangouts meeting name.  It should not be used for other purposes.
+  // TODO(crbug.com/786208): Remove this at a future date when Hangouts names
+  // are no longer required.
   string? description;
   // Optional domain of the sink if this sink is associated with an identity.
   string? domain;
@@ -93,7 +98,11 @@ struct MediaRoute {
   string? media_source;
   // The ID of sink that is rendering the media content.
   string media_sink_id;
-  // Human readable description of this route, e.g. "Tab casting".
+  // Human readable description of the casting activity.  Examples:
+  // "Mirroring tab (www.example.com)", "Casting media", "Casting YouTube"
+  //
+  // TODO(crbug.com/786208): Localize this properly by passing it in a way that
+  // permits use of template strings and placeholders.
   string description;
   // Specifies that the route is requested locally.
   bool is_local;

chrome/common/media_router/mojo/media_status.mojom

@@ -16,12 +16,19 @@ struct MediaStatus {
     BUFFERING
   };
 
-  // The main title of the media. For example, in a MediaStatus representing
-  // a YouTube Cast session, this could be the title of the video.
+  // The title of the currently playing media. For example, in a MediaStatus
+  // representing a YouTube Cast route, this is the title of the video.
+  // For a tab mirroring or media remoting route, it's the page title.
+  // For a Presentation API route, it is the presentation page title.
   string title;
 
   // Text describing the media, or a secondary title. For example, in a
   // MediaStatus representing a YouTube Cast session, this could be "YouTube".
+  //
+  // DEPRECATED.  This is used to override MediaRoute.description (defined in
+  // media_router.mojom) in the Media Router UX.  We will be using
+  // MediaRoute.description exclusively once all MRPs have been updated.
+  // TODO(crbug.com/786208): Remove this when that is done.
   string description;
 
   // If this is true, the media can be played and paused through its

chrome/test/data/webui/media_router/route_controls_tests.js

@@ -82,17 +82,11 @@ cr.define('route_controls', function() {
       test('initial text setting', function() {
         // Set |route|.
         controls.onRouteUpdated_(fakeRouteOne);
-        assertElementText(
-            loadTimeData.getStringF(
-                'castingActivityStatus', fakeRouteOne.description),
-            'route-description');
+        assertElementText(fakeRouteOne.description, 'route-description');
 
         // Set |route| to a different route.
         controls.onRouteUpdated_(fakeRouteTwo);
-        assertElementText(
-            loadTimeData.getStringF(
-                'castingActivityStatus', fakeRouteTwo.description),
-            'route-description');
+        assertElementText(fakeRouteTwo.description, 'route-description');
       });
 
       // Tests that the route title and status are shown when RouteStatus is

chrome/test/data/webui/media_router/route_details_tests.js

@@ -45,9 +45,17 @@ cr.define('route_details', function() {
             details.$$('#' + elementId).querySelector('span').innerText);
       };
 
+      // Checks whether |expected| and the text in the element in the
+      // |elementId| element are equal.
+      var checkElementText = function(expected, elementId) {
+        assertEquals(
+            expected,
+            details.$$('#' + elementId).innerText);
+      };
+
       // Checks the default route view is shown.
       var checkDefaultViewIsShown = function() {
-        assertFalse(details.$$('#route-information').hasAttribute('hidden'));
+        assertFalse(details.$$('#route-description').hasAttribute('hidden'));
         assertTrue(
             !details.$$('extension-view-wrapper') ||
             details.$$('extension-view-wrapper').hasAttribute('hidden'));
@@ -67,7 +75,7 @@ cr.define('route_details', function() {
 
       // Checks the custom controller is shown.
       var checkCustomControllerIsShown = function() {
-        assertTrue(details.$$('#route-information').hasAttribute('hidden'));
+        assertTrue(details.$$('#route-description').hasAttribute('hidden'));
         assertFalse(
             details.$$('extension-view-wrapper').hasAttribute('hidden'));
       };
@@ -182,7 +190,7 @@ cr.define('route_details', function() {
         checkSpanText(
             loadTimeData.getString('startCastingButtonText').toUpperCase(),
             'start-casting-to-route-button');
-        checkSpanText('', 'route-information');
+        checkElementText('', 'route-description');
       });
 
       // Tests when |route| is undefined or set.
@@ -194,16 +202,14 @@ cr.define('route_details', function() {
         // Set |route|.
         details.route = fakeRouteOne;
         assertEquals(fakeRouteOne, details.route);
-        checkSpanText(loadTimeData.getStringF('castingActivityStatus',
-            fakeRouteOne.description), 'route-information');
+        checkElementText(fakeRouteOne.description, 'route-description');
         checkDefaultViewIsShown();
         checkStartCastButtonIsNotShown();
 
         // Set |route| to a different route.
         details.route = fakeRouteTwo;
         assertEquals(fakeRouteTwo, details.route);
-        checkSpanText(loadTimeData.getStringF('castingActivityStatus',
-            fakeRouteTwo.description), 'route-information');
+        checkElementText(fakeRouteTwo.description, 'route-description');
         checkDefaultViewIsShown();
         checkStartCastButtonIsShown();
       });