MD Settings: Certificate manager, delete confirmation dialog.

chrome/browser/resources/settings/certificate_manager_page/certificate_delete_confirmation_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 A confirmation dialog allowing the user to delete various types
+ * of certificates.
+ */
+Polymer({
+  is: 'settings-certificate-delete-confirmation-dialog',
+
+  properties: {
+    /** @private {!settings.CertificatesBrowserProxy} */
+    browserProxy_: Object,
+
+    /** @type {!CertificateSubnode} */
+    model: Object,
+
+    /** @type {!settings.CertificateType} */
+    certificateType: String,
+  },
+
+  /** @override */
+  ready: function() {
+    this.browserProxy_ = settings.CertificatesBrowserProxyImpl.getInstance();
+  },
+
+  /** @override */
+  attached: function() {
+    this.$.dialog.open();
+  },
+
+  /**
+   * @private
+   * @return {string}
+   */
+  getTitleText_: function() {
+    var localizedMessageId = null;
+    switch (this.certificateType) {
+      case settings.CertificateType.PERSONAL:
+        localizedMessageId = 'certificateManagerDeleteUserTitle';
+        break;
+      case settings.CertificateType.SERVER:
+        localizedMessageId = 'certificateManagerDeleteServerTitle';
+        break;
+      case settings.CertificateType.CA:
+        localizedMessageId = 'certificateManagerDeleteCaTitle';
+        break;
+      default: // settings.CertificateType.OTHER

[Dan Beam, 2016/03/10 05:03:05]

why are you using default instead of case settings.CertificateType.OTHER: to
enumerate all types?

[dpapad, 2016/03/10 18:31:07]

Changed. I was using "default" for the only remaining type, which must be OTHER
at this point (indicated by the comment), but assertNotReached() is probably
better.

+        localizedMessageId = 'certificateManagerDeleteOtherTitle';
+    }
+    return loadTimeData.getStringF(localizedMessageId, this.model.name);
+  },
+
+  /**
+   * @private
+   * @return {string}
+   */
+  getDescriptionText_: function() {
+    var localizedMessageId = null;
+    switch (this.certificateType) {
+      case settings.CertificateType.PERSONAL:
+        localizedMessageId = 'certificateManagerDeleteUserDescription';
+        break;
+      case settings.CertificateType.SERVER:
+        localizedMessageId = 'certificateManagerDeleteServerDescription';
+        break;
+      case settings.CertificateType.CA:
+        localizedMessageId = 'certificateManagerDeleteCaDescription';
+        break;
+      default: // settings.CertificateType.OTHER
+        break;
+    }
+    return localizedMessageId == null ? '' :

[Dan Beam, 2016/03/10 05:03:05]

nit: maybe just return from each case with an

  assertNotReached();

at the bottom so the compiler knows a string is always returned?

[dpapad, 2016/03/10 18:31:07]

Changed similarly to previous comment. Regardless of this change, a string is
always returned (execution always gets to line 74), so I don't see any problems
for the compiler here.

+        loadTimeData.getString(localizedMessageId);
+  },
+
+  /** @private */
+  onCancelTap_: function() {
+    this.$.dialog.close();
+  },
+
+  /** @private */
+  onOkTap_: function() {
+    this.browserProxy_.deleteCertificate(this.model.id).then(function() {
+      this.$.dialog.close();
+    }.bind(this),
+    /** @param {!CertificatesError} error */
+    function(error) {
+      // TODO(dpapad): Display error here.
+    });
+  },
+});