MD Settings: Privacy: Show dialog when changing do-not-track

chrome/browser/resources/settings/privacy_page/privacy_page.js

Index: chrome/browser/resources/settings/privacy_page/privacy_page.js
diff --git a/chrome/browser/resources/settings/privacy_page/privacy_page.js b/chrome/browser/resources/settings/privacy_page/privacy_page.js
index 825633f929ffc159ac677f3a70cc9916a6b7c954..5990d7ef60ed666cf19789a814801266f83df8a4 100644
--- a/chrome/browser/resources/settings/privacy_page/privacy_page.js
+++ b/chrome/browser/resources/settings/privacy_page/privacy_page.js
@@ -77,6 +77,12 @@ Polymer({
     /** @private */
     showClearBrowsingDataDialog_: Boolean,
 
+    /** @private */
+    showDoNotTrackDialog_: {
+      type: Boolean,
+      value: false,
+    },
+
     /**
      * Used for HTML bindings. This is defined as a property rather than within
      * the ready callback, because the value needs to be available before
@@ -89,6 +95,10 @@ Polymer({
     },
   },
 
+  listeners: {
+    'dom-change': 'onDomChange_',
+  },
+
   ready: function() {
     this.ContentSettingsTypes = settings.ContentSettingsTypes;
 
@@ -111,6 +121,70 @@ Polymer({
         settings.getCurrentRoute() == settings.Route.CLEAR_BROWSER_DATA;
   },
 
+  /**
+   * @param {Event} event
+   * @private
+   */
+  onDomChange_: function(event) {
+    var elementId = event.path[0].id;

[Dan Beam, 2017/03/10 23:30:50]

i assume you added this variable to use once because it was easier to understand
than event.path[0].id?  (as in: I'd probably just inline it)

[stevenjb, 2017/03/10 23:58:20]

Yes.

+    if (elementId == 'doNotTrackDialogIf') {
+      if (this.showDoNotTrackDialog_)

[Dan Beam, 2017/03/10 23:30:50]

nit: combine (and maybe inline?)

if (e.path[0].id == 'doNotTrackDialogIf' && this.showDoNotTrackDialog_)

[stevenjb, 2017/03/10 23:58:20]

I also think this is more clear: First check for which dom change, then do the
logic. Otherwise if we were to add another dom change in an else, it would
continue to do more tests. (minor preference; I can change it if you feel
strongly).

+        this.maybeShowDoNotTrackDialog_();
+    }
+  },
+
+  /**
+   * Handles the change event for the do-not-track toggle. Shows a
+   * confirmation dialog when enabling the setting.
+   * @param {{target: !SettingsToggleButtonElement}} event
+   * @private
+   */
+  onDoNotTrackChange_: function(event) {
+    if (!event.target.checked) {
+      // Always allow disabling the pref.
+      event.target.sendPrefChange();
+      return;
+    }
+    this.showDoNotTrackDialog_ = true;
+    // If the dialog has already been stamped, show it. Otherwise it will be
+    // shown in onDomChange_.
+    this.maybeShowDoNotTrackDialog_();
+  },
+
+  /** @private */
+  maybeShowDoNotTrackDialog_: function() {
+    var dialog = this.$$('#confirmDoNotTrackDialog');
+    if (dialog && !dialog.open)
+      dialog.showModal();
+  },
+
+  /** @private */
+  closeDoNotTrackDialog_: function() {
+    this.$$('#confirmDoNotTrackDialog').close();
+    this.showDoNotTrackDialog_ = false;
+  },
+
+  /**
+   * Handles the shared proxy confirmation dialog 'Confirm' button.
+   * @private
+   */
+  onDoNotTrackDialogConfirm_: function() {
+    /** @type {!SettingsToggleButtonElement} */ (this.$.doNotTrack)
+        .sendPrefChange();
+    this.closeDoNotTrackDialog_();
+  },
+
+  /**
+   * Handles the shared proxy confirmation dialog 'Cancel' button or a cancel
+   * event.
+   * @private
+   */
+  onDoNotTrackDialogCancel_: function() {
+    /** @type {!SettingsToggleButtonElement} */ (this.$.doNotTrack)
+        .resetToPrefValue();
+    this.closeDoNotTrackDialog_();
+  },
+
   /** @private */
   onManageCertificatesTap_: function() {
 // <if expr="use_nss_certs">