We should use SSLInfo or related types for HTTPS...

chrome/browser/page_info_model.cc

 // Copyright (c) 2010 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/page_info_model.h"
 
 #include <string>
 
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
@@ skipping 143 matching lines @@
       description,
       SECTION_INFO_IDENTITY));
 
   // Connection section.
   // 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.
   icon_id = ICON_STATE_OK;
   headline.clear();
   description.clear();
-  if (ssl.security_bits() < 0) {
+  if (!ssl.cert_id()) {
+    // Not HTTPS.
+    DCHECK_EQ(ssl.security_style(), SECURITY_STYLE_UNAUTHENTICATED);

[Finnur, 2010/11/09 11:40:04]

Wait... we are not failing safe in this case, are we? If we get to a state where
security_style is != unauthenticated don't we have a big problem somewhere?
Should we say:

icon_id = ssl.security_style() == SECURITY_STYLE_UNAUTHENTICATED ?
ICON_STATE_WARNING_MAJOR : ICON_STATE_ERROR;

WDYT?

Or, alternatively, add (... && ssl.security_style() ==
SECURITY_STYLE_UNAUTHENTICATED) to line 164 since then we fall into the clause
on line 171.

No?

[wtc, 2010/11/10 20:40:56]

In general I trust our code maintains invariants correctly.
This is why I simply use a DCHECK here to assert an
invariant.

But I implemented your first suggestion as fail-safe/defensive
programming.  Please review Patch Set 3.
This won't work because the subsequent clauses starting on
line 171 need to be assured that ssl.cert_id() is valid
(nonzero).  That's the idea of this CL: we should access
ssl.security_bits() only for HTTPS (when ssl.cert_id() is
valid).

+    icon_id = ICON_STATE_WARNING_MAJOR;
+    description.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.
     icon_id = ICON_STATE_ERROR;
   } else if (ssl.security_bits() == 0) {
-    icon_id = ssl.security_style() == SECURITY_STYLE_UNAUTHENTICATED ?
-        ICON_STATE_WARNING_MAJOR : ICON_STATE_ERROR;
+    DCHECK_NE(ssl.security_style(), SECURITY_STYLE_UNAUTHENTICATED);
+    icon_id = ICON_STATE_ERROR;
     description.assign(l10n_util::GetStringFUTF16(
         IDS_PAGE_INFO_SECURITY_TAB_NOT_ENCRYPTED_CONNECTION_TEXT,
         subject_name));
   } else if (ssl.security_bits() < 80) {
     icon_id = ICON_STATE_ERROR;
     description.assign(l10n_util::GetStringFUTF16(
         IDS_PAGE_INFO_SECURITY_TAB_WEAK_ENCRYPTION_CONNECTION_TEXT,
         subject_name));
   } else {
     description.assign(l10n_util::GetStringFUTF16(
@@ skipping 169 matching lines @@
 gfx::NativeImage PageInfoModel::GetBitmapNamed(int resource_id) {
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
   gfx::NativeImage image = rb.GetNativeImageNamed(resource_id);
 #if defined(OS_MACOSX)
   // Unlike other platforms, the Mac ResourceBundle does not keep a shared image
   // cache. These are released in the dtor.
   mac_util::NSObjectRetain(image);
 #endif
   return image;
 }