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 160 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): A certificate that was previously provided by *both* the

[msw, 2016/08/29 17:08:29]

What exactly is the TODO here? Will you try to fix the incorrect filtering? If
that is the actual fix we want, can we avoid this change, or just add a
DCHECK(!certificates_.empty()) here instead?

[Ivan Šandrk, 2016/08/30 14:23:22]

So this is just a quick fix for the crash, the actual bug is tracked in
crbug.com/641440 and will be fixed at a later time.

+  // platform and an extension will get incorrectly filtered out if the
+  // extension stops providing it (both instances will be filtered out), hence
+  // the certificates_ array will be empty. Displaying a dialog with an empty

[msw, 2016/08/29 17:08:29]

nit: |certificates_|

[Ivan Šandrk, 2016/08/30 14:23:22]

Done.

+  // 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 and that's why an early exit
+  //  is performed here. See crbug.com/641440 for more details.

[msw, 2016/08/29 17:08:29]

nit: fix indent.

[Ivan Šandrk, 2016/08/30 14:23:23]

Done.

+  if (certificates_.empty()) {
+    GetWidget()->Close();

[msw, 2016/08/29 17:08:28]

How is this dialog triggered? If the user clicks a button to open this manually,
what happens in that case? It seems odd to just no-op if the user clicks
something and expects to see a dialog. Should it show a message instead?

[Ivan Šandrk, 2016/08/30 14:23:22]

Chrome network stack requests the dialog to be open during TLS handshake, this
is not directly triggered by user action. So this behaviour should be fine.

+    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