MD Settings: Certificate manager, move "Import" button to the tab level.

chrome/test/data/webui/settings/certificate_manager_page_test.js

Index: chrome/test/data/webui/settings/certificate_manager_page_test.js
diff --git a/chrome/test/data/webui/settings/certificate_manager_page_test.js b/chrome/test/data/webui/settings/certificate_manager_page_test.js
index bdb71b38d220bf889a491ff9d1cb35623f5ca50b..407a6627dfdcfb4c4ffa1e04010c07d01f09e63e 100644
--- a/chrome/test/data/webui/settings/certificate_manager_page_test.js
+++ b/chrome/test/data/webui/settings/certificate_manager_page_test.js
@@ -20,7 +20,10 @@ cr.define('certificate_manager_page', function() {
       'exportPersonalCertificate',
       'exportPersonalCertificatePasswordSelected',
       'getCaCertificateTrust',
+      'importCaCertificate',
+      'importPersonalCertificate',
       'importPersonalCertificatePasswordSelected',
+      'importServerCertificate',
       'refreshCertificates',
       'viewCertificate',
     ]);
@@ -49,6 +52,18 @@ cr.define('certificate_manager_page', function() {
     },
 
     /** @override */
+    importServerCertificate: function() {
+      this.methodCalled('importServerCertificate');
+      return Promise.resolve();
+    },
+
+    /** @override */
+    importCaCertificate: function() {
+      this.methodCalled('importCaCertificate');
+      return Promise.resolve('dummyName');
+    },
+
+    /** @override */
     editCaCertificateTrust: function(id, ssl, email, objSign) {
       this.methodCalled('editCaCertificateTrust', {
         id: id, ssl: ssl, email: email, objSign: objSign,
@@ -89,6 +104,12 @@ cr.define('certificate_manager_page', function() {
     },
 
     /** @override */
+    importPersonalCertificate: function() {
+      this.methodCalled('importPersonalCertificate');
+      return Promise.resolve(true);
+    },
+
+    /** @override */
     importPersonalCertificatePasswordSelected: function(password) {
       this.resolverMap_.get(
           'importPersonalCertificatePasswordSelected').resolve(password);
@@ -554,7 +575,7 @@ cr.define('certificate_manager_page', function() {
   }
 
   function registerPageTests() {
-    /** @type {?SettingsCertificateManagerPage} */
+    /** @type {?SettingsCertificateManagerPageElement} */
     var page = null;
 
     /** @type {?TestCertificatesBrowserProxy} */
@@ -635,13 +656,13 @@ cr.define('certificate_manager_page', function() {
        *     simulate.
        */
       function dispatchCertificateActionEvent(action) {
-        var eventDetail = /** @type {!CertificateActionEventDetail} */ ({
+        page.fire(
+            settings.CertificateActionEvent,
+            /** @type {!CertificateActionEventDetail} */ ({
           action: action,
           subnode: createSampleCertificateSubnode(),
           certificateType: settings.CertificateType.PERSONAL
-        });
-        page.dispatchEvent(new CustomEvent(
-            settings.CertificateActionEvent, {detail: eventDetail}));
+        }));
       }
 
       /**
@@ -660,25 +681,98 @@ cr.define('certificate_manager_page', function() {
 
       test('OpensDialog_DeleteConfirmation', function() {
         testDialogOpensOnAction(
-              'settings-certificate-delete-confirmation-dialog',
-              settings.CertificateAction.DELETE);
+            'settings-certificate-delete-confirmation-dialog',
+            settings.CertificateAction.DELETE);
       });
 
       test('OpensDialog_PasswordEncryption', function() {
         testDialogOpensOnAction(
-              'settings-certificate-password-encryption-dialog',
-              settings.CertificateAction.EXPORT_PERSONAL);
+            'settings-certificate-password-encryption-dialog',
+            settings.CertificateAction.EXPORT_PERSONAL);
       });
 
       test('OpensDialog_PasswordDecryption', function() {
         testDialogOpensOnAction(
-              'settings-certificate-password-decryption-dialog',
-              settings.CertificateAction.IMPORT_PERSONAL);
+            'settings-certificate-password-decryption-dialog',
+            settings.CertificateAction.IMPORT);
       });
 
       test('OpensDialog_CaTrustEdit', function() {
         testDialogOpensOnAction(
-              'settings-ca-trust-edit-dialog', settings.CertificateAction.EDIT);
+            'settings-ca-trust-edit-dialog', settings.CertificateAction.EDIT);
+      });
+    });
+  }
+
+  function registerCertificateListTests() {
+    /** @type {?SettingsCertificateListElement} */
+    var element = null;
+
+    /** @type {?TestCertificatesBrowserProxy} */
+    var browserProxy = null;
+
+    suite('CertificateListTests', function() {
+      setup(function() {
+        browserProxy = new TestCertificatesBrowserProxy();
+        settings.CertificatesBrowserProxyImpl.instance_ = browserProxy;
+        PolymerTest.clearBody();
+        element = document.createElement('settings-certificate-list');
+        document.body.appendChild(element);
+      });
+
+      teardown(function() { element.remove(); });
+

[Dan Beam, 2016/04/01 18:38:02]

nit: \n\n -> \n

[dpapad, 2016/04/01 21:13:17]

Done.

+
+      /**
+       * Tests the "Import" button functionality.
+       * @param {!settings.CertificateType} certificateType
+       * @param {string} proxyMethodName The name of the proxy method expected
+       *     to be called.
+       * @param {boolean} actionEventExpected Whether a
+       *     settings.CertificateActionEvent is expected to fire as a result
+       *     tapping the Import button.
+       */
+      function testImportForCertificateType(
+          certificateType, proxyMethodName, actionEventExpected) {
+        element.certificateType = certificateType
+        Polymer.dom.flush();
+
+        var importButton = element.$$('paper-button');
+        assertTrue(!!importButton);
+
+        var waitForActionEvent = actionEventExpected ?
+            eventToPromise(settings.CertificateActionEvent, element) :
+            Promise.resolve(null);
+
+        MockInteractions.tap(importButton);
+        return browserProxy.whenCalled(proxyMethodName).then(
+            function() {
+              return waitForActionEvent;
+            }).then(
+            function(event) {

[Dan Beam, 2016/04/01 18:38:02]

should this be

  .then(function(event) {

on the same line or more continuation indent?

[dpapad, 2016/04/01 21:13:17]

Done.

Personally I am equally OK with either way. My brain has been trained to parse
Promise-based syntax enough to not struggle with the \n positioning. I chose
this syntax because of a comment by @tommycli in a previous review for a similar
case, see
https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource....

Ultimately it would be nice if we come to a conclusion about such cases, by
either of the following:
1) Allow flexible formatting, and stop striving to be 100% consistent on \n
positioning across the codebase (with regards to promise callbacks).
2) Find a single formatting style that keeps everyone happy and stick with it.

No1 sounds more viable and realistic IMO. Current situation of formatting \n
based on who is reviewing, leads to the same result as option 1 above (having
both occur in the code), but requires more time (from reviewee and reviewer)
than option 1.

+              if (actionEventExpected) {
+                assertEquals(
+                    settings.CertificateAction.IMPORT, event.detail.action);
+                assertEquals(certificateType, event.detail.certificateType);
+              }
+            });

[Dan Beam, 2016/04/01 18:38:02]

i'm not trying to enforce my ident on you, but

return browserProxy.whenCalled(proxyMethodName).then(function() {
  return waitForActionEvent;
}.then(function(event) {
  ...
});

seems less convoluted to me

[dpapad, 2016/04/01 21:13:17]

Done.

+      }
+
+      test('ImportButton_Personal', function() {
+        return testImportForCertificateType(
+            settings.CertificateType.PERSONAL,
+            'importPersonalCertificate', true);
+      });
+
+      test('ImportButton_Server', function() {
+        return testImportForCertificateType(
+            settings.CertificateType.SERVER, 'importServerCertificate',
+            false);
+      });
+
+      test('ImportButton_CA', function() {
+        return testImportForCertificateType(
+            settings.CertificateType.CA, 'importCaCertificate', true);
       });
     });
   }
@@ -691,6 +785,7 @@ cr.define('certificate_manager_page', function() {
       registerPasswordDecryptDialogTests();
       registerPageTests();
       registerCertificateSubentryTests();
+      registerCertificateListTests();
     },
   };
 });