For the SSL cert status, convert anonymous enum that gives bit values into a typedefed uint32. This allows code all over Chromium to use an explicit type instead of "int". (This isn't possible by simply naming the enum as technically the enum doesn't define all of the possible combinations of bits.) This also means the individual named bit constants themselves have the same explicit type. I find the resulting code to be noticeably clearer. This also exposed a bug in SSLErrorInfo::GetErrorsForCertStatus() where not having an explicit type allowed a function argument ordering bug to creep in, so I claim this is safer too.

I also added CERT_STATUS_NO_ERROR in place of "0" as a magic number.

Normally this makes things like DCHECK_EQ() unhappy, but when I'd originally tested this I didn't seem to need to make any changes due to that. Will be watching the trybots...

The original motiviation for this change was to find a way to eliminate some cases of passing anonymous-typed values as template arguments (which happens when you use a value from the enum in e.g. EXPECT_EQ()), which is technically illegal in C++03, though we don't warn about it. Simply naming the enum would have done this, but this would have encouraged readers to actually use the enum name as a type, which for a bitfield is inappropriate for the reason given in the first paragraph.

BUG=92247
TEST=Compiles
Review URL: http://codereview.chromium.org/7819009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@102322 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 24 insertions, 18 deletions

diff --git a/content/browser/ssl/ssl_manager.cc b/content/browser/ssl/ssl_manager.cc
index 2f88040..6ae6fb9 100644
--- a/content/browser/ssl/ssl_manager.cc
+++ b/content/browser/ssl/ssl_manager.cc
@@ -55,12 +55,12 @@ void SSLManager::NotifySSLInternalStateChanged(
 
 // static
 std::string SSLManager::SerializeSecurityInfo(int cert_id,
-                                              int cert_status,
+                                              net::CertStatus cert_status,
                                               int security_bits,
                                               int ssl_connection_status) {
   Pickle pickle;
   pickle.WriteInt(cert_id);
-  pickle.WriteInt(cert_status);
+  pickle.WriteUInt32(cert_status);
   pickle.WriteInt(security_bits);
   pickle.WriteInt(ssl_connection_status);
   return std::string(static_cast<const char*>(pickle.data()), pickle.size());
@@ -69,7 +69,7 @@ std::string SSLManager::SerializeSecurityInfo(int cert_id,
 // static
 bool SSLManager::DeserializeSecurityInfo(const std::string& state,
                                          int* cert_id,
-                                         int* cert_status,
+                                         net::CertStatus* cert_status,
                                          int* security_bits,
                                          int* ssl_connection_status) {
   DCHECK(cert_id && cert_status && security_bits && ssl_connection_status);
@@ -86,7 +86,7 @@ bool SSLManager::DeserializeSecurityInfo(const std::string& state,
   Pickle pickle(state.data(), static_cast<int>(state.size()));
   void * iter = NULL;
   return pickle.ReadInt(&iter, cert_id) &&
-         pickle.ReadInt(&iter, cert_status) &&
+         pickle.ReadUInt32(&iter, cert_status) &&
          pickle.ReadInt(&iter, security_bits) &&
          pickle.ReadInt(&iter, ssl_connection_status);
 }
@@ -124,8 +124,10 @@ void SSLManager::DidCommitProvisionalLoad(
   if (details->is_main_frame) {
     if (entry) {
       // Decode the security details.
-      int ssl_cert_id, ssl_cert_status, ssl_security_bits,
-          ssl_connection_status;
+      int ssl_cert_id;
+      net::CertStatus ssl_cert_status;
+      int ssl_security_bits;
+      int ssl_connection_status;
       DeserializeSecurityInfo(details->serialized_security_info,
                               &ssl_cert_id,
                               &ssl_cert_status,
diff --git a/content/browser/ssl/ssl_manager.h b/content/browser/ssl/ssl_manager.h
index 1e59e7b..24892bf 100644
--- a/content/browser/ssl/ssl_manager.h
+++ b/content/browser/ssl/ssl_manager.h
@@ -15,6 +15,7 @@
 #include "content/common/notification_observer.h"
 #include "content/common/notification_registrar.h"
 #include "googleurl/src/gurl.h"
+#include "net/base/cert_status_flags.h"
 #include "net/base/net_errors.h"
 
 class LoadFromMemoryCacheDetails;
@@ -57,14 +58,15 @@ class SSLManager : public NotificationObserver {
 
   // Convenience methods for serializing/deserializing the security info.
   static std::string SerializeSecurityInfo(int cert_id,
-                                           int cert_status,
+                                           net::CertStatus cert_status,
                                            int security_bits,
                                            int connection_status);
-  CONTENT_EXPORT static bool DeserializeSecurityInfo(const std::string& state,
-                                                     int* cert_id,
-                                                     int* cert_status,
-                                                     int* security_bits,
-                                                     int* connection_status);
+  CONTENT_EXPORT static bool DeserializeSecurityInfo(
+      const std::string& state,
+      int* cert_id,
+      net::CertStatus* cert_status,
+      int* security_bits,
+      int* connection_status);
 
   // Construct an SSLManager for the specified tab.
   // If |delegate| is NULL, SSLPolicy::GetDefaultPolicy() is used.
diff --git a/content/browser/ssl/ssl_policy.cc b/content/browser/ssl/ssl_policy.cc
index 1d7c981..1d9f3f6 100644
--- a/content/browser/ssl/ssl_policy.cc
+++ b/content/browser/ssl/ssl_policy.cc
@@ -131,7 +131,8 @@ void SSLPolicy::UpdateEntry(NavigationEntry* entry, TabContents* tab_contents) {
 
   // If CERT_STATUS_UNABLE_TO_CHECK_REVOCATION is the only certificate error,
   // don't lower the security style to SECURITY_STYLE_AUTHENTICATION_BROKEN.
-  int cert_errors = entry->ssl().cert_status() & net::CERT_STATUS_ALL_ERRORS;
+  net::CertStatus cert_errors =
+      entry->ssl().cert_status() & net::CERT_STATUS_ALL_ERRORS;
   if (cert_errors) {
     if (cert_errors != net::CERT_STATUS_UNABLE_TO_CHECK_REVOCATION)
       entry->ssl().set_security_style(SECURITY_STYLE_AUTHENTICATION_BROKEN);
diff --git a/content/browser/ssl/ssl_request_info.cc b/content/browser/ssl/ssl_request_info.cc
index 19d4e4d..b999799 100644
--- a/content/browser/ssl/ssl_request_info.cc
+++ b/content/browser/ssl/ssl_request_info.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -8,7 +8,7 @@ SSLRequestInfo::SSLRequestInfo(const GURL& url,
                                ResourceType::Type resource_type,
                                int child_id,
                                int ssl_cert_id,
-                               int ssl_cert_status)
+                               net::CertStatus ssl_cert_status)
     : url_(url),
       resource_type_(resource_type),
       child_id_(child_id),
diff --git a/content/browser/ssl/ssl_request_info.h b/content/browser/ssl/ssl_request_info.h
index 1ab433d..a7f9fbe 100644
--- a/content/browser/ssl/ssl_request_info.h
+++ b/content/browser/ssl/ssl_request_info.h
@@ -10,6 +10,7 @@
 
 #include "base/memory/ref_counted.h"
 #include "googleurl/src/gurl.h"
+#include "net/base/cert_status_flags.h"
 #include "webkit/glue/resource_type.h"
 
 // SSLRequestInfo wraps up the information SSLPolicy needs about a request in
@@ -21,13 +22,13 @@ class SSLRequestInfo : public base::RefCounted<SSLRequestInfo> {
                  ResourceType::Type resource_type,
                  int child_id,
                  int ssl_cert_id,
-                 int ssl_cert_status);
+                 net::CertStatus ssl_cert_status);
 
   const GURL& url() const { return url_; }
   ResourceType::Type resource_type() const { return resource_type_; }
   int child_id() const { return child_id_; }
   int ssl_cert_id() const { return ssl_cert_id_; }
-  int ssl_cert_status() const { return ssl_cert_status_; }
+  net::CertStatus ssl_cert_status() const { return ssl_cert_status_; }
 
  private:
   friend class base::RefCounted<SSLRequestInfo>;
@@ -38,7 +39,7 @@ class SSLRequestInfo : public base::RefCounted<SSLRequestInfo> {
   ResourceType::Type resource_type_;
   int child_id_;
   int ssl_cert_id_;
-  int ssl_cert_status_;
+  net::CertStatus ssl_cert_status_;
 
   DISALLOW_COPY_AND_ASSIGN(SSLRequestInfo);
 };