Fixed a crash in Chrome caused by duplicate certificates

chrome/browser/ui/views/certificate_selector.cc

Index: chrome/browser/ui/views/certificate_selector.cc
diff --git a/chrome/browser/ui/views/certificate_selector.cc b/chrome/browser/ui/views/certificate_selector.cc
index f6cb4f7443b5f3bc19a98e229ecbb41e7786803f..c41fe5bdc9187efde0b5ff85a376cf02bd5c5450 100644
--- a/chrome/browser/ui/views/certificate_selector.cc
+++ b/chrome/browser/ui/views/certificate_selector.cc
@@ -130,6 +130,13 @@ CertificateSelector::CertificateSelector(
       extensions::ExtensionRegistryFactory::GetForBrowserContext(
           web_contents->GetBrowserContext());
 
+  // TODO(isandrk): See crbug.com/641440. A certificate that was previously

[emaxx, 2016/08/29 14:16:40]

nit: I think it makes more sense to move this crbug reference into the very end
of comment.

[Ivan Šandrk, 2016/08/29 15:07:49]

Done.

+  //    provided by *both* the TPM and an extension will get incorrectly

[emaxx, 2016/08/29 14:16:39]

nit: I don't think that the text in comments should be indented. (And I don't
see any examples of doing this in the existing Chrome code base.)

[emaxx, 2016/08/29 14:16:40]

nit: s/TPM/platform/
(this is a more generic and correct description)

[Ivan Šandrk, 2016/08/29 15:07:49]

Done.

+  //    filtered out if the extension stops providing it (both instances will be
+  //    filtered out). Hence the certificates_ array will be empty and an empty
+  //    certificate selection dialog will be shown which will cause a crash

[emaxx, 2016/08/29 14:16:40]

nit: I would suggest rephrasing this sentence in a little bit more careful
fashion:
"..., hence the certificates_ array will be empty. But displaying a dialog with
an empty list won't make much sense for the user, and also there are some CHECKs
in the code that will fail when the list is empty. That's why an early exit is
performed here."

(I'm assuming that the comment will be moved into the Show method.)

[Ivan Šandrk, 2016/08/29 15:07:49]

Done.

+  //    (temporary workaround added in CertificateSelector::Show to prevent the
+  //     crash).

[emaxx, 2016/08/29 14:16:40]

nit: Generally, why not move this text and merge with the comment in the Show
method? The actual change is inside the Show method.
And, if you want, you may just refer to the code in the constructor, e.g.:
"... will get incorrectly filtered out in the constructor ..."

[Ivan Šandrk, 2016/08/29 15:07:49]

Done.

   for (const auto& cert : certificates) {
     std::string provider_name;
     bool has_extension = false;
@@ -178,6 +185,13 @@ bool CertificateSelector::CanShow(content::WebContents* web_contents) {
 void CertificateSelector::Show() {
   constrained_window::ShowWebModalDialogViews(this, web_contents_);
 
+  // TODO(isandrk): Temporary workaround for crbug.com/641440. certificates_
+  //    array may sometimes be empty by mistake.
+  if (certificates_.empty()) {
+    GetWidget()->Close();
+    return;
+  }
+
   // Select the first row automatically.  This must be done after the dialog has
   // been created.
   table_->Select(0);