[SSL] Have SSLErrorHandler own ChromeSecurityBlockingPageFactory object

As one of the last stages of componentizing SSLErrorHandler for sharing
with WebLayer, this CL starts the process of eliminating
SSLErrorHandler's knowledge of ChromeSecurityBlockingPageFactory. To
do so, it turns ChromeSecurityBlockingPageFactory's static methods into
instance methods and has SSLErrorHandler(DelegateImpl) own an instance
of ChromeSecurityBlockingPageFactory. The next CL will introduce a
ChromeSecurityBlockingPageFactory interface and have SSLErrorHandler
talk in terms of that rather than in terms of the //chrome-level
implementation.

Bug: 1030692
Change-Id: I42f337c9f3141cd23d46d370e0dfba065e95a6bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2019242
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Carlos IL <carlosil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735347}

chrome/browser/chrome_content_browser_client.cc

@@ -132,6 +132,7 @@
 #include "chrome/browser/site_isolation/site_isolation_policy.h"
 #include "chrome/browser/speech/chrome_speech_recognition_manager_delegate.h"
 #include "chrome/browser/speech/tts_controller_delegate_impl.h"
+#include "chrome/browser/ssl/chrome_security_blocking_page_factory.h"
 #include "chrome/browser/ssl/ssl_client_auth_metrics.h"
 #include "chrome/browser/ssl/ssl_client_certificate_selector.h"
 #include "chrome/browser/ssl/ssl_error_handler.h"
@@ -698,6 +699,7 @@ void HandleSSLErrorWrapper(
       web_contents, cert_error, ssl_info, request_url,
       std::move(ssl_cert_reporter), std::move(blocking_page_ready_callback),
       g_browser_process->network_time_tracker(), captive_portal_service,
+      std::make_unique<ChromeSecurityBlockingPageFactory>(),
       profile->GetPrefs()->GetBoolean(prefs::kSSLErrorOverrideAllowed));
 }
 

chrome/browser/ssl/captive_portal_blocking_page_browsertest.cc

@@ -120,8 +120,9 @@ CaptivePortalTestingNavigationThrottle::WillFailRequest() {
   ssl_info.cert =
       net::ImportCertFromFile(net::GetTestCertsDirectory(), "ok_cert.pem");
   ssl_info.cert_status = net::CERT_STATUS_COMMON_NAME_INVALID;
+  ChromeSecurityBlockingPageFactory blocking_page_factory;
   CaptivePortalBlockingPage* blocking_page =
-      ChromeSecurityBlockingPageFactory::CreateCaptivePortalBlockingPage(
+      blocking_page_factory.CreateCaptivePortalBlockingPage(
           navigation_handle()->GetWebContents(), GURL(kBrokenSSL), login_url_,
           std::move(ssl_cert_reporter_), ssl_info,
           net::ERR_CERT_COMMON_NAME_INVALID);
@@ -423,8 +424,9 @@ class CaptivePortalBlockingPageIDNTest : public SecurityInterstitialIDNTest {
       const GURL& request_url) const override {
     net::SSLInfo empty_ssl_info;
     // Blocking page is owned by the interstitial.
+    ChromeSecurityBlockingPageFactory blocking_page_factory;
     CaptivePortalBlockingPage* blocking_page =
-        ChromeSecurityBlockingPageFactory::CreateCaptivePortalBlockingPage(
+        blocking_page_factory.CreateCaptivePortalBlockingPage(
             contents, GURL(kBrokenSSL), request_url, nullptr, empty_ssl_info,
             net::ERR_CERT_COMMON_NAME_INVALID);
     blocking_page->OverrideWifiInfoForTesting(false, "");

chrome/browser/ssl/chrome_security_blocking_page_factory.cc

@@ -159,7 +159,6 @@ std::unique_ptr<ChromeMetricsHelper> CreateBlockedInterceptionMetricsHelper(
 
 }  // namespace
 
-// static
 SSLBlockingPage* ChromeSecurityBlockingPageFactory::CreateSSLPage(
     content::WebContents* web_contents,
     int cert_error,
@@ -222,7 +221,6 @@ SSLBlockingPage* ChromeSecurityBlockingPageFactory::CreateSSLPage(
   return page.release();
 }
 
-// static
 CaptivePortalBlockingPage*
 ChromeSecurityBlockingPageFactory::CreateCaptivePortalBlockingPage(
     content::WebContents* web_contents,
@@ -243,7 +241,6 @@ ChromeSecurityBlockingPageFactory::CreateCaptivePortalBlockingPage(
   return page.release();
 }
 
-// static
 BadClockBlockingPage*
 ChromeSecurityBlockingPageFactory::CreateBadClockBlockingPage(
     content::WebContents* web_contents,
@@ -264,7 +261,6 @@ ChromeSecurityBlockingPageFactory::CreateBadClockBlockingPage(
   return page.release();
 }
 
-// static
 MITMSoftwareBlockingPage*
 ChromeSecurityBlockingPageFactory::CreateMITMSoftwareBlockingPage(
     content::WebContents* web_contents,
@@ -284,7 +280,6 @@ ChromeSecurityBlockingPageFactory::CreateMITMSoftwareBlockingPage(
   return page.release();
 }
 
-// static
 BlockedInterceptionBlockingPage*
 ChromeSecurityBlockingPageFactory::CreateBlockedInterceptionBlockingPage(
     content::WebContents* web_contents,

chrome/browser/ssl/chrome_security_blocking_page_factory.h

@@ -17,10 +17,17 @@
 // interstitial pages.
 class ChromeSecurityBlockingPageFactory {
  public:
+  ChromeSecurityBlockingPageFactory() = default;
+  ~ChromeSecurityBlockingPageFactory() = default;
+  ChromeSecurityBlockingPageFactory(const ChromeSecurityBlockingPageFactory&) =
+      delete;
+  ChromeSecurityBlockingPageFactory& operator=(
+      const ChromeSecurityBlockingPageFactory&) = delete;
+
   // Creates an SSL blocking page. |options_mask| must be a bitwise mask of
   // SSLErrorUI::SSLErrorOptionsMask values. The caller is responsible for
   // ownership of the returned object.
-  static SSLBlockingPage* CreateSSLPage(
+  SSLBlockingPage* CreateSSLPage(
       content::WebContents* web_contents,
       int cert_error,
       const net::SSLInfo& ssl_info,
@@ -32,7 +39,7 @@ class ChromeSecurityBlockingPageFactory {
 
   // Creates a captive portal blocking page. The caller is responsible for
   // ownership of the returned object.
-  static CaptivePortalBlockingPage* CreateCaptivePortalBlockingPage(
+  CaptivePortalBlockingPage* CreateCaptivePortalBlockingPage(
       content::WebContents* web_contents,
       const GURL& request_url,
       const GURL& login_url,
@@ -42,7 +49,7 @@ class ChromeSecurityBlockingPageFactory {
 
   // Creates a bad clock blocking page. The caller is responsible for
   // ownership of the returned object.
-  static BadClockBlockingPage* CreateBadClockBlockingPage(
+  BadClockBlockingPage* CreateBadClockBlockingPage(
       content::WebContents* web_contents,
       int cert_error,
       const net::SSLInfo& ssl_info,
@@ -53,7 +60,7 @@ class ChromeSecurityBlockingPageFactory {
 
   // Creates a man-in-the-middle software blocking page. The caller is
   // responsible for ownership of the returned object.
-  static MITMSoftwareBlockingPage* CreateMITMSoftwareBlockingPage(
+  MITMSoftwareBlockingPage* CreateMITMSoftwareBlockingPage(
       content::WebContents* web_contents,
       int cert_error,
       const GURL& request_url,
@@ -63,22 +70,20 @@ class ChromeSecurityBlockingPageFactory {
 
   // Creates a blocked interception blocking page. The caller is
   // responsible for ownership of the returned object.
-  static BlockedInterceptionBlockingPage* CreateBlockedInterceptionBlockingPage(
+  BlockedInterceptionBlockingPage* CreateBlockedInterceptionBlockingPage(
       content::WebContents* web_contents,
       int cert_error,
       const GURL& request_url,
       std::unique_ptr<SSLCertReporter> ssl_cert_reporter,
       const net::SSLInfo& ssl_info);
 
-  // Does setup on |page| that is specific to the client (Chrome).
-  static void DoChromeSpecificSetup(SSLBlockingPageBase* page);
-
   // Overrides the calculation of whether the app is enterprise-managed for
   // tests.
   static void SetEnterpriseManagedForTesting(bool enterprise_managed);
 
  private:
-  DISALLOW_IMPLICIT_CONSTRUCTORS(ChromeSecurityBlockingPageFactory);
+  // Does setup on |page| that is specific to the client (Chrome).
+  static void DoChromeSpecificSetup(SSLBlockingPageBase* page);
 };
 
 #endif  // CHROME_BROWSER_SSL_CHROME_SECURITY_BLOCKING_PAGE_FACTORY_H_

chrome/browser/ssl/ssl_blocking_page_unittest.cc

@@ -58,12 +58,12 @@ TEST_F(SSLBlockingPageTest,
   ssl_info.cert_status = net::CERT_STATUS_DATE_INVALID;
 
   // Simulates the showing of a SSL blocking page.
-  SSLBlockingPage* blocking_page =
-      ChromeSecurityBlockingPageFactory::CreateSSLPage(
-          web_contents(), net::ERR_CERT_DATE_INVALID, ssl_info, request_url,
-          /*options_mask=*/0, base::Time::NowFromSystemTime(),
-          /*support_url=*/GURL(),
-          /*ssl_cert_reporter=*/nullptr);
+  ChromeSecurityBlockingPageFactory blocking_page_factory;
+  SSLBlockingPage* blocking_page = blocking_page_factory.CreateSSLPage(
+      web_contents(), net::ERR_CERT_DATE_INVALID, ssl_info, request_url,
+      /*options_mask=*/0, base::Time::NowFromSystemTime(),
+      /*support_url=*/GURL(),
+      /*ssl_cert_reporter=*/nullptr);
   blocking_page->DontCreateViewForTesting();
   blocking_page->Show();
 

chrome/browser/ssl/ssl_browsertest.cc

@@ -5343,7 +5343,8 @@ class SSLBlockingPageIDNTest
     net::SSLInfo ssl_info;
     ssl_info.cert =
         net::ImportCertFromFile(net::GetTestCertsDirectory(), "ok_cert.pem");
-    return ChromeSecurityBlockingPageFactory::CreateSSLPage(
+    ChromeSecurityBlockingPageFactory blocking_page_factory;
+    return blocking_page_factory.CreateSSLPage(
         contents, net::ERR_CERT_CONTAINS_ERRORS, ssl_info, request_url, 0,
         base::Time::NowFromSystemTime(), GURL(), nullptr);
   }

chrome/browser/ssl/ssl_error_handler.cc

@@ -347,6 +347,7 @@ class SSLErrorHandlerDelegateImpl : public SSLErrorHandler::Delegate {
       const GURL& request_url,
       std::unique_ptr<SSLCertReporter> ssl_cert_reporter,
       CaptivePortalService* captive_portal_service,
+      std::unique_ptr<ChromeSecurityBlockingPageFactory> blocking_page_factory,
       SSLErrorHandler::OnBlockingPageShownCallback
           on_blocking_page_shown_callback,
       SSLErrorHandler::BlockingPageReadyCallback blocking_page_ready_callback)
@@ -360,6 +361,7 @@ class SSLErrorHandlerDelegateImpl : public SSLErrorHandler::Delegate {
 #if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
         captive_portal_service_(captive_portal_service),
 #endif
+        blocking_page_factory_(std::move(blocking_page_factory)),
         on_blocking_page_shown_callback_(on_blocking_page_shown_callback),
         blocking_page_ready_callback_(std::move(blocking_page_ready_callback)) {
     DCHECK(!blocking_page_ready_callback_.is_null());
@@ -403,6 +405,7 @@ class SSLErrorHandlerDelegateImpl : public SSLErrorHandler::Delegate {
 #if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
   CaptivePortalService* captive_portal_service_;
 #endif
+  std::unique_ptr<ChromeSecurityBlockingPageFactory> blocking_page_factory_;
   SSLErrorHandler::OnBlockingPageShownCallback on_blocking_page_shown_callback_;
   SSLErrorHandler::BlockingPageReadyCallback blocking_page_ready_callback_;
 };
@@ -463,24 +466,22 @@ bool SSLErrorHandlerDelegateImpl::IsErrorOverridable() const {
 void SSLErrorHandlerDelegateImpl::ShowCaptivePortalInterstitial(
     const GURL& landing_url) {
   // Show captive portal blocking page. The interstitial owns the blocking page.
-  OnBlockingPageReady(
-      ChromeSecurityBlockingPageFactory::CreateCaptivePortalBlockingPage(
-          web_contents_, request_url_, landing_url,
-          std::move(ssl_cert_reporter_), ssl_info_, cert_error_));
+  OnBlockingPageReady(blocking_page_factory_->CreateCaptivePortalBlockingPage(
+      web_contents_, request_url_, landing_url, std::move(ssl_cert_reporter_),
+      ssl_info_, cert_error_));
 }
 
 void SSLErrorHandlerDelegateImpl::ShowMITMSoftwareInterstitial(
     const std::string& mitm_software_name) {
   // Show MITM software blocking page. The interstitial owns the blocking page.
-  OnBlockingPageReady(
-      ChromeSecurityBlockingPageFactory::CreateMITMSoftwareBlockingPage(
-          web_contents_, cert_error_, request_url_,
-          std::move(ssl_cert_reporter_), ssl_info_, mitm_software_name));
+  OnBlockingPageReady(blocking_page_factory_->CreateMITMSoftwareBlockingPage(
+      web_contents_, cert_error_, request_url_, std::move(ssl_cert_reporter_),
+      ssl_info_, mitm_software_name));
 }
 
 void SSLErrorHandlerDelegateImpl::ShowSSLInterstitial(const GURL& support_url) {
   // Show SSL blocking page. The interstitial owns the blocking page.
-  OnBlockingPageReady(ChromeSecurityBlockingPageFactory::CreateSSLPage(
+  OnBlockingPageReady(blocking_page_factory_->CreateSSLPage(
       web_contents_, cert_error_, ssl_info_, request_url_, options_mask_,
       base::Time::NowFromSystemTime(), support_url,
       std::move(ssl_cert_reporter_)));
@@ -490,16 +491,15 @@ void SSLErrorHandlerDelegateImpl::ShowBadClockInterstitial(
     const base::Time& now,
     ssl_errors::ClockState clock_state) {
   // Show bad clock page. The interstitial owns the blocking page.
-  OnBlockingPageReady(
-      ChromeSecurityBlockingPageFactory::CreateBadClockBlockingPage(
-          web_contents_, cert_error_, ssl_info_, request_url_, now, clock_state,
-          std::move(ssl_cert_reporter_)));
+  OnBlockingPageReady(blocking_page_factory_->CreateBadClockBlockingPage(
+      web_contents_, cert_error_, ssl_info_, request_url_, now, clock_state,
+      std::move(ssl_cert_reporter_)));
 }
 
 void SSLErrorHandlerDelegateImpl::ShowBlockedInterceptionInterstitial() {
   // Show interception blocking page. The interstitial owns the blocking page.
   OnBlockingPageReady(
-      ChromeSecurityBlockingPageFactory::CreateBlockedInterceptionBlockingPage(
+      blocking_page_factory_->CreateBlockedInterceptionBlockingPage(
           web_contents_, cert_error_, request_url_,
           std::move(ssl_cert_reporter_), ssl_info_));
 }
@@ -549,6 +549,7 @@ void SSLErrorHandler::HandleSSLError(
         blocking_page_ready_callback,
     network_time::NetworkTimeTracker* network_time_tracker,
     CaptivePortalService* captive_portal_service,
+    std::unique_ptr<ChromeSecurityBlockingPageFactory> blocking_page_factory,
     bool user_can_proceed_past_interstitial /*=true*/) {
   DCHECK(!FromWebContents(web_contents));
 
@@ -563,6 +564,7 @@ void SSLErrorHandler::HandleSSLError(
               web_contents, ssl_info, web_contents->GetBrowserContext(),
               cert_error, options_mask, request_url,
               std::move(ssl_cert_reporter), captive_portal_service,
+              std::move(blocking_page_factory),
               g_config.Pointer()->on_blocking_page_shown_callback(),
               std::move(blocking_page_ready_callback))),
       web_contents, cert_error, ssl_info, network_time_tracker,

chrome/browser/ssl/ssl_error_handler.h

@@ -26,6 +26,7 @@
 #include "net/ssl/ssl_info.h"
 #include "url/gurl.h"
 
+class ChromeSecurityBlockingPageFactory;
 class CommonNameMismatchHandler;
 struct DynamicInterstitialInfo;
 
@@ -150,6 +151,7 @@ class SSLErrorHandler : public content::WebContentsUserData<SSLErrorHandler>,
       BlockingPageReadyCallback blocking_page_ready_callback,
       network_time::NetworkTimeTracker* network_time_tracker,
       CaptivePortalService* captive_portal_service,
+      std::unique_ptr<ChromeSecurityBlockingPageFactory> blocking_page_factory,
       bool user_can_proceed_past_interstitial = true);
 
   // Sets the binary proto for SSL error assistant. The binary proto

chrome/browser/ui/webui/interstitials/interstitial_ui.cc

@@ -156,9 +156,10 @@ SSLBlockingPage* CreateSslBlockingPage(content::WebContents* web_contents) {
   if (strict_enforcement)
     options_mask |=
         security_interstitials::SSLErrorOptionsMask::STRICT_ENFORCEMENT;
-  return ChromeSecurityBlockingPageFactory::CreateSSLPage(
-      web_contents, cert_error, ssl_info, request_url, options_mask,
-      time_triggered_, GURL(), nullptr);
+  ChromeSecurityBlockingPageFactory blocking_page_factory;
+  return blocking_page_factory.CreateSSLPage(web_contents, cert_error, ssl_info,
+                                             request_url, options_mask,
+                                             time_triggered_, GURL(), nullptr);
 }
 
 MITMSoftwareBlockingPage* CreateMITMSoftwareBlockingPage(
@@ -174,12 +175,12 @@ MITMSoftwareBlockingPage* CreateMITMSoftwareBlockingPage(
     is_enterprise_managed = is_enterprise_managed_param == "1";
   }
 
-  ChromeSecurityBlockingPageFactory::SetEnterpriseManagedForTesting(
-      is_enterprise_managed);
+  ChromeSecurityBlockingPageFactory blocking_page_factory;
+  blocking_page_factory.SetEnterpriseManagedForTesting(is_enterprise_managed);
 
   net::SSLInfo ssl_info;
   ssl_info.cert = ssl_info.unverified_cert = CreateFakeCert();
-  return ChromeSecurityBlockingPageFactory::CreateMITMSoftwareBlockingPage(
+  return blocking_page_factory.CreateMITMSoftwareBlockingPage(
       web_contents, cert_error, request_url, nullptr, ssl_info,
       mitm_software_name);
 }
@@ -191,9 +192,9 @@ BlockedInterceptionBlockingPage* CreateBlockedInterceptionBlockingPage(
 
   net::SSLInfo ssl_info;
   ssl_info.cert = ssl_info.unverified_cert = CreateFakeCert();
-  return ChromeSecurityBlockingPageFactory::
-      CreateBlockedInterceptionBlockingPage(web_contents, cert_error,
-                                            request_url, nullptr, ssl_info);
+  ChromeSecurityBlockingPageFactory blocking_page_factory;
+  return blocking_page_factory.CreateBlockedInterceptionBlockingPage(
+      web_contents, cert_error, request_url, nullptr, ssl_info);
 }
 
 BadClockBlockingPage* CreateBadClockBlockingPage(
@@ -241,7 +242,8 @@ BadClockBlockingPage* CreateBadClockBlockingPage(
   if (strict_enforcement)
     options_mask |=
         security_interstitials::SSLErrorOptionsMask::STRICT_ENFORCEMENT;
-  return ChromeSecurityBlockingPageFactory::CreateBadClockBlockingPage(
+  ChromeSecurityBlockingPageFactory blocking_page_factory;
+  return blocking_page_factory.CreateBadClockBlockingPage(
       web_contents, cert_error, ssl_info, request_url, base::Time::Now(),
       clock_state, nullptr);
 }
@@ -401,8 +403,9 @@ CaptivePortalBlockingPage* CreateCaptivePortalBlockingPage(
   }
   net::SSLInfo ssl_info;
   ssl_info.cert = ssl_info.unverified_cert = CreateFakeCert();
+  ChromeSecurityBlockingPageFactory blocking_page_factory;
   CaptivePortalBlockingPage* blocking_page =
-      ChromeSecurityBlockingPageFactory::CreateCaptivePortalBlockingPage(
+      blocking_page_factory.CreateCaptivePortalBlockingPage(
           web_contents, request_url, landing_url, nullptr, ssl_info,
           net::ERR_CERT_COMMON_NAME_INVALID);
   blocking_page->OverrideWifiInfoForTesting(is_wifi_connection, wifi_ssid);