Navigation throttle for the Safe Browsing Subresource Filter.

components/subresource_filter/content/browser/subresource_filter_navigation_throttle.cc

Index: components/subresource_filter/content/browser/subresource_filter_navigation_throttle.cc
diff --git a/components/subresource_filter/content/browser/subresource_filter_navigation_throttle.cc b/components/subresource_filter/content/browser/subresource_filter_navigation_throttle.cc
new file mode 100644
index 0000000000000000000000000000000000000000..1829d8d0d86c1d6978fb8a65e68d96dc37858af0
--- /dev/null
+++ b/components/subresource_filter/content/browser/subresource_filter_navigation_throttle.cc
@@ -0,0 +1,60 @@
+// Copyright 2016 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 "components/subresource_filter/content/browser/subresource_filter_navigation_throttle.h"
+
+#include "components/subresource_filter/content/browser/content_subresource_filter_driver.h"
+#include "components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h"
+#include "components/subresource_filter/core/browser/subresource_filter_features.h"
+#include "components/subresource_filter/core/common/activation_state.h"
+#include "content/public/browser/navigation_handle.h"
+
+namespace subresource_filter {
+
+// static
+std::unique_ptr<content::NavigationThrottle>
+SubresourceFilterNavigationThrottle::Create(content::NavigationHandle* handle) {
+  return std::unique_ptr<content::NavigationThrottle>(
+      new SubresourceFilterNavigationThrottle(handle));
+}
+
+SubresourceFilterNavigationThrottle::SubresourceFilterNavigationThrottle(
+    content::NavigationHandle* handle)
+    : content::NavigationThrottle(handle),
+      initial_url_(navigation_handle()->GetURL()) {}
+
+SubresourceFilterNavigationThrottle::~SubresourceFilterNavigationThrottle() {}
+
+content::NavigationThrottle::ThrottleCheckResult
+SubresourceFilterNavigationThrottle::WillRedirectRequest() {

[battre, 2016/07/01 15:51:36]

I wonder whether the NavigationThrottle should just delegate these functions.
Currently you have two places that determine what should go into the activation
set:
SubresourceFilterNavigationThrottle::WillRedirectRequest() and
ContentSubresourceFilterDriverFactory::OnMainResourceMatchedSafeBrowsingBlacklist().

WillRedirectRequest() would become

ContentSubresourceFilterDriverFactory::FromWebContents(
   
navigation_handle()->GetWebContents())->activation_set()->Redirect(initial_url_,
navigation_handle()->GetURL());

And WillProcessResponse() would become

ContentSubresourceFilterDriverFactory::FromWebContents(
    navigation_handle()->GetWebContents())->OnProcessResponse();

OnProcessResponse would decide whether to create a
ContentSubresourceFilterDriver and do it if appropriate.

If you do this, my comments in the headerfile would become obsolete.

[melandory, 2016/07/18 15:16:47]

I like the suggestion, although I would do this refactoring as follow up if you
don't mind.

[engedy, 2016/07/19 11:27:02]

I really like this approach as well. Fine to do in a follow-up CL.

+  ContentSubresourceFilterDriverFactory* driver_factory =
+      ContentSubresourceFilterDriverFactory::FromWebContents(
+          navigation_handle()->GetWebContents());

[battre, 2016/07/01 15:51:36]

// Ensure that the activation state of the subresource filter is persisted
beyond redirects.

[melandory, 2016/07/18 15:16:47]

Done.

+  if (driver_factory && driver_factory->ShouldActivateForURL(initial_url_)) {
+    driver_factory->AddOriginOfURLToActivationSet(
+        navigation_handle()->GetURL());
+  }
+
+  return NavigationThrottle::PROCEED;
+}
+
+content::NavigationThrottle::ThrottleCheckResult
+SubresourceFilterNavigationThrottle::WillProcessResponse() {
+  if (!navigation_handle()->GetURL().SchemeIsHTTPOrHTTPS())
+    return NavigationThrottle::PROCEED;
+
+  ContentSubresourceFilterDriverFactory* driver_factory =
+      ContentSubresourceFilterDriverFactory::FromWebContents(
+          navigation_handle()->GetWebContents());
+  if (driver_factory && driver_factory->ShouldActivateForURL(initial_url_)) {
+    ContentSubresourceFilterDriver* driver =
+        driver_factory->DriverFromFrameHost(
+            navigation_handle()->GetRenderFrameHost());
+    driver->ActivateForProvisionalLoad(GetMaximumActivationState());
+  }
+
+  return NavigationThrottle::PROCEED;
+}
+
+}  // namespace subresource_filter