MD Settings: Eliminate use of ES6 for Chrome OS

chrome/browser/resources/settings/device_page/display.js

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview
  * 'settings-display' is the settings subpage for display settings.
  */
 
 cr.define('settings.display', function() {
@@ skipping 135 matching lines @@
       if (selectedDisplay.modes[i].isSelected)
         return i;
     }
     return 0;
   },
 
   /** @private */
   selectedDisplayChanged_: function() {
     // Set |modeValues_| before |selectedModeIndex_| so that the slider updates
     // correctly.
-    let numModes = this.selectedDisplay.modes.length;
+    var numModes = this.selectedDisplay.modes.length;
     if (numModes == 0) {
       this.modeValues_ = [];
       this.selectedModeIndex_ = 0;
       this.currentSelectedModeIndex_ = 0;
       return;
     }
     this.modeValues_ = Array.from(Array(numModes).keys());
     this.selectedModeIndex_ = this.getSelectedModeIndex_(this.selectedDisplay);
     this.currentSelectedModeIndex_ = this.selectedModeIndex_;
   },
@@ skipping 96 matching lines @@
 
   /**
    * @return {string}
    * @private
    */
   getResolutionText_: function() {
     if (this.selectedDisplay.modes.length == 0 ||
         this.currentSelectedModeIndex_ == -1) {
       // If currentSelectedModeIndex_ == -1, selectedDisplay and
       // selectedModeIndex_ are not in sync.
-      let widthStr = this.selectedDisplay.bounds.width.toString();
-      let heightStr = this.selectedDisplay.bounds.height.toString();
-      return this.i18n('displayResolutionText', widthStr, heightStr);
+      return this.i18n(
+          'displayResolutionText', this.selectedDisplay.bounds.width.toString(),
+          this.selectedDisplay.bounds.height.toString());
     }
-    let mode = this.selectedDisplay.modes[this.selectedModeIndex_];
-    let best =
+    var mode = this.selectedDisplay.modes[this.selectedModeIndex_];
+    var best =
         this.selectedDisplay.isInternal ? mode.uiScale == 1.0 : mode.isNative;
-    let widthStr = mode.width.toString();
-    let heightStr = mode.height.toString();
+    var widthStr = mode.width.toString();

[dpapad, 2017/01/23 19:12:53]

Nit (optional): You can shorten as follows.

var messageId =
    best ? 'displayResolutionTextBest' :
    (mode.isNative ? 'displayResolutionTextNative' : 'displayResolutionText');

return this.i18n(messageId, mode.width.toString(), mode.height.toString());

[stevenjb, 2017/01/24 02:02:03]

I wanted to keep this CL as simple as possible in case we revert later. That
didn't make sense evereywhere, and we probably won't want to just revert, but
I'd still prefer to keep changes to a minimum.

+    var heightStr = mode.height.toString();
     if (best)
       return this.i18n('displayResolutionTextBest', widthStr, heightStr);
     else if (mode.isNative)
       return this.i18n('displayResolutionTextNative', widthStr, heightStr);
     return this.i18n('displayResolutionText', widthStr, heightStr);
   },
 
   /**
    * @param {!{detail: string}} e |e.detail| is the id of the selected display.
    * @private
    */
   onSelectDisplay_: function(e) {
     var id = e.detail;
-    for (let display of this.displays) {
+    for (var d = 0; d < this.displays.length; ++d) {

[dpapad, 2017/01/23 19:12:53]

Nit (optional): Same nit as before, 'i' is a more common name for a loop
counter, (then 'j', 'k').

'd' would make more sense if the variable was holding a display variable, but it
is holding just an integer, so IMO 'd' is more confusing than worth it.

[stevenjb, 2017/01/24 02:02:03]

Done.

+      var display = this.displays[d];
       if (id != display.id)
         continue;
       this.currentSelectedModeIndex_ = -1;
       this.selectedDisplay = display;

[dpapad, 2017/01/23 19:12:53]

Do we need to keep iterating if we already found the selectedDisplay? Why not
break here?

[stevenjb, 2017/01/24 02:02:03]

I don't recall for certain, would also want to fix that separately.

     }
   },
 
   /**
    * Handles event when a display tab is selected.
    * @param {!{detail: !{item: !{displayId: string}}}} e
    * @private
    */
   onSelectDisplayTab_: function(e) {
     this.onSelectDisplay_({detail: e.detail.item.displayId});
@@ skipping 50 matching lines @@
     settings.display.systemDisplayApi.setDisplayProperties(
         this.selectedDisplay.id, properties,
         this.setPropertiesCallback_.bind(this));
   },
 
   /**
    * @param {!Event} event
    * @private
    */
   onOrientationChange_: function(event) {
-    let target = /** @type {!HTMLSelectElement} */ (event.target);
+    var target = /** @type {!HTMLSelectElement} */ (event.target);
     /** @type {!chrome.system.display.DisplayProperties} */ var properties = {
       rotation: parseInt(target.value, 10)
     };
     settings.display.systemDisplayApi.setDisplayProperties(
         this.selectedDisplay.id, properties,
         this.setPropertiesCallback_.bind(this));
   },
 
   /** @private */
   onMirroredTap_: function() {
     var id = '';
     /** @type {!chrome.system.display.DisplayProperties} */ var properties = {};
     if (this.isMirrored_(this.displays)) {
       id = this.primaryDisplayId;
       properties.mirroringSourceId = '';
     } else {
       // Set the mirroringSourceId of the secondary (first non-primary) display.
-      for (var display of this.displays) {
+      for (var d = 0; d < this.displays.length; ++d) {
+        var display = this.displays[d];
         if (display.id != this.primaryDisplayId) {
           id = display.id;
           break;
         }
       }
       properties.mirroringSourceId = this.primaryDisplayId;
     }
     settings.display.systemDisplayApi.setDisplayProperties(
         id, properties, this.setPropertiesCallback_.bind(this));
   },
 
   /**
    * @param {!Event} e
    * @private
    */
   onOverscanTap_: function(e) {
     e.preventDefault();
     this.overscanDisplayId = this.selectedDisplay.id;
     this.showOverscanDialog_(true);
   },
 
   /** @private */
   updateDisplayInfo_: function() {
     var displayIds = '';
     var primaryDisplay = undefined;
     var selectedDisplay = undefined;
-    for (var display of this.displays) {
+    for (var d = 0; d < this.displays.length; ++d) {
+      var display = this.displays[d];
       if (displayIds)
         displayIds += ',';
       displayIds += display.id;

[dpapad, 2017/01/23 19:12:53]

Why not hold all the display IDs in an array first, and then just do
displayIds.join(',') at line 427? This would simplify the logic (no need to
manually keep track of when to add the comma or not).

[stevenjb, 2017/01/24 02:02:03]

Ditto.

       if (display.isPrimary && !primaryDisplay)
         primaryDisplay = display;
       if (this.selectedDisplay && display.id == this.selectedDisplay.id)
         selectedDisplay = display;
     }
     this.displayIds = displayIds;
     this.primaryDisplayId = (primaryDisplay && primaryDisplay.id) || '';
     this.selectedDisplay = selectedDisplay || primaryDisplay ||
         (this.displays && this.displays[0]);
     // Save the selected mode index received from Chrome so that we do not
     // send an unnecessary setDisplayProperties call (which would log an error).
     this.currentSelectedModeIndex_ =
         this.getSelectedModeIndex_(this.selectedDisplay);
 
     this.$.displayLayout.updateDisplays(this.displays, this.layouts);
   },
 
   /** @private */
   setPropertiesCallback_: function() {
     if (chrome.runtime.lastError) {
       console.error(
           'setDisplayProperties Error: ' + chrome.runtime.lastError.message);
     }
   },
 });