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;
+      case settings.CertificateType.OTHER:
+        localizedMessageId = 'certificateManagerDeleteOtherTitle';
+        break;
+      default:

[Dan Beam, 2016/03/10 19:01:26]

if you're enumerating every case, remove the default:, it may give you better
compiler coverage

[Dan Beam, 2016/03/10 19:53:53]

here's some examples of doing this to make both compile-time and run-time
checking possible:
https://gist.github.com/anonymous/f4b1abfbcffb075c5c2b
https://gist.github.com/anonymous/e4f94b232bd0c26649cf

[dpapad, 2016/03/10 20:21:12]

Changed per suggestion.

As a side-comment, suggestion 1 relies on "assertNotReached()" being handled
specially by Chromium's version of the compiler, which means future readers need
this implicit knowledge to understand how this is passing the "missing return"
compiler check.

Similar example with vanilla compiler fails, see
https://closure-compiler.appspot.com/home#code%3D%252F%252F%2520%253D%253DClo...

[Dan Beam, 2016/03/10 21:44:13]

feel free to fix this however you see fit
i recommend using the same compiler that will check your code

+        assertNotReached();
+    }
+    return loadTimeData.getStringF(localizedMessageId, this.model.name);
+  },
+
+  /**
+   * @private
+   * @return {string}
+   */
+  getDescriptionText_: function() {
+    var localizedMessageId = null;

[Dan Beam, 2016/03/10 19:01:26]

nit: why null over undefined?

[dpapad, 2016/03/10 20:21:12]

Removed.

For the record, I think "null" serves as a much better sentinel value
(https://en.wikipedia.org/wiki/Sentinel_value) compared to "undefined". The
latter can be the result of various programmer errors, whereas when a variable
has a null value, it means that it was explicitly assigned to null at some
previous execution point.

[Dan Beam, 2016/03/10 21:44:13]

either one is fine, null just requires an assignment

generally: i don't really see why they both exist, but that ship has sailed.

I do know that undefined is the language's default state

  var blah;  // typeof blah == 'undefined'
  typeof window.randomkey // == 'undefined'

either are fine, i was just wondering why

+    switch (this.certificateType) {
+      case settings.CertificateType.PERSONAL:
+        localizedMessageId = 'certificateManagerDeleteUserDescription';
+        break;
+      case settings.CertificateType.SERVER:
+        localizedMessageId = 'certificateManagerDeleteServerDescription';
+        break;
+      case settings.CertificateType.CA:
+        localizedMessageId = 'certificateManagerDeleteCaDescription';
+        break;
+      case settings.CertificateType.OTHER:
+        break;
+      default:
+        assertNotReached();
+    }
+    return localizedMessageId == null ? '' :

[Dan Beam, 2016/03/10 19:01:26]

nit: use === when multiple falsy values are involved

[dpapad, 2016/03/10 20:21:12]

Removed.

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