MD Settings: Internet: Cleanup JS

chrome/browser/resources/settings/internet_page/network_apnlist.js

Index: chrome/browser/resources/settings/internet_page/network_apnlist.js
diff --git a/chrome/browser/resources/settings/internet_page/network_apnlist.js b/chrome/browser/resources/settings/internet_page/network_apnlist.js
index cd7508c0c5a20933df352b310048baec7f5f58de..1092da1c08950709e041629f117099ea627a8c07 100644
--- a/chrome/browser/resources/settings/internet_page/network_apnlist.js
+++ b/chrome/browser/resources/settings/internet_page/network_apnlist.js
@@ -19,10 +19,8 @@ Polymer({
       observer: 'networkPropertiesChanged_',
     },
 
-    /**
-     * The CrOnc.APNProperties.AccessPointName value of the selected APN.
-     */
-    selectedApn: {
+    /** The CrOnc.APNProperties.AccessPointName value of the selected APN. */
+    selectedApn_: {
       type: String,
       value: '',
     },
@@ -32,7 +30,7 @@ Polymer({
      * corresponding to |otherApn| (see below).
      * @type {!Array<!CrOnc.APNProperties>}
      */
-    apnSelectList: {
+    apnSelectList_: {
       type: Array,
       value: function() {
         return [];
@@ -45,7 +43,7 @@ Polymer({
      * active APN if it does not match an existing list entry.
      * @type {CrOnc.APNProperties|undefined}
      */
-    otherApn: {
+    otherApn_: {
       type: Object,
     },
 
@@ -102,7 +100,7 @@ Polymer({
 
   /**
    * Sets the list of selectable APNs for the UI. Appends an 'Other' entry
-   * (see comments for |otherApn| above).
+   * (see comments for |otherApn_| above).
    * @param {CrOnc.APNProperties|undefined} activeApn The currently active APN
    *     properties.
    * @private
@@ -119,7 +117,7 @@ Polymer({
     // If |activeApn| is specified and not in the list, use the active
     // properties for 'other'. Otherwise use any existing 'other' properties.
     var otherApnProperties =
-        (activeApn && !activeApnInList) ? activeApn : this.otherApn;
+        (activeApn && !activeApnInList) ? activeApn : this.otherApn_;
     var otherApn = this.createApnObject_(otherApnProperties);
 
     // Always use 'Other' for the name of custom APN entries (the name does
@@ -131,13 +129,13 @@ Polymer({
         otherApn.AccessPointName || this.DefaultAccessPointName;
 
     // Save the 'other' properties.
-    this.otherApn = otherApn;
+    this.otherApn_ = otherApn;
 
     // Append 'other' to the end of the list of APNs.
     result.push(otherApn);
 
-    this.apnSelectList = result;
-    this.selectedApn =
+    this.apnSelectList_ = result;
+    this.selectedApn_ =
         (activeApn && activeApn.AccessPointName) || otherApn.AccessPointName;
     // We need to flush the DOM here, otherwise the paper-dropdown-menu-light
     // will not update to correctly display the selected AccessPointName.
@@ -175,16 +173,16 @@ Polymer({
 
   /**
    * Event triggered when the selectApn selection changes.
-   * @param {!{detail: !{selected: string}}} e
+   * @param {!{detail: !{selected: string}}} event

[Dan Beam, 2016/08/31 21:59:47]

nit: CustomEvent (and then cast detail to {selected: string})

    * @private
    */
-  onSelectApnChange_: function(e) {
-    var selectedApn = e.detail.selected;
+  onSelectApnChange_: function(event) {
+    var selectedApn = event.detail.selected;
     // When selecting 'Other', don't set a change event unless a valid
     // non-default value has been set for Other.
-    if (this.isOtherSelected_(this.networkProperties, selectedApn) &&
-        (!this.otherApn || !this.otherApn.AccessPointName ||
-         this.otherApn.AccessPointName == this.DefaultAccessPointName)) {
+    if (this.isOtherSelected_(selectedApn) &&
+        (!this.otherApn_ || !this.otherApn_.AccessPointName ||
+         this.otherApn_.AccessPointName == this.DefaultAccessPointName)) {
       return;
     }
     this.sendApnChange_(selectedApn);
@@ -206,7 +204,7 @@ Polymer({
    * @private
    */
   onSaveOtherTap_: function(event) {
-    this.sendApnChange_(this.selectedApn);
+    this.sendApnChange_(this.selectedApn_);
   },
 
   /**
@@ -219,23 +217,22 @@ Polymer({
     var apn = this.findApnInList(apnList, selectedApn);
     if (apn == undefined) {
       apn = this.createApnObject_();
-      if (this.otherApn) {
-        apn.AccessPointName = this.otherApn.AccessPointName;
-        apn.Username = this.otherApn.Username;
-        apn.Password = this.otherApn.Password;
+      if (this.otherApn_) {
+        apn.AccessPointName = this.otherApn_.AccessPointName;
+        apn.Username = this.otherApn_.Username;
+        apn.Password = this.otherApn_.Password;
       }
     }
     this.fire('apn-change', {field: 'APN', value: apn});
   },
 
   /**
-   * @param {!CrOnc.NetworkProperties|undefined} networkProperties
    * @param {string} selectedApn

[Dan Beam, 2016/08/31 21:59:47]

are you mixing this.* with @params?

[stevenjb, 2016/08/31 22:10:16]

I'm not sure I understand your question.

I am avoiding including properties as parameters (using this.foo instead). If a
property is a dependency and a non-property parameter is needed, I append the
properties at the end in the HTML. Does that seem reasonable?

    * @return {boolean} True if the 'other' APN is currently selected.
    * @private
    */
-  isOtherSelected_: function(networkProperties, selectedApn) {
-    if (!networkProperties || !networkProperties.Cellular)
+  isOtherSelected_: function(selectedApn) {
+    if (!this.networkProperties || !this.networkProperties.Cellular)
       return false;
     var apnList = this.getApnList_();
     var apn = this.findApnInList(apnList, selectedApn);