MD Settings: Certificate manager, hooking up all dialogs.

chrome/browser/resources/settings/certificate_manager_page/ca_trust_edit_dialog.js

Index: chrome/browser/resources/settings/certificate_manager_page/ca_trust_edit_dialog.js
diff --git a/chrome/browser/resources/settings/certificate_manager_page/ca_trust_edit_dialog.js b/chrome/browser/resources/settings/certificate_manager_page/ca_trust_edit_dialog.js
index 019f8fd85d5ec5605e60c2f1911e407e69e3ab3c..8160fc87c8d3dc61f0a07b7cf7067b680bb4b00b 100644
--- a/chrome/browser/resources/settings/certificate_manager_page/ca_trust_edit_dialog.js
+++ b/chrome/browser/resources/settings/certificate_manager_page/ca_trust_edit_dialog.js
@@ -61,7 +61,9 @@ Polymer({
     }.bind(this),
     /** @param {!CertificatesError} error */
     function(error) {

[tommycli, 2016/03/23 19:28:04]

Nit: Can we redo the indentation on this a bit?

It took me a while to understand that the function(error) was another parameter
of the then() call because of indendation.

[dpapad, 2016/03/24 22:06:10]

Can you suggest an alternative indentation that works? I could not find another
one given the existing constraints. The existence of @param prevents me from
moving "function (error)" to the previous line.

[tommycli, 2016/03/24 22:34:14]

Does this work? I realize it's a great deal of indenting, but it disambiguates
it for me.

    this.browserProxy_.editCaCertificateTrust(
        this.model.id, this.$.ssl.checked,
        this.$.email.checked, this.$.objSign.checked).then(
            function() {
              this.$.spinner.active = false;
              this.$.dialog.close();
            }.bind(this),
            /** @param {!CertificatesError} error */
            function(error) {
              // TODO(dpapad): Display error here.
            });

[dpapad, 2016/03/25 17:36:03]

Done.

-      // TODO(dpapad): Display error here.
-    });
+      this.$.dialog.close();
+      this.dispatchEvent(

[tommycli, 2016/03/23 19:28:04]

Any particular reason we're using dispatchEvent here instead of fire? I've seen
fire more commonly used around our Settings code.

[dpapad, 2016/03/24 22:06:10]

Changed all of them to fire() for consistency.

The reason I was not using it is that "fire()" seems like an unnecessary wrapper
around what I was doing. If you take a look at the implementation
(https://github.com/Polymer/polymer/blob/441103159f7b9fc9af567b2caea145de21a95...),
it mostly makes sense if the event caching feature of Polymer was being used.

[tommycli, 2016/03/24 22:34:14]

Arg... that's unfortunate. I'm not so sure about my suggestion anymore. I guess
for consistency it's probably worth paying the performance cost.

+          new CustomEvent('certificates-error', {detail: error}));
+    }.bind(this));
   },
 });