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

chrome/browser/page_info_model.cc

Index: chrome/browser/page_info_model.cc
===================================================================
--- chrome/browser/page_info_model.cc	(revision 64489)
+++ chrome/browser/page_info_model.cc	(working copy)
@@ -161,12 +161,19 @@
   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));