Fixed a crash in Chrome caused by duplicate certificates

chrome/browser/ui/views/certificate_selector.cc

 // Copyright 2015 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.
 
 #include "chrome/browser/ui/views/certificate_selector.h"
 
 #include <stddef.h>  // For size_t.
 #include <string>
 #include <vector>
 
@@ skipping 112 matching lines @@
   // The entry at index |i| is the provider name for |certificates_[i]|.
   std::vector<std::string> provider_names;
 #if defined(OS_CHROMEOS)
   chromeos::CertificateProviderService* service =
       chromeos::CertificateProviderServiceFactory::GetForBrowserContext(
           web_contents->GetBrowserContext());
   extensions::ExtensionRegistry* extension_registry =
       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;
     std::string extension_id;
     if (service->LookUpCertificate(*cert, &has_extension, &extension_id)) {
       if (!has_extension) {
         // This certificate was provided by an extension but isn't provided by
         // any extension currently. Don't expose it to the user.
         continue;
       }
@@ skipping 28 matching lines @@
   // GetTopLevelWebContents returns |web_contents| if it is not a guest.
   content::WebContents* top_level_web_contents =
       guest_view::GuestViewBase::GetTopLevelWebContents(web_contents);
   return web_modal::WebContentsModalDialogManager::FromWebContents(
              top_level_web_contents) != nullptr;
 }
 
 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);
 }
 
 void CertificateSelector::InitWithText(
     std::unique_ptr<views::View> text_label) {
   views::GridLayout* const layout = views::GridLayout::CreatePanel(this);
   SetLayoutManager(layout);
 
@@ skipping 82 matching lines @@
 void CertificateSelector::OnSelectionChanged() {
   GetDialogClientView()->ok_button()->SetEnabled(GetSelectedCert() != nullptr);
 }
 
 void CertificateSelector::OnDoubleClick() {
   if (GetSelectedCert())
     GetDialogClientView()->AcceptWindow();
 }
 
 }  // namespace chrome