[subresource_filter] Move throttle insertion into the client

This patch also  makes the SB throttle pass-through if there isn't a valid
DB manager.

BUG=717590

Review-Url: https://codereview.chromium.org/2858483003
Cr-Commit-Position: refs/heads/master@{#469388}

chrome/browser/BUILD.gn

@@ -1295,8 +1295,6 @@ split_static_library("browser") {
     "storage/storage_info_fetcher.h",
     "subresource_filter/chrome_subresource_filter_client.cc",
     "subresource_filter/chrome_subresource_filter_client.h",
-    "subresource_filter/navigation_throttle_util.cc",
-    "subresource_filter/navigation_throttle_util.h",
     "subresource_filter/subresource_filter_content_settings_manager.cc",
     "subresource_filter/subresource_filter_content_settings_manager.h",
     "subresource_filter/subresource_filter_profile_context.cc",

chrome/browser/chrome_content_browser_client.cc

@@ -84,7 +84,7 @@
 #include "chrome/browser/ssl/ssl_cert_reporter.h"
 #include "chrome/browser/ssl/ssl_client_certificate_selector.h"
 #include "chrome/browser/ssl/ssl_error_handler.h"
-#include "chrome/browser/subresource_filter/navigation_throttle_util.h"
+#include "chrome/browser/subresource_filter/chrome_subresource_filter_client.h"
 #include "chrome/browser/sync_file_system/local/sync_file_system_backend.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/tracing/chrome_tracing_delegate.h"
@@ -3472,22 +3472,11 @@ ChromeContentBrowserClient::CreateThrottlesForNavigation(
   if (delay_navigation_throttle)
     throttles.push_back(std::move(delay_navigation_throttle));
 
-  content::NavigationThrottle* subresource_filter_activation_throttle =
-      MaybeCreateSubresourceFilterNavigationThrottle(
-          handle, g_browser_process->safe_browsing_service());
-  if (subresource_filter_activation_throttle) {
-    throttles.push_back(
-        base::WrapUnique(subresource_filter_activation_throttle));
-  }
-
-  // These throttles must come after the
-  // SubresourceFilterSafeBrowsingActivationThrottle.
   content::WebContents* web_contents = handle->GetWebContents();
-  if (auto* factory =
-          subresource_filter::ContentSubresourceFilterDriverFactory::
-              FromWebContents(web_contents)) {
-    factory->throttle_manager()->MaybeAppendNavigationThrottles(handle,
-                                                                &throttles);
+  if (auto* subresource_filter_client =
+          ChromeSubresourceFilterClient::FromWebContents(web_contents)) {
+    subresource_filter_client->MaybeAppendNavigationThrottles(handle,
+                                                              &throttles);
   }
 
   return throttles;

chrome/browser/subresource_filter/chrome_subresource_filter_client.cc

@@ -5,19 +5,28 @@
 #include "chrome/browser/subresource_filter/chrome_subresource_filter_client.h"
 
 #include <string>
+#include <utility>
 
+#include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/subresource_filter/subresource_filter_profile_context_factory.h"
 #include "chrome/browser/ui/android/content_settings/subresource_filter_infobar_delegate.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings_types.h"
+#include "components/safe_browsing_db/database_manager.h"
+#include "components/safe_browsing_db/v4_feature_list.h"
 #include "components/subresource_filter/content/browser/content_ruleset_service.h"
 #include "components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h"
+#include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.h"
+#include "components/subresource_filter/core/browser/subresource_filter_features.h"
+#include "components/subresource_filter/core/common/activation_scope.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_handle.h"
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(ChromeSubresourceFilterClient);
@@ -35,6 +44,43 @@ ChromeSubresourceFilterClient::ChromeSubresourceFilterClient(
 
 ChromeSubresourceFilterClient::~ChromeSubresourceFilterClient() {}
 
+void ChromeSubresourceFilterClient::MaybeAppendNavigationThrottles(
+    content::NavigationHandle* navigation_handle,
+    std::vector<std::unique_ptr<content::NavigationThrottle>>* throttles) {
+  // Don't add any throttles if the feature isn't enabled at all.
+  if (subresource_filter::GetActiveConfigurations()
+          ->the_one_and_only()
+          .activation_scope == subresource_filter::ActivationScope::NO_SITES) {
+    return;
+  }
+
+  if (navigation_handle->IsInMainFrame()) {
+    safe_browsing::SafeBrowsingService* safe_browsing_service =
+        g_browser_process->safe_browsing_service();
+    scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> database_manager;
+    if (safe_browsing_service &&
+        safe_browsing_service->database_manager()->IsSupported() &&
+        safe_browsing::V4FeatureList::GetV4UsageStatus() ==
+            safe_browsing::V4FeatureList::V4UsageStatus::V4_ONLY) {
+      database_manager = safe_browsing_service->database_manager();
+    }
+
+    throttles->push_back(
+        base::MakeUnique<subresource_filter::
+                             SubresourceFilterSafeBrowsingActivationThrottle>(
+            navigation_handle,
+            content::BrowserThread::GetTaskRunnerForThread(
+                content::BrowserThread::IO),
+            std::move(database_manager)));
+  }
+
+  auto* driver_factory =
+      subresource_filter::ContentSubresourceFilterDriverFactory::
+          FromWebContents(navigation_handle->GetWebContents());
+  driver_factory->throttle_manager()->MaybeAppendNavigationThrottles(
+      navigation_handle, throttles);
+}
+
 void ChromeSubresourceFilterClient::ToggleNotificationVisibility(
     bool visibility) {
   if (did_show_ui_for_navigation_ && visibility)

chrome/browser/subresource_filter/chrome_subresource_filter_client.h

@@ -5,8 +5,10 @@
 #ifndef CHROME_BROWSER_SUBRESOURCE_FILTER_CHROME_SUBRESOURCE_FILTER_CLIENT_H_
 #define CHROME_BROWSER_SUBRESOURCE_FILTER_CHROME_SUBRESOURCE_FILTER_CLIENT_H_
 
+#include <memory>
 #include <set>
 #include <string>
+#include <vector>
 
 #include "base/macros.h"
 #include "components/content_settings/core/common/content_settings.h"
@@ -17,6 +19,7 @@ class GURL;
 
 namespace content {
 class NavigationHandle;
+class NavigationThrottle;
 class WebContents;
 }  // namespace content
 
@@ -71,6 +74,10 @@ class ChromeSubresourceFilterClient
   explicit ChromeSubresourceFilterClient(content::WebContents* web_contents);
   ~ChromeSubresourceFilterClient() override;
 
+  void MaybeAppendNavigationThrottles(
+      content::NavigationHandle* navigation_handle,
+      std::vector<std::unique_ptr<content::NavigationThrottle>>* throttles);
+
   // SubresourceFilterClient:
   void ToggleNotificationVisibility(bool visibility) override;
   bool ShouldSuppressActivation(

chrome/browser/subresource_filter/navigation_throttle_util.cc

-// Copyright 2017 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chrome/browser/subresource_filter/navigation_throttle_util.h"
-
-#include "chrome/browser/safe_browsing/safe_browsing_service.h"
-#include "components/safe_browsing_db/database_manager.h"
-#include "components/safe_browsing_db/v4_feature_list.h"
-#include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.h"
-#include "components/subresource_filter/core/browser/subresource_filter_features.h"
-#include "content/public/browser/browser_thread.h"
-#include "content/public/browser/navigation_handle.h"
-
-content::NavigationThrottle* MaybeCreateSubresourceFilterNavigationThrottle(
-    content::NavigationHandle* navigation_handle,
-    safe_browsing::SafeBrowsingService* safe_browsing_service) {
-  if (navigation_handle->IsInMainFrame() && safe_browsing_service &&
-      safe_browsing_service->database_manager()->IsSupported() &&
-      safe_browsing::V4FeatureList::GetV4UsageStatus() ==
-          safe_browsing::V4FeatureList::V4UsageStatus::V4_ONLY) {
-    return new subresource_filter::
-        SubresourceFilterSafeBrowsingActivationThrottle(
-            navigation_handle,
-            content::BrowserThread::GetTaskRunnerForThread(
-                content::BrowserThread::IO),
-            safe_browsing_service->database_manager());
-  }
-  return nullptr;
-}

chrome/browser/subresource_filter/navigation_throttle_util.h

-// Copyright (c) 2017 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CHROME_BROWSER_SUBRESOURCE_FILTER_NAVIGATION_THROTTLE_UTIL_H_
-#define CHROME_BROWSER_SUBRESOURCE_FILTER_NAVIGATION_THROTTLE_UTIL_H_
-
-#include "content/public/browser/navigation_throttle.h"
-
-namespace safe_browsing {
-class SafeBrowsingService;
-}
-
-// Creates the CreateSubresourceFilterNavigationThrottle for the main frame and
-// the |safe_browsing_service| supporting pver4, |safe_browsing_service| can be
-// a nullptr.
-content::NavigationThrottle* MaybeCreateSubresourceFilterNavigationThrottle(
-    content::NavigationHandle* navigation_handle,
-    safe_browsing::SafeBrowsingService* safe_browsing_service);
-
-#endif  // CHROME_BROWSER_SUBRESOURCE_FILTER_NAVIGATION_THROTTLE_UTIL_H_

components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.cc

@@ -24,13 +24,16 @@ SubresourceFilterSafeBrowsingActivationThrottle::
         scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager>
             database_manager)
     : NavigationThrottle(handle),
-      database_manager_(std::move(database_manager)),
-      io_task_runner_(io_task_runner),
-      database_client_(new SubresourceFilterSafeBrowsingClient(
-                           database_manager_,
-                           AsWeakPtr(),
-                           io_task_runner,
-                           base::ThreadTaskRunnerHandle::Get()),
+      io_task_runner_(std::move(io_task_runner)),
+      // The throttle can be created without a valid database manager. If so, it
+      // becomes a pass-through throttle and should never defer.
+      database_client_(database_manager
+                           ? new SubresourceFilterSafeBrowsingClient(
+                                 std::move(database_manager),
+                                 AsWeakPtr(),
+                                 io_task_runner_,
+                                 base::ThreadTaskRunnerHandle::Get())
+                           : nullptr,
                        base::OnTaskRunnerDeleter(io_task_runner_)) {}
 
 SubresourceFilterSafeBrowsingActivationThrottle::
@@ -52,6 +55,9 @@ SubresourceFilterSafeBrowsingActivationThrottle::WillRedirectRequest() {
 
 content::NavigationThrottle::ThrottleCheckResult
 SubresourceFilterSafeBrowsingActivationThrottle::WillProcessResponse() {
+  if (!database_client_)
+    return content::NavigationThrottle::PROCEED;
+
   // No need to defer the navigation if the check already happened.
   if (check_results_.back().finished) {
     NotifyResult();
@@ -82,6 +88,8 @@ void SubresourceFilterSafeBrowsingActivationThrottle::OnCheckUrlResultOnUI(
 }
 
 void SubresourceFilterSafeBrowsingActivationThrottle::CheckCurrentUrl() {
+  if (!database_client_)
+    return;
   check_results_.emplace_back();
   size_t id = check_results_.size() - 1;
   io_task_runner_->PostTask(