Componentize SSLErrorNavigationThrottle for reuse by WebLayer

This CL componentizes ssl_error_navigation_throttle.* in preparation
for using it within the implementation of SSL interstitials for
WebLayer. The only meaningful change is to move the concrete detection
of being within hosted apps to be implementation within
chrome_content_browser_client.cc and passed to
SSLErrorNavigationThrottle as a callback.

This CL leaves out the following, which we will undertake as followup:
- Componentization of the unittest. This requires extra work, notably
  the componentization of SSLBlockingPage
  (which we will be undertaking).
- Putting the moved files into the security_interstitials into their
  own namespace. This would just serve to add noise to this CL.

The (internal-only) design doc for this effort is here:
https://docs.google.com/document/d/1ab_zZ6TF6tMZE54IJ2dz8LSwsIp6XEwky9yzH7WPFqk/edit#

Code authored by blundell@chromium.org

Change-Id: I2e410f733cbdd7ed41bdbac0318f9dfe8d9c17a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1880032
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Carlos IL <carlosil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710171}

chrome/browser/BUILD.gn

@@ -1679,7 +1679,6 @@ jumbo_static_library("browser") {
     "ssl/ssl_blocking_page.h",
     "ssl/ssl_blocking_page_base.cc",
     "ssl/ssl_blocking_page_base.h",
-    "ssl/ssl_cert_reporter.h",
     "ssl/ssl_client_auth_metrics.cc",
     "ssl/ssl_client_auth_metrics.h",
     "ssl/ssl_client_certificate_selector.h",
@@ -1691,8 +1690,6 @@ jumbo_static_library("browser") {
     "ssl/ssl_error_controller_client.h",
     "ssl/ssl_error_handler.cc",
     "ssl/ssl_error_handler.h",
-    "ssl/ssl_error_navigation_throttle.cc",
-    "ssl/ssl_error_navigation_throttle.h",
     "ssl/tls_deprecation_config.cc",
     "ssl/tls_deprecation_config.h",
     "ssl/typed_navigation_timing_throttle.cc",

chrome/browser/chrome_content_browser_client.cc

@@ -127,11 +127,9 @@
 #include "chrome/browser/speech/chrome_speech_recognition_manager_delegate.h"
 #include "chrome/browser/speech/tts_controller_delegate_impl.h"
 #include "chrome/browser/ssl/ssl_blocking_page.h"
-#include "chrome/browser/ssl/ssl_cert_reporter.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"
-#include "chrome/browser/ssl/ssl_error_navigation_throttle.h"
 #include "chrome/browser/ssl/typed_navigation_timing_throttle.h"
 #include "chrome/browser/subresource_filter/chrome_subresource_filter_client.h"
 #include "chrome/browser/sync_file_system/local/sync_file_system_backend.h"
@@ -239,6 +237,8 @@
 #include "components/safe_browsing/features.h"
 #include "components/safe_browsing/password_protection/password_protection_navigation_throttle.h"
 #include "components/security_interstitials/content/origin_policy_ui.h"
+#include "components/security_interstitials/content/ssl_cert_reporter.h"
+#include "components/security_interstitials/content/ssl_error_navigation_throttle.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
 #include "components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h"
 #include "components/translate/core/common/translate_switches.h"
@@ -510,6 +510,7 @@
 #include "chrome/browser/extensions/user_script_listener.h"
 #include "chrome/browser/media/cast_transport_host_filter.h"
 #include "chrome/browser/speech/extension_api/tts_engine_extension_api.h"
+#include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/web_applications/app_browser_controller.h"
 #include "components/guest_view/browser/guest_view_base.h"
 #include "components/guest_view/browser/guest_view_manager.h"
@@ -1042,6 +1043,17 @@ void MaybeAddThrottle(
     throttles->push_back(std::move(maybe_throttle));
 }
 
+// Returns whether |web_contents| is within a hosted app.
+bool IsInHostedApp(WebContents* web_contents) {
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+  Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
+  return (browser &&
+          web_app::AppBrowserController::IsForWebAppBrowser(browser));
+#else
+  return false;
+#endif
+}
+
 }  // namespace
 
 std::string GetUserAgent() {
@@ -3897,7 +3909,8 @@ ChromeContentBrowserClient::CreateThrottlesForNavigation(
   throttles.push_back(std::make_unique<SSLErrorNavigationThrottle>(
       handle,
       std::make_unique<CertificateReportingServiceCertReporter>(web_contents),
-      base::Bind(&SSLErrorHandler::HandleSSLError)));
+      base::Bind(&SSLErrorHandler::HandleSSLError),
+      base::Bind(&IsInHostedApp)));
 
   throttles.push_back(std::make_unique<LoginNavigationThrottle>(handle));
 

chrome/browser/ssl/bad_clock_blocking_page.cc

@@ -12,9 +12,9 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_preferences_util.h"
 #include "chrome/browser/ssl/cert_report_helper.h"
-#include "chrome/browser/ssl/ssl_cert_reporter.h"
 #include "chrome/browser/ssl/ssl_error_controller_client.h"
 #include "components/safe_browsing/common/safe_browsing_prefs.h"
+#include "components/security_interstitials/content/ssl_cert_reporter.h"
 #include "components/security_interstitials/core/bad_clock_ui.h"
 #include "components/security_interstitials/core/metrics_helper.h"
 #include "content/public/browser/interstitial_page.h"

chrome/browser/ssl/bad_clock_blocking_page.h

@@ -12,7 +12,7 @@
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "chrome/browser/ssl/ssl_blocking_page_base.h"
-#include "chrome/browser/ssl/ssl_cert_reporter.h"
+#include "components/security_interstitials/content/ssl_cert_reporter.h"
 #include "components/ssl_errors/error_classification.h"
 #include "content/public/browser/certificate_request_result_type.h"
 #include "net/ssl/ssl_info.h"

chrome/browser/ssl/captive_portal_blocking_page.cc

@@ -18,11 +18,11 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ssl/cert_report_helper.h"
 #include "chrome/browser/ssl/certificate_error_reporter.h"
-#include "chrome/browser/ssl/ssl_cert_reporter.h"
 #include "chrome/browser/ssl/ssl_error_controller_client.h"
 #include "components/captive_portal/captive_portal_detector.h"
 #include "components/captive_portal/captive_portal_metrics.h"
 #include "components/safe_browsing/common/safe_browsing_prefs.h"
+#include "components/security_interstitials/content/ssl_cert_reporter.h"
 #include "components/security_interstitials/core/controller_client.h"
 #include "components/security_interstitials/core/metrics_helper.h"
 #include "components/strings/grit/components_strings.h"

chrome/browser/ssl/captive_portal_blocking_page_browsertest.cc

@@ -23,7 +23,6 @@
 #include "chrome/browser/ssl/cert_report_helper.h"
 #include "chrome/browser/ssl/certificate_reporting_test_utils.h"
 #include "chrome/browser/ssl/security_state_tab_helper.h"
-#include "chrome/browser/ssl/ssl_cert_reporter.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -34,6 +33,7 @@
 #include "components/captive_portal/captive_portal_detector.h"
 #include "components/prefs/pref_service.h"
 #include "components/security_interstitials/content/security_interstitial_tab_helper.h"
+#include "components/security_interstitials/content/ssl_cert_reporter.h"
 #include "components/security_state/core/security_state.h"
 #include "components/variations/variations_params_manager.h"
 #include "content/public/browser/browser_task_traits.h"

chrome/browser/ssl/cert_report_helper.cc

@@ -17,11 +17,11 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_process_platform_part.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ssl/ssl_cert_reporter.h"
 #include "chrome/common/channel_info.h"
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_service.h"
 #include "components/safe_browsing/common/safe_browsing_prefs.h"
+#include "components/security_interstitials/content/ssl_cert_reporter.h"
 #include "components/security_interstitials/core/controller_client.h"
 #include "components/security_interstitials/core/metrics_helper.h"
 #include "components/strings/grit/components_strings.h"

chrome/browser/ssl/certificate_reporting_test_utils.h

@@ -12,7 +12,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/ssl/cert_logger.pb.h"
 #include "chrome/browser/ssl/certificate_error_reporter.h"
-#include "chrome/browser/ssl/ssl_cert_reporter.h"
+#include "components/security_interstitials/content/ssl_cert_reporter.h"
 
 #if !defined(OS_ANDROID)
 class Browser;

chrome/browser/ssl/mitm_software_blocking_page.cc

@@ -10,9 +10,9 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_preferences_util.h"
 #include "chrome/browser/ssl/cert_report_helper.h"
-#include "chrome/browser/ssl/ssl_cert_reporter.h"
 #include "chrome/browser/ssl/ssl_error_controller_client.h"
 #include "components/safe_browsing/common/safe_browsing_prefs.h"
+#include "components/security_interstitials/content/ssl_cert_reporter.h"
 #include "components/security_interstitials/core/metrics_helper.h"
 #include "components/security_interstitials/core/mitm_software_ui.h"
 #include "content/public/browser/interstitial_page.h"

chrome/browser/ssl/mitm_software_blocking_page.h

@@ -11,7 +11,7 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "chrome/browser/ssl/ssl_blocking_page_base.h"
-#include "chrome/browser/ssl/ssl_cert_reporter.h"
+#include "components/security_interstitials/content/ssl_cert_reporter.h"
 #include "components/ssl_errors/error_classification.h"
 #include "content/public/browser/certificate_request_result_type.h"
 #include "net/ssl/ssl_info.h"

chrome/browser/ssl/ssl_blocking_page.cc

@@ -20,10 +20,10 @@
 #include "chrome/browser/ssl/cert_report_helper.h"
 #include "chrome/browser/ssl/chrome_ssl_host_state_delegate.h"
 #include "chrome/browser/ssl/chrome_ssl_host_state_delegate_factory.h"
-#include "chrome/browser/ssl/ssl_cert_reporter.h"
 #include "chrome/browser/ssl/ssl_error_controller_client.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/safe_browsing/common/safe_browsing_prefs.h"
+#include "components/security_interstitials/content/ssl_cert_reporter.h"
 #include "components/security_interstitials/core/controller_client.h"
 #include "components/security_interstitials/core/metrics_helper.h"
 #include "components/security_interstitials/core/ssl_error_ui.h"

chrome/browser/ssl/ssl_blocking_page.h

@@ -15,8 +15,8 @@
 #include "base/time/time.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ssl/ssl_blocking_page_base.h"
-#include "chrome/browser/ssl/ssl_cert_reporter.h"
 #include "components/security_interstitials/content/security_interstitial_page.h"
+#include "components/security_interstitials/content/ssl_cert_reporter.h"
 #include "content/public/browser/certificate_request_result_type.h"
 #include "extensions/buildflags/buildflags.h"
 #include "net/ssl/ssl_info.h"

chrome/browser/ssl/ssl_blocking_page_base.cc

@@ -6,8 +6,8 @@
 
 #include "chrome/browser/interstitials/enterprise_util.h"
 #include "chrome/browser/ssl/cert_report_helper.h"
-#include "chrome/browser/ssl/ssl_cert_reporter.h"
 #include "components/security_interstitials/content/security_interstitial_controller_client.h"
+#include "components/security_interstitials/content/ssl_cert_reporter.h"
 
 SSLBlockingPageBase::SSLBlockingPageBase(
     content::WebContents* web_contents,

chrome/browser/ssl/ssl_error_handler.cc

@@ -29,13 +29,13 @@
 #include "chrome/browser/ssl/captive_portal_helper.h"
 #include "chrome/browser/ssl/mitm_software_blocking_page.h"
 #include "chrome/browser/ssl/ssl_blocking_page.h"
-#include "chrome/browser/ssl/ssl_cert_reporter.h"
 #include "chrome/browser/ssl/ssl_error_assistant.h"
 #include "chrome/common/buildflags.h"
 #include "chrome/common/pref_names.h"
 #include "components/network_time/network_time_tracker.h"
 #include "components/prefs/pref_service.h"
 #include "components/security_interstitials/content/security_interstitial_page.h"
+#include "components/security_interstitials/content/ssl_cert_reporter.h"
 #include "components/security_interstitials/core/ssl_error_ui.h"
 #include "components/ssl_errors/error_classification.h"
 #include "components/ssl_errors/error_info.h"

chrome/browser/ssl/ssl_error_handler.h

@@ -15,9 +15,9 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ssl/common_name_mismatch_handler.h"
-#include "chrome/browser/ssl/ssl_cert_reporter.h"
 #include "chrome/browser/ssl/ssl_error_assistant.pb.h"
 #include "components/security_interstitials/content/security_interstitial_page.h"
+#include "components/security_interstitials/content/ssl_cert_reporter.h"
 #include "components/ssl_errors/error_classification.h"
 #include "content/public/browser/certificate_request_result_type.h"
 #include "content/public/browser/notification_observer.h"

chrome/browser/ssl/ssl_error_navigation_throttle_unittest.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/ssl/ssl_error_navigation_throttle.h"
+#include "components/security_interstitials/content/ssl_error_navigation_throttle.h"
 
 #include "base/bind.h"
 #include "base/run_loop.h"
@@ -43,8 +43,11 @@ void MockHandleSSLError(
   } else {
     std::move(blocking_page_ready_callback).Run(std::move(blocking_page));
   }
+}
 
-}  // namespace
+bool IsInHostedApp(content::WebContents* web_contents) {
+  return false;
+}
 
 class TestSSLErrorNavigationThrottle : public SSLErrorNavigationThrottle {
  public:
@@ -60,7 +63,8 @@ class TestSSLErrorNavigationThrottle : public SSLErrorNavigationThrottle {
                                     const chrome_browser_ssl::
                                         CertLoggerRequest_ChromeChannel)>(),
                 certificate_reporting_test_utils::CERT_REPORT_NOT_EXPECTED),
-            base::Bind(&MockHandleSSLError, async_handle_ssl_error)),
+            base::Bind(&MockHandleSSLError, async_handle_ssl_error),
+            base::Bind(&IsInHostedApp)),
         on_cancel_deferred_navigation_(
             std::move(on_cancel_deferred_navigation)) {}
 

components/security_interstitials/content/BUILD.gn

@@ -16,6 +16,9 @@ static_library("security_interstitial_page") {
     "security_interstitial_page.h",
     "security_interstitial_tab_helper.cc",
     "security_interstitial_tab_helper.h",
+    "ssl_cert_reporter.h",
+    "ssl_error_navigation_throttle.cc",
+    "ssl_error_navigation_throttle.h",
     "unsafe_resource.cc",
     "unsafe_resource.h",
     "urls.cc",

chrome/browser/ssl/ssl_cert_reporter.h → components/security_interstitials/content/ssl_cert_reporter.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_SSL_SSL_CERT_REPORTER_H_
-#define CHROME_BROWSER_SSL_SSL_CERT_REPORTER_H_
+#ifndef COMPONENTS_SECURITY_INTERSTITIALS_CONTENT_SSL_CERT_REPORTER_H_
+#define COMPONENTS_SECURITY_INTERSTITIALS_CONTENT_SSL_CERT_REPORTER_H_
 
 #include <string>
 
@@ -19,4 +19,4 @@ class SSLCertReporter {
       const std::string& serialized_report) = 0;
 };
 
-#endif  // CHROME_BROWSER_SSL_SSL_CERT_REPORTER_H_
+#endif  // COMPONENTS_SECURITY_INTERSTITIALS_CONTENT_SSL_CERT_REPORTER_H_

chrome/browser/ssl/ssl_error_navigation_throttle.cc → components/security_interstitials/content/ssl_error_navigation_throttle.cc

@@ -2,32 +2,27 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/ssl/ssl_error_navigation_throttle.h"
+#include "components/security_interstitials/content/ssl_error_navigation_throttle.h"
 
 #include "base/bind.h"
 #include "base/feature_list.h"
 #include "base/threading/thread_task_runner_handle.h"
-#include "chrome/common/chrome_features.h"
+#include "build/buildflag.h"
 #include "components/security_interstitials/content/security_interstitial_page.h"
 #include "components/security_interstitials/content/security_interstitial_tab_helper.h"
 #include "content/public/browser/navigation_handle.h"
 #include "net/cert/cert_status_flags.h"
 
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-#include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/browser_commands.h"
-#include "chrome/browser/ui/browser_finder.h"
-#include "chrome/browser/ui/web_applications/app_browser_controller.h"
-#endif  // BUILDFLAG(ENABLE_EXTENSIONS)
-
 SSLErrorNavigationThrottle::SSLErrorNavigationThrottle(
     content::NavigationHandle* navigation_handle,
     std::unique_ptr<SSLCertReporter> ssl_cert_reporter,
     SSLErrorNavigationThrottle::HandleSSLErrorCallback
-        handle_ssl_error_callback)
+        handle_ssl_error_callback,
+    IsInHostedAppCallback is_in_hosted_app_callback)
     : content::NavigationThrottle(navigation_handle),
       ssl_cert_reporter_(std::move(ssl_cert_reporter)),
-      handle_ssl_error_callback_(std::move(handle_ssl_error_callback)) {}
+      handle_ssl_error_callback_(std::move(handle_ssl_error_callback)),
+      is_in_hosted_app_callback_(std::move(is_in_hosted_app_callback)) {}
 
 SSLErrorNavigationThrottle::~SSLErrorNavigationThrottle() {}
 
@@ -76,15 +71,12 @@ SSLErrorNavigationThrottle::WillProcessResponse() {
     return content::NavigationThrottle::PROCEED;
   }
 
-#if BUILDFLAG(ENABLE_EXTENSIONS)
   // Hosted Apps should not be allowed to run if there is a problem with their
   // certificate. So, when a user tries to open such an app, we show an
   // interstitial, even if the user has previously clicked through one. Clicking
   // through the interstitial will continue the navigation in a regular browser
   // window.
-  Browser* browser =
-      chrome::FindBrowserWithWebContents(handle->GetWebContents());
-  if (browser && web_app::AppBrowserController::IsForWebAppBrowser(browser)) {
+  if (std::move(is_in_hosted_app_callback_).Run(handle->GetWebContents())) {
     QueueShowInterstitial(
         std::move(handle_ssl_error_callback_), handle->GetWebContents(),
         // The navigation handle's net error code will be
@@ -96,7 +88,6 @@ SSLErrorNavigationThrottle::WillProcessResponse() {
     return content::NavigationThrottle::ThrottleCheckResult(
         content::NavigationThrottle::DEFER);
   }
-#endif  // BUILDFLAG(ENABLE_EXTENSIONS)
 
   return content::NavigationThrottle::PROCEED;
 }

chrome/browser/ssl/ssl_error_navigation_throttle.h → components/security_interstitials/content/ssl_error_navigation_throttle.h

@@ -2,19 +2,22 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_SSL_SSL_ERROR_NAVIGATION_THROTTLE_H_
-#define CHROME_BROWSER_SSL_SSL_ERROR_NAVIGATION_THROTTLE_H_
+#ifndef COMPONENTS_SECURITY_INTERSTITIALS_CONTENT_SSL_ERROR_NAVIGATION_THROTTLE_H_
+#define COMPONENTS_SECURITY_INTERSTITIALS_CONTENT_SSL_ERROR_NAVIGATION_THROTTLE_H_
 
 #include <memory>
 
 #include "base/memory/weak_ptr.h"
-#include "chrome/browser/ssl/ssl_error_handler.h"
+#include "components/security_interstitials/content/ssl_cert_reporter.h"
+#include "content/public/browser/certificate_request_result_type.h"
 #include "content/public/browser/navigation_throttle.h"
+#include "net/ssl/ssl_info.h"
 
 class SSLCertReporter;
 
 namespace content {
 class NavigationHandle;
+class WebContents;
 }  // namespace content
 
 namespace security_interstitials {
@@ -42,10 +45,20 @@ class SSLErrorNavigationThrottle : public content::NavigationThrottle {
           blocking_page_ready_callback)>
       HandleSSLErrorCallback;
 
+  // Returns whether |web_contents| is in the context of a hosted app, as the
+  // logic of when to display interstitials for SSL errors is specialized for
+  // hosted apps. This is exposed as a callback because although the WebContents
+  // is known at the time of creating SSLErrorNavigationThrottle, it may not
+  // have been inserted into a browser by the time the navigation begins. See
+  // browser_navigator.cc.
+  typedef base::OnceCallback<bool(content::WebContents* web_contents)>
+      IsInHostedAppCallback;
+
   explicit SSLErrorNavigationThrottle(
       content::NavigationHandle* handle,
       std::unique_ptr<SSLCertReporter> ssl_cert_reporter,
-      HandleSSLErrorCallback handle_ssl_error_callback);
+      HandleSSLErrorCallback handle_ssl_error_callback,
+      IsInHostedAppCallback is_in_hosted_app_callback);
   ~SSLErrorNavigationThrottle() override;
 
   // content::NavigationThrottle:
@@ -69,7 +82,8 @@ class SSLErrorNavigationThrottle : public content::NavigationThrottle {
 
   std::unique_ptr<SSLCertReporter> ssl_cert_reporter_;
   HandleSSLErrorCallback handle_ssl_error_callback_;
+  IsInHostedAppCallback is_in_hosted_app_callback_;
   base::WeakPtrFactory<SSLErrorNavigationThrottle> weak_ptr_factory_{this};
 };
 
-#endif  // CHROME_BROWSER_SSL_SSL_ERROR_NAVIGATION_THROTTLE_H_
+#endif  // COMPONENTS_SECURITY_INTERSTITIALS_CONTENT_SSL_ERROR_NAVIGATION_THROTTLE_H_