MD Settings: Hook up import data dialog.

chrome/browser/resources/settings/people_page/import_data_dialog.js

Index: chrome/browser/resources/settings/people_page/import_data_dialog.js
diff --git a/chrome/browser/resources/settings/people_page/import_data_dialog.js b/chrome/browser/resources/settings/people_page/import_data_dialog.js
index d58a5c568987f8ba53dfb45cbb2528dd3e2e0de3..3c8aae3722f58cf22b9d0202c3bb33d558f9efd0 100644
--- a/chrome/browser/resources/settings/people_page/import_data_dialog.js
+++ b/chrome/browser/resources/settings/people_page/import_data_dialog.js
@@ -18,7 +18,31 @@ Polymer({
     /** @private {!settings.BrowserProfile} */
     selected_: Object,
 
+    /**
+     * Whether none of the import data categories is selected.
+     * @private
+     */
+    noCategorySelected_: {
+      type: Boolean,
+      value: false,
+    },
+
     prefs: Object,
+
+    /** @private */
+    importStatus_: {
+      type: String,
+      value: settings.ImportDataStatus.INITIAL,
+    },
+
+    /**
+     * Mirroring the enum so that it can be used from HTML bindings.
+     * @private
+     */
+    importStatusEnum_: {
+      type: Object,
+      value: settings.ImportDataStatus,
+    },
   },
 
   /** @private {?settings.ImportDataBrowserProxy} */
@@ -27,18 +51,44 @@ Polymer({
   /** @override */
   attached: function() {
     this.browserProxy_ = settings.ImportDataBrowserProxyImpl.getInstance();
-    this.browserProxy_.initializeImportDialog().then(function(data) {
-      this.browserProfiles_ = data;
-      this.selected_ = this.browserProfiles_[0];
-    }.bind(this));
+    this.browserProxy_.initializeImportDialog().then(
+        /** @param {!Array<!settings.BrowserProfile>} data */
+        function(data) {
+          this.browserProfiles_ = data;
+          this.selected_ = this.browserProfiles_[0];
+        }.bind(this));
 
-    this.addWebUIListener('import-data-status-changed', function(e) {
-      // TODO(dpapad): Handle events to show spinner or success message.
-    });
+    this.addWebUIListener(
+        'import-data-status-changed',
+        /** @param {settings.ImportDataStatus} importStatus */
+        function(importStatus) {
+          this.importStatus_ = importStatus;
+          if (this.hasImportStatus_(settings.ImportDataStatus.FAILED))
+            this.closeDialog_();

[dpapad, 2016/11/16 00:34:38]

FYI this is what the old Options UI did on error,
https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/options/import_d...,
which should be good enough for now (addressing the dev blocker).

[tommycli, 2016/11/16 17:23:53]

Okay seems fine.

+        }.bind(this));
 
     this.$.dialog.showModal();
   },
 
+  /**
+   * Listener executing whenever the state of a checkbox changes.
+   * @private
+   */
+  onCheckboxChange_: function() {

[tommycli, 2016/11/16 17:23:53]

onImportedDataTypeChange_?

[dpapad, 2016/11/16 21:33:02]

No longer listening for iron-chage event, so this method has been removed.
Instead I am using an observer, see prefsChanged_. The reason is recorded at
crbug.com/665979.

+    var checkboxes = this.root.querySelectorAll(

[tommycli, 2016/11/16 17:23:53]

Can we make the selector operate something like this?

#importedDataTypes settings-checkbox:not([hidden])[checked]

I ask because it would be good to never accidentally select a non-datatype
checkbox.

[dpapad, 2016/11/16 21:33:02]

There are no other checkboxes on this dialog, so don't see much value in this
(extra ID, longer selector).

+        'settings-checkbox:not([hidden])[checked]');
+    this.noCategorySelected_ = checkboxes.length == 0;

[tommycli, 2016/11/16 17:23:53]

I was initially confused. Maybe we should rename this to
noDataTypesSelectedForImport_ or something.

[dpapad, 2016/11/16 21:33:02]

Done.

+  },
+
+  /**
+   * @param {!settings.ImportDataStatus} status
+   * @return {boolean} Whether |status| is the current status.
+   * @private
+   */
+  hasImportStatus_: function(status) {
+    return this.importStatus_ == status;
+  },
+
   /** @private */
   isImportFromFileSelected_: function() {
     // The last entry in |browserProfiles_| always refers to dummy profile for
@@ -58,19 +108,28 @@ Polymer({
   /** @private */
   onChange_: function() {

[tommycli, 2016/11/16 17:23:53]

Since there is now a onCheckboxChange_ method, maybe rename to
onBrowserProfileSelectionChange_?

[dpapad, 2016/11/16 21:33:02]

onCheckboxChange_ has been removed. I renamed anyway, so done.

     this.selected_ = this.browserProfiles_[this.$.browserSelect.selectedIndex];
+    this.onCheckboxChange_();
   },
 
   /** @private */
   onActionButtonTap_: function() {
-    if (this.isImportFromFileSelected_()) {
+    if (this.isImportFromFileSelected_())
       this.browserProxy_.importFromBookmarksFile();
-    } else {
-      // TODO(dpapad): Implement this.
-    }
+    else
+      this.browserProxy_.importData(this.$.browserSelect.selectedIndex);
   },
 
   /** @private */
-  onCancelTap_: function() {
+  closeDialog_: function() {
     this.$.dialog.close();
   },
+
+  /**
+   * @return {boolean} Whether the action button should be disabled.
+   * @private
+   */
+  disallowAction_: function() {

[tommycli, 2016/11/16 17:23:53]

isImportButtonDisabled_?

[dpapad, 2016/11/16 21:33:02]

Renamed to |shouldDisableImport_|. My issue with the |isImportButtonDisabled_|
naming is that it reverses cause/effect. |isImportButtonDisabled_| is used to
determine whether the button should be disabled, but just by reading the name I
am expecting to find something as follows in the contents of that method

isImportButtonDisabled_: function() { return this.importButton.disabled; }

+    return this.hasImportStatus_(settings.ImportDataStatus.IN_PROGRESS) ||
+        this.noCategorySelected_;
+  },
 });