Remove unneeded DCHECK for connection status.

chrome/browser/ui/website_settings/website_settings.cc

 // Copyright (c) 2012 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/website_settings/website_settings.h"
 
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 417 matching lines @@
           IDS_PAGE_INFO_SECURITY_TAB_NON_UNIQUE_NAME);
     }
   }
 
   // Site Connection
   // We consider anything less than 80 bits encryption to be weak encryption.
   // TODO(wtc): Bug 1198735: report mixed/unsafe content for unencrypted and
   // weakly encrypted connections.
   site_connection_status_ = SITE_CONNECTION_STATUS_UNKNOWN;
 
-  if (!ssl.cert_id) {
-    // Not HTTPS.
-    DCHECK_EQ(ssl.security_style, content::SECURITY_STYLE_UNAUTHENTICATED);

[wtc, 2014/02/25 22:55:27]

Babu:

I just looked at bug 344891 that you filed. I didn't know that
this DCHECK actually failed. That is important info.

What was the value of ssl.security_style when the DCHECK failed?
content::SECURITY_STYLE_UNKNOWN? If so, we may need a different
solution. Either we should not display the page infobubble while
the page is still loading, or we should add a new case for
content::SECURITY_STYLE_UNKNOWN, like this:

  if (ssl.security_style == content::SECURITY_STYLE_UNKNOWN) {
    // Page is still loading.
    ...
  } else if (ssl.security_style == content::SECURITY_STYLE_UNAUTHENTICATED) {
    // HTTPS without a certificate, or not HTTPS.
    ...
  } else if (ssl.security_bits < 0) {
    ...

Perhaps the content::SECURITY_STYLE_UNKNOWN case is already handled by
the ssl.security_bits < 0 check?

-    if (ssl.security_style == content::SECURITY_STYLE_UNAUTHENTICATED)
-      site_connection_status_ = SITE_CONNECTION_STATUS_UNENCRYPTED;
-    else
-      site_connection_status_ = SITE_CONNECTION_STATUS_ENCRYPTED_ERROR;
+  if (ssl.security_style == content::SECURITY_STYLE_UNAUTHENTICATED) {

[wtc, 2014/02/27 22:30:29]

As a workaround for the inability to disable the page info bubble during the
initial page load phase, I suggest we add a new case before this case:

  if (ssl.security_style == content::SECURITY_STYLE_UNKNOWN) {
    // Page is still loading, so SSL status is not yet available. Say nothing.
    DCHECK_EQ(security_bits, -1);
    site_connection_status_ = SITE_CONNECTION_STATUS_UNENCRYPTED;
  } else if (ssl.security_style == content::SECURITY_STYLE_UNAUTHENTICATED) {
    ...

Ideally we should set site_connection_status_ SITE_CONNECTION_STATUS_UNKNOWN,
but I am worried we can't do that (see the DCHECK on line 645). Setting it to
SITE_CONNECTION_STATUS_ENCRYPTED_ERROR (if we fall through to line 448) seems
wrong because we really don't know.

+    // HTTPS without a certificate, or not HTTPS.
+    DCHECK(!ssl.cert_id);
+    site_connection_status_ = SITE_CONNECTION_STATUS_UNENCRYPTED;
 
     site_connection_details_.assign(l10n_util::GetStringFUTF16(
         IDS_PAGE_INFO_SECURITY_TAB_NOT_ENCRYPTED_CONNECTION_TEXT,
         subject_name));
   } else if (ssl.security_bits < 0) {
     // Security strength is unknown.  Say nothing.
     site_connection_status_ = SITE_CONNECTION_STATUS_ENCRYPTED_ERROR;
   } else if (ssl.security_bits == 0) {
     DCHECK_NE(ssl.security_style, content::SECURITY_STYLE_UNAUTHENTICATED);
     site_connection_status_ = SITE_CONNECTION_STATUS_ENCRYPTED_ERROR;
@@ skipping 225 matching lines @@
   if (visited_before_today) {
     first_visit_text = l10n_util::GetStringFUTF16(
         IDS_PAGE_INFO_SECURITY_TAB_VISITED_BEFORE_TODAY,
         base::TimeFormatShortDate(first_visit));
   } else {
     first_visit_text = l10n_util::GetStringUTF16(
         IDS_PAGE_INFO_SECURITY_TAB_FIRST_VISITED_TODAY);
   }
   ui_->SetFirstVisit(first_visit_text);
 }