Add custom interstitial for captive portals.

chrome/browser/chrome_content_browser_client.cc

Index: chrome/browser/chrome_content_browser_client.cc
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index f57d75c04f098c77d5fe3bdf545caba967341b31..ad2b11a77b1fd854bc9958b64e5791118397578d 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -70,6 +70,8 @@
 #include "chrome/browser/ssl/ssl_add_certificate.h"
 #include "chrome/browser/ssl/ssl_blocking_page.h"
 #include "chrome/browser/ssl/ssl_client_certificate_selector.h"
+#include "chrome/browser/ssl/ssl_error_handler.h"
+#include "chrome/browser/sync_file_system/local/sync_file_system_backend.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/ui/blocked_content/blocked_window_params.h"
 #include "chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h"
@@ -1722,13 +1724,6 @@ void ChromeContentBrowserClient::AllowCertificateError(
     return;
   }
 
-#if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
-  CaptivePortalTabHelper* captive_portal_tab_helper =
-      CaptivePortalTabHelper::FromWebContents(tab);
-  if (captive_portal_tab_helper)
-    captive_portal_tab_helper->OnSSLCertError(ssl_info);
-#endif
-
   // Otherwise, display an SSL blocking page. The interstitial page takes
   // ownership of ssl_blocking_page.
   int options_mask = 0;
@@ -1738,9 +1733,24 @@ void ChromeContentBrowserClient::AllowCertificateError(
     options_mask |= SSLBlockingPage::STRICT_ENFORCEMENT;
   if (expired_previous_decision)
     options_mask |= SSLBlockingPage::EXPIRED_BUT_PREVIOUSLY_ALLOWED;
-  SSLBlockingPage* ssl_blocking_page = new SSLBlockingPage(
-      tab, cert_error, ssl_info, request_url, options_mask, callback);
-  ssl_blocking_page->Show();
+
+  // SSL interstitials aren't delayed if captive portal detection is disabled.
+  base::TimeDelta ssl_error_delay;
+#if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
+  CaptivePortalTabHelper* captive_portal_tab_helper =
+      CaptivePortalTabHelper::FromWebContents(tab);
+  if (captive_portal_tab_helper) {
+    captive_portal_tab_helper->OnSSLCertError(ssl_info);
+    ssl_error_delay = captive_portal_tab_helper->GetSSLErrorDelay();
+  }
+#endif

[mmenke, 2014/10/30 19:28:01]

I think this code makes much more sense in SSLErrorHandler, than in a 2600-line
catch-all class.

If this is because you don't want tests to depend on CaptivePortalTabHelper,
there are other options.

[meacer, 2014/11/06 21:21:55]

Moved to SSLErrorHandler. The tests depend on CaptivePortalTabHelper to be able
to set the delay value.

+
+  SSLErrorHandler* ssl_error_handler = new SSLErrorHandler(

[mmenke, 2014/10/30 19:28:01]

Suggest making this a static function, since nothing else event needs to touch
the handler.

SSLErrorHandler::HandleSSLError(blah);  or
SSLErrorHandler::ShowInterstatial(blah);

[meacer, 2014/11/06 21:21:55]

Done.

+      tab, cert_error, ssl_info, request_url,
+      options_mask,
+      ssl_error_delay,
+      callback);

[mmenke, 2014/10/30 19:28:01]

nit:  Should either fit args in as few lines as possible, or put one on each
line, unless you have some particular reason for not doing so.

[meacer, 2014/11/06 21:21:55]

Done.

+  ssl_error_handler->Handle();
 }
 
 void ChromeContentBrowserClient::SelectClientCertificate(