(Original patch reviewed at http://codereview.chromium.org/2067003 )

Track "display" and "run" separately for mixed content, and make the latter downgrade the SSL state to "authentication broken".

Make the "display" state only affect the current tab (not the entire host).

Fix an SSL browser test by supplying the appropriate SiteInstance*.

Move a test from "disabled" to "flaky" since it at least passes for me.

Make the SSLManager header and .cc files put functions in the same order, and make that order somewhat saner.

BUG=15072, 18626, 40932, 42758
TEST=Covered by browser tests
Review URL: http://codereview.chromium.org/2063008

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

1 files changed, 125 insertions, 124 deletions

diff --git a/chrome/browser/ssl/ssl_manager.cc b/chrome/browser/ssl/ssl_manager.cc
index 7683221..99729b6 100644
--- a/chrome/browser/ssl/ssl_manager.cc
+++ b/chrome/browser/ssl/ssl_manager.cc
@@ -28,38 +28,6 @@ void SSLManager::RegisterUserPrefs(PrefService* prefs) {
                              FilterPolicy::DONT_FILTER);
 }
 
-SSLManager::SSLManager(NavigationController* controller)
-    : backend_(controller),
-      policy_(new SSLPolicy(&backend_)),
-      controller_(controller) {
-  DCHECK(controller_);
-
-  // Subscribe to various notifications.
-  registrar_.Add(this, NotificationType::FAIL_PROVISIONAL_LOAD_WITH_ERROR,
-                 Source<NavigationController>(controller_));
-  registrar_.Add(this, NotificationType::RESOURCE_RESPONSE_STARTED,
-                 Source<NavigationController>(controller_));
-  registrar_.Add(this, NotificationType::RESOURCE_RECEIVED_REDIRECT,
-                 Source<NavigationController>(controller_));
-  registrar_.Add(this, NotificationType::LOAD_FROM_MEMORY_CACHE,
-                 Source<NavigationController>(controller_));
-  registrar_.Add(this, NotificationType::SSL_INTERNAL_STATE_CHANGED,
-                 NotificationService::AllSources());
-}
-
-SSLManager::~SSLManager() {
-}
-
-bool SSLManager::ProcessedSSLErrorFromRequest() const {
-  NavigationEntry* entry = controller_->GetActiveEntry();
-  if (!entry) {
-    NOTREACHED();
-    return false;
-  }
-
-  return net::IsCertStatusError(entry->ssl().cert_status());
-}
-
 // static
 void SSLManager::OnSSLCertificateError(ResourceDispatcherHost* rdh,
                                        URLRequest* request,
@@ -86,8 +54,109 @@ void SSLManager::OnSSLCertificateError(ResourceDispatcherHost* rdh,
                         &SSLCertErrorHandler::Dispatch));
 }
 
-void SSLManager::DidDisplayInsecureContent() {
-  policy()->DidDisplayInsecureContent(controller_->GetActiveEntry());
+// static
+void SSLManager::NotifySSLInternalStateChanged() {
+  NotificationService::current()->Notify(
+      NotificationType::SSL_INTERNAL_STATE_CHANGED,
+      NotificationService::AllSources(),
+      NotificationService::NoDetails());
+}
+
+// static
+std::string SSLManager::SerializeSecurityInfo(int cert_id,
+                                              int cert_status,
+                                              int security_bits) {
+  Pickle pickle;
+  pickle.WriteInt(cert_id);
+  pickle.WriteInt(cert_status);
+  pickle.WriteInt(security_bits);
+  return std::string(static_cast<const char*>(pickle.data()), pickle.size());
+}
+
+// static
+bool SSLManager::DeserializeSecurityInfo(const std::string& state,
+                                         int* cert_id,
+                                         int* cert_status,
+                                         int* security_bits) {
+  DCHECK(cert_id && cert_status && security_bits);
+  if (state.empty()) {
+    // No SSL used.
+    *cert_id = 0;
+    *cert_status = 0;
+    *security_bits = -1;
+    return false;
+  }
+
+  Pickle pickle(state.data(), static_cast<int>(state.size()));
+  void * iter = NULL;
+  return pickle.ReadInt(&iter, cert_id) &&
+         pickle.ReadInt(&iter, cert_status) &&
+         pickle.ReadInt(&iter, security_bits);
+}
+
+// static
+std::wstring SSLManager::GetEVCertName(const net::X509Certificate& cert) {
+  // EV are required to have an organization name and country.
+  if (cert.subject().organization_names.empty() ||
+      cert.subject().country_name.empty()) {
+    NOTREACHED();
+    return std::wstring();
+  }
+
+  return l10n_util::GetStringF(IDS_SECURE_CONNECTION_EV,
+                               UTF8ToWide(cert.subject().organization_names[0]),
+                               UTF8ToWide(cert.subject().country_name));
+}
+
+SSLManager::SSLManager(NavigationController* controller)
+    : backend_(controller),
+      policy_(new SSLPolicy(&backend_)),
+      controller_(controller) {
+  DCHECK(controller_);
+
+  // Subscribe to various notifications.
+  registrar_.Add(this, NotificationType::FAIL_PROVISIONAL_LOAD_WITH_ERROR,
+                 Source<NavigationController>(controller_));
+  registrar_.Add(this, NotificationType::RESOURCE_RESPONSE_STARTED,
+                 Source<NavigationController>(controller_));
+  registrar_.Add(this, NotificationType::RESOURCE_RECEIVED_REDIRECT,
+                 Source<NavigationController>(controller_));
+  registrar_.Add(this, NotificationType::LOAD_FROM_MEMORY_CACHE,
+                 Source<NavigationController>(controller_));
+  registrar_.Add(this, NotificationType::SSL_INTERNAL_STATE_CHANGED,
+                 NotificationService::AllSources());
+}
+
+SSLManager::~SSLManager() {
+}
+
+void SSLManager::DidCommitProvisionalLoad(
+    const NotificationDetails& in_details) {
+  NavigationController::LoadCommittedDetails* details =
+      Details<NavigationController::LoadCommittedDetails>(in_details).ptr();
+
+  NavigationEntry* entry = controller_->GetActiveEntry();
+
+  if (details->is_main_frame) {
+    if (entry) {
+      // Decode the security details.
+      int ssl_cert_id, ssl_cert_status, ssl_security_bits;
+      DeserializeSecurityInfo(details->serialized_security_info,
+                              &ssl_cert_id,
+                              &ssl_cert_status,
+                              &ssl_security_bits);
+
+      // We may not have an entry if this is a navigation to an initial blank
+      // page. Reset the SSL information and add the new data we have.
+      entry->ssl() = NavigationEntry::SSLStatus();
+      entry->ssl().set_cert_id(ssl_cert_id);
+      entry->ssl().set_cert_status(ssl_cert_status);
+      entry->ssl().set_security_bits(ssl_security_bits);
+    }
+    backend_.ShowPendingMessages();
+  }
+
+  UpdateEntry(entry);
 }
 
 void SSLManager::DidRunInsecureContent(const std::string& security_origin) {
@@ -95,6 +164,16 @@ void SSLManager::DidRunInsecureContent(const std::string& security_origin) {
                                   security_origin);
 }
 
+bool SSLManager::ProcessedSSLErrorFromRequest() const {
+  NavigationEntry* entry = controller_->GetActiveEntry();
+  if (!entry) {
+    NOTREACHED();
+    return false;
+  }
+
+  return net::IsCertStatusError(entry->ssl().cert_status());
+}
+
 void SSLManager::Observe(NotificationType type,
                          const NotificationSource& source,
                          const NotificationDetails& details) {
@@ -123,27 +202,6 @@ void SSLManager::Observe(NotificationType type,
   }
 }
 
-void SSLManager::DispatchSSLVisibleStateChanged() {
-  NotificationService::current()->Notify(
-      NotificationType::SSL_VISIBLE_STATE_CHANGED,
-      Source<NavigationController>(controller_),
-      NotificationService::NoDetails());
-}
-
-void SSLManager::UpdateEntry(NavigationEntry* entry) {
-  // We don't always have a navigation entry to update, for example in the
-  // case of the Web Inspector.
-  if (!entry)
-    return;
-
-  NavigationEntry::SSLStatus original_ssl_status = entry->ssl();  // Copy!
-
-  policy()->UpdateEntry(entry);
-
-  if (!entry->ssl().Equals(original_ssl_status))
-    DispatchSSLVisibleStateChanged();
-}
-
 void SSLManager::DidLoadFromMemoryCache(LoadFromMemoryCacheDetails* details) {
   DCHECK(details);
 
@@ -166,35 +224,6 @@ void SSLManager::DidLoadFromMemoryCache(LoadFromMemoryCacheDetails* details) {
   policy()->OnRequestStarted(info.get());
 }
 
-void SSLManager::DidCommitProvisionalLoad(
-    const NotificationDetails& in_details) {
-  NavigationController::LoadCommittedDetails* details =
-      Details<NavigationController::LoadCommittedDetails>(in_details).ptr();
-
-  NavigationEntry* entry = controller_->GetActiveEntry();
-
-  if (details->is_main_frame) {
-    if (entry) {
-      // Decode the security details.
-      int ssl_cert_id, ssl_cert_status, ssl_security_bits;
-      DeserializeSecurityInfo(details->serialized_security_info,
-                              &ssl_cert_id,
-                              &ssl_cert_status,
-                              &ssl_security_bits);
-
-      // We may not have an entry if this is a navigation to an initial blank
-      // page. Reset the SSL information and add the new data we have.
-      entry->ssl() = NavigationEntry::SSLStatus();
-      entry->ssl().set_cert_id(ssl_cert_id);
-      entry->ssl().set_cert_status(ssl_cert_status);
-      entry->ssl().set_security_bits(ssl_security_bits);
-    }
-    backend_.ShowPendingMessages();
-  }
-
-  UpdateEntry(entry);
-}
-
 void SSLManager::DidFailProvisionalLoadWithError(
     ProvisionalLoadDetails* details) {
   DCHECK(details);
@@ -238,48 +267,20 @@ void SSLManager::DidChangeSSLInternalState() {
   UpdateEntry(controller_->GetActiveEntry());
 }
 
-// static
-std::string SSLManager::SerializeSecurityInfo(int cert_id,
-                                              int cert_status,
-                                              int security_bits) {
-  Pickle pickle;
-  pickle.WriteInt(cert_id);
-  pickle.WriteInt(cert_status);
-  pickle.WriteInt(security_bits);
-  return std::string(static_cast<const char*>(pickle.data()), pickle.size());
-}
+void SSLManager::UpdateEntry(NavigationEntry* entry) {
+  // We don't always have a navigation entry to update, for example in the
+  // case of the Web Inspector.
+  if (!entry)
+    return;
 
-// static
-bool SSLManager::DeserializeSecurityInfo(const std::string& state,
-                                         int* cert_id,
-                                         int* cert_status,
-                                         int* security_bits) {
-  DCHECK(cert_id && cert_status && security_bits);
-  if (state.empty()) {
-    // No SSL used.
-    *cert_id = 0;
-    *cert_status = 0;
-    *security_bits = -1;
-    return false;
-  }
+  NavigationEntry::SSLStatus original_ssl_status = entry->ssl();  // Copy!
 
-  Pickle pickle(state.data(), static_cast<int>(state.size()));
-  void * iter = NULL;
-  return pickle.ReadInt(&iter, cert_id) &&
-         pickle.ReadInt(&iter, cert_status) &&
-         pickle.ReadInt(&iter, security_bits);
-}
+  policy()->UpdateEntry(entry, controller_->tab_contents());
 
-// static
-std::wstring SSLManager::GetEVCertName(const net::X509Certificate& cert) {
-  // EV are required to have an organization name and country.
-  if (cert.subject().organization_names.empty() ||
-      cert.subject().country_name.empty()) {
-    NOTREACHED();
-    return std::wstring();
+  if (!entry->ssl().Equals(original_ssl_status)) {
+    NotificationService::current()->Notify(
+        NotificationType::SSL_VISIBLE_STATE_CHANGED,
+        Source<NavigationController>(controller_),
+        NotificationService::NoDetails());
   }
-
-  return l10n_util::GetStringF(IDS_SECURE_CONNECTION_EV,
-                               UTF8ToWide(cert.subject().organization_names[0]),
-                               UTF8ToWide(cert.subject().country_name));
 }