Don't automatically deny subresource requests that have minor

certificate errors (ERR_CERT_NO_REVOCATION_MECHANISM and
ERR_CERT_UNABLE_TO_CHECK_REVOCATION).

This requires moving the code that automatically denies
subresource requests into only the SSLPolicy subclasses
that need it.  To avoid duplicating that code in five classes,
I added convenience methods for handling overridable certificate
errors and fatal errors.

R=jcampan,abarth
Review URL: http://codereview.chromium.org/8085

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

2 files changed, 43 insertions, 26 deletions

diff --git a/chrome/browser/ssl_policy.cc b/chrome/browser/ssl_policy.cc
index 099f5c0..0fbb9eb 100644
--- a/chrome/browser/ssl_policy.cc
+++ b/chrome/browser/ssl_policy.cc
@@ -126,8 +126,7 @@ class CommonNameInvalidPolicy : public SSLPolicy {
 
   void OnCertError(const GURL& main_frame_url,
                    SSLManager::CertError* error) {
-    // We need to ask the user to approve this certificate.
-    ShowBlockingPage(this, error);
+    OnOverridableCertError(main_frame_url, error);
   }
 };
 
@@ -139,8 +138,7 @@ class DateInvalidPolicy : public SSLPolicy {
 
   void OnCertError(const GURL& main_frame_url,
                    SSLManager::CertError* error) {
-    // We need to ask the user to approve this certificate.
-    ShowBlockingPage(this, error);
+    OnOverridableCertError(main_frame_url, error);
   }
 };
 
@@ -152,8 +150,7 @@ class AuthorityInvalidPolicy : public SSLPolicy {
 
   void OnCertError(const GURL& main_frame_url,
                    SSLManager::CertError* error) {
-    // We need to ask the user to approve this certificate.
-    ShowBlockingPage(this, error);
+    OnOverridableCertError(main_frame_url, error);
   }
 };
 
@@ -165,9 +162,7 @@ class ContainsErrorsPolicy : public SSLPolicy {
 
   void OnCertError(const GURL& main_frame_url,
                    SSLManager::CertError* error) {
-    error->CancelRequest();
-    ShowErrorPage(this, error);
-    // No need to degrade our security indicators because we didn't continue.
+    OnFatalCertError(main_frame_url, error);
   }
 };
 
@@ -207,10 +202,7 @@ class RevokedPolicy : public SSLPolicy {
 
   void OnCertError(const GURL& main_frame_url,
                    SSLManager::CertError* error) {
-    error->CancelRequest();
-    DCHECK(error->GetTabContents()->type() == TAB_CONTENTS_WEB);
-    ShowErrorPage(this, error);
-    // No need to degrade our security indicators because we didn't continue.
+    OnFatalCertError(main_frame_url, error);
   }
 };
 
@@ -222,10 +214,7 @@ class InvalidPolicy : public SSLPolicy {
 
   void OnCertError(const GURL& main_frame_url,
                    SSLManager::CertError* error) {
-    error->CancelRequest();
-    DCHECK(error->GetTabContents()->type() == TAB_CONTENTS_WEB);
-    ShowErrorPage(this, error);
-    // No need to degrade our security indicators because we didn't continue.
+    OnFatalCertError(main_frame_url, error);
   }
 };
 
@@ -283,14 +272,6 @@ class DefaultPolicy : public SSLPolicy {
         // For now we handle the DENIED as the UNKNOWN, which means a blocking
         // page is shown to the user every time he comes back to the page.
       case net::X509Certificate::Policy::UNKNOWN:
-        if (error->resource_type() != ResourceType::MAIN_FRAME) {
-          // A sub-resource has a certificate error.  The user doesn't really
-          // have a context for making the right decision, so block the
-          // request hard, without an info bar to allow showing the insecure
-          // content.
-          error->DenyRequest();
-          break;
-        }
         // We don't know how to handle this error.  Ask our sub-policies.
         sub_policies_[index]->OnCertError(main_frame_url, error);
         break;
@@ -479,3 +460,28 @@ void SSLPolicy::OnAllowCertificate(SSLManager::CertError* error) {
                                      error->request_url().host());
 }
 
+void SSLPolicy::OnOverridableCertError(const GURL& main_frame_url,
+                                       SSLManager::CertError* error) {
+  if (error->resource_type() != ResourceType::MAIN_FRAME) {
+    // A sub-resource has a certificate error.  The user doesn't really
+    // have a context for making the right decision, so block the
+    // request hard, without an info bar to allow showing the insecure
+    // content.
+    error->DenyRequest();
+    return;
+  }
+  // We need to ask the user to approve this certificate.
+  ShowBlockingPage(this, error);
+}
+
+void SSLPolicy::OnFatalCertError(const GURL& main_frame_url,
+                                 SSLManager::CertError* error) {
+  if (error->resource_type() != ResourceType::MAIN_FRAME) {
+    error->DenyRequest();
+    return;
+  }
+  error->CancelRequest();
+  DCHECK(error->GetTabContents()->type() == TAB_CONTENTS_WEB);
+  ShowErrorPage(this, error);
+  // No need to degrade our security indicators because we didn't continue.
+}
diff --git a/chrome/browser/ssl_policy.h b/chrome/browser/ssl_policy.h
index e68a439..f3082bd 100644
--- a/chrome/browser/ssl_policy.h
+++ b/chrome/browser/ssl_policy.h
@@ -50,9 +50,20 @@ class SSLPolicy : public SSLManager::Delegate,
   // Allow our subclasses to construct us.
   SSLPolicy();
 
+  // Helper method for derived classes handling certificate errors that can be
+  // overridden by the user.
+  // Show a blocking page and let the user continue or cancel the request.
+  void OnOverridableCertError(const GURL& main_frame_url,
+                              SSLManager::CertError* error);
+
+  // Helper method for derived classes handling fatal certificate errors.
+  // Cancel the request and show an error page.
+  void OnFatalCertError(const GURL& main_frame_url,
+                        SSLManager::CertError* error);
+
  private:
   DISALLOW_EVIL_CONSTRUCTORS(SSLPolicy);
 };
 
-#endif // CHROME_BROWSER_SSL_POLICY_H__
+#endif  // CHROME_BROWSER_SSL_POLICY_H__