MD Settings: Certificate manager, hooking up all dialogs.

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview
  * 'settings-ca-trust-edit-dialog' is the a dialog allowing the user to edit the
  * trust lever of a given certificate authority.
  *
  * @group Chrome Settings Elements
@@ skipping 42 matching lines @@
   /** @private */
   onOkTap_: function() {
     this.$.spinner.active = true;
     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) {

[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));
   },
 });