Navigation throttle for the Safe Browsing Subresource Filter.

components/subresource_filter/content/browser/subresource_filter_navigation_throttle_unittests.cc

Index: components/subresource_filter/content/browser/subresource_filter_navigation_throttle_unittests.cc
diff --git a/components/subresource_filter/content/browser/subresource_filter_navigation_throttle_unittests.cc b/components/subresource_filter/content/browser/subresource_filter_navigation_throttle_unittests.cc
new file mode 100644
index 0000000000000000000000000000000000000000..7904bb51c9d8f5c784a21d63423a4ee318afb342
--- /dev/null
+++ b/components/subresource_filter/content/browser/subresource_filter_navigation_throttle_unittests.cc
@@ -0,0 +1,240 @@
+// 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 <memory>
+
+#include "base/memory/ptr_util.h"
+#include "base/metrics/field_trial.h"
+#include "components/safe_browsing_db/util.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/browser/subresource_filter_features_test_support.h"
+#include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/navigation_throttle.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/test_renderer_host.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using content::NavigationThrottle;
+
+namespace {
+
+const char kExampleURL[] = "http://example.com";
+const char kTestURL[] = "http://test.com";
+const char kRedirectURLFirst[] = "http://example1.com";
+const char kRedirectURLSecond[] = "http://example2.com";
+const char kRedirectURLThird[] = "http://example3.com";
+
+}  //  namespace
+
+namespace subresource_filter {
+
+class TestContentSubresourceFilterDriver
+    : public ContentSubresourceFilterDriver {
+ public:
+  TestContentSubresourceFilterDriver(
+      content::RenderFrameHost* render_frame_host)
+      : ContentSubresourceFilterDriver(render_frame_host) {
+    activation_state_ = ActivationState::DISABLED;
+  }
+  ~TestContentSubresourceFilterDriver() override {}
+
+  void ActivateForProvisionalLoad(
+      ActivationState new_activation_state) override {
+    activation_state_ = new_activation_state;
+  }
+
+  ActivationState activation_state() { return activation_state_; }
+
+ private:
+  ActivationState activation_state_;
+  DISALLOW_COPY_AND_ASSIGN(TestContentSubresourceFilterDriver);
+};
+
+class SubresourceFilterNavigationThrottleTest
+    : public content::RenderViewHostTestHarness {
+ public:
+  SubresourceFilterNavigationThrottleTest() {}
+
+  // content::RenderViewHostTestHarness:
+  void SetUp() override {
+    RenderViewHostTestHarness::SetUp();
+    ContentSubresourceFilterDriverFactory::CreateForWebContents(web_contents());
+
+    driver_ = new TestContentSubresourceFilterDriver(main_rfh());
+    factory()->SetDriverForFrameHostForTesting(main_rfh(),
+                                               base::WrapUnique(driver_));
+  }
+
+  void TearDown() override {
+    handle_.reset();
+    RenderViewHostTestHarness::TearDown();
+  }
+
+  void SetUpNavigationHandleForURL(const GURL& url) {
+    handle_ = content::NavigationHandle::CreateNavigationHandleForTesting(
+        url, main_rfh());
+    handle_->RegisterThrottleForTesting(
+        SubresourceFilterNavigationThrottle::Create(handle_.get()));
+  }
+
+  content::NavigationHandle* handle() { return handle_.get(); }
+
+  ContentSubresourceFilterDriverFactory* factory() {
+    return ContentSubresourceFilterDriverFactory::FromWebContents(
+        web_contents());
+  }
+
+  TestContentSubresourceFilterDriver* driver() { return driver_; }
+
+  NavigationThrottle::ThrottleCheckResult SimulateWillStart() {
+    return handle()->CallWillStartRequestForTesting(
+        false /* is_post */, content::Referrer(), false /* has_user_gesture */,
+        ui::PAGE_TRANSITION_LINK, false /* is_external_protocol */);
+  }
+
+  NavigationThrottle::ThrottleCheckResult SimulateRedirects(
+      const GURL& redirect) {
+    return handle()->CallWillRedirectRequestForTesting(
+        redirect, false /* new_method_is_post */, GURL() /* new_referrer_url */,
+        false /* new_is_external_protocol */);
+  }
+
+  void SimulateWillProcessResponse() {
+    handle()->CallWillProcessResponseForTesting(main_rfh());
+  }
+
+ private:
+  TestContentSubresourceFilterDriver* driver_;
+  std::unique_ptr<content::NavigationHandle> handle_;
+
+  DISALLOW_COPY_AND_ASSIGN(SubresourceFilterNavigationThrottleTest);
+};
+
+TEST_F(SubresourceFilterNavigationThrottleTest, RequestWithoutRedirects) {
+  base::FieldTrialList field_trial_list(nullptr);
+  testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
+      base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationStateEnabled);
+
+  const GURL url(kExampleURL);
+  SetUpNavigationHandleForURL(url);
+  factory()->OnMainResourceMatchedSafeBrowsingBlacklist(
+      url, std::vector<GURL>(),
+      safe_browsing::ThreatPatternType::SOCIAL_ENGINEERING_ADS);

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

Why does this happen before SimulateWillStart()?

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

Moved SimulateWillStart.

+
+  EXPECT_EQ(ActivationState::DISABLED, driver()->activation_state());
+  SimulateWillStart();
+  SimulateWillProcessResponse();
+
+  EXPECT_EQ(1U, factory()->activation_set().size());
+  EXPECT_TRUE(factory()->ShouldActivateForURL(url));
+  EXPECT_EQ(GetMaximumActivationState(), driver()->activation_state());
+}
+
+TEST_F(SubresourceFilterNavigationThrottleTest,
+       RequestWithoutRedirectsNoActivation) {

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

No activation of the Field Trial?

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

Done.

+  const GURL url(kExampleURL);
+
+  factory()->OnMainResourceMatchedSafeBrowsingBlacklist(
+      url, std::vector<GURL>(),
+      safe_browsing::ThreatPatternType::SOCIAL_ENGINEERING_ADS);
+
+  EXPECT_EQ(ActivationState::DISABLED, driver()->activation_state());
+  SetUpNavigationHandleForURL(GURL(kTestURL));
+  SimulateWillStart();
+  SimulateWillProcessResponse();
+
+  EXPECT_EQ(1U, factory()->activation_set().size());
+  EXPECT_TRUE(factory()->ShouldActivateForURL(url));
+  EXPECT_FALSE(factory()->ShouldActivateForURL(GURL(kTestURL)));

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

move GURL(kTestURL) up:
const GURL url_with_activation(kExampleURL);
const GURL url_without_activation(kTestURL);
?

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

Done.

+  EXPECT_EQ(ActivationState::DISABLED, driver()->activation_state());
+}
+
+TEST_F(SubresourceFilterNavigationThrottleTest,
+       AddRedirectFromNavThrottleToServiceEnptyInitRedirects) {

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

typo: Enpty

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

Can you add comments to what these tests test? The testnames are pretty hard to
parse.

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

Done.

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

Done.

+  base::FieldTrialList field_trial_list(nullptr);
+  testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
+      base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationStateEnabled);
+
+  const GURL url(kExampleURL);
+  const GURL redirect(kRedirectURLFirst);
+
+  SetUpNavigationHandleForURL(url);
+  factory()->OnMainResourceMatchedSafeBrowsingBlacklist(
+      url, std::vector<GURL>(),
+      safe_browsing::ThreatPatternType::SOCIAL_ENGINEERING_ADS);
+
+  EXPECT_EQ(ActivationState::DISABLED, driver()->activation_state());
+  SimulateRedirects(redirect);

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

No WillStart before Redirects?

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

Done. But it's not really nessary.

+  SimulateWillProcessResponse();
+
+  EXPECT_EQ(2U, factory()->activation_set().size());
+  EXPECT_TRUE(factory()->ShouldActivateForURL(url));
+  EXPECT_TRUE(factory()->ShouldActivateForURL(redirect));
+  EXPECT_EQ(GetMaximumActivationState(), driver()->activation_state());
+}
+
+TEST_F(SubresourceFilterNavigationThrottleTest,
+       AddRedirectFromNavThrottleToServiceNonEmptyInitRedirects) {
+  base::FieldTrialList field_trial_list(nullptr);
+  testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
+      base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationStateEnabled);
+
+  const GURL url(kExampleURL);
+  const GURL nav_throttle_redirect(kTestURL);

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

What is the meaning of this variable?

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

Renamed. It's the redirect which has happened after Safe Browsing hit.

+  SetUpNavigationHandleForURL(url);
+
+  std::vector<GURL> redirects;

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

you can initialize vectors better now:
std::vector<GURL> redirects = {
  GURL(kRedirectURLFirst), ... };

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

Done.

+
+  redirects.push_back(GURL(kRedirectURLFirst));
+  redirects.push_back(GURL(kRedirectURLSecond));
+  redirects.push_back(GURL(kRedirectURLThird));
+  factory()->OnMainResourceMatchedSafeBrowsingBlacklist(
+      url, redirects, safe_browsing::ThreatPatternType::SOCIAL_ENGINEERING_ADS);
+
+  EXPECT_EQ(ActivationState::DISABLED, driver()->activation_state());
+  SimulateRedirects(nav_throttle_redirect);

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

again: Redirects before WillStart()?

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

Done.

+  SimulateWillProcessResponse();
+
+  EXPECT_EQ(redirects.size() + 2U, factory()->activation_set().size());
+  EXPECT_TRUE(factory()->ShouldActivateForURL(url));
+  EXPECT_TRUE(factory()->ShouldActivateForURL(GURL(kRedirectURLFirst)));
+  EXPECT_TRUE(factory()->ShouldActivateForURL(GURL(kRedirectURLSecond)));
+  EXPECT_TRUE(factory()->ShouldActivateForURL(GURL(kRedirectURLThird)));
+  EXPECT_TRUE(factory()->ShouldActivateForURL(GURL(kTestURL)));
+  EXPECT_EQ(GetMaximumActivationState(), driver()->activation_state());
+}
+
+TEST_F(SubresourceFilterNavigationThrottleTest,
+       RequestRedirectWithMatchRedirectTest) {
+  base::FieldTrialList field_trial_list(nullptr);
+  testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
+      base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationStateEnabled);
+
+  const GURL init_url(kExampleURL);
+  const GURL redirect_with_match(kRedirectURLFirst);
+  const GURL final_url(kRedirectURLSecond);
+  std::vector<GURL> redirects;

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

redirects = {redirect_with_match};

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

Done.

+  redirects.push_back(redirect_with_match);
+  factory()->OnMainResourceMatchedSafeBrowsingBlacklist(
+      init_url, redirects,
+      safe_browsing::ThreatPatternType::SOCIAL_ENGINEERING_ADS);
+
+  EXPECT_EQ(ActivationState::DISABLED, driver()->activation_state());
+  SetUpNavigationHandleForURL(init_url);

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

WillStart?

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

Done.

+  SimulateRedirects(redirect_with_match);
+  SimulateRedirects(final_url);
+  SimulateWillProcessResponse();
+
+  EXPECT_EQ(3U, factory()->activation_set().size());
+  EXPECT_TRUE(factory()->ShouldActivateForURL(init_url));
+  EXPECT_TRUE(factory()->ShouldActivateForURL(redirect_with_match));
+  EXPECT_TRUE(factory()->ShouldActivateForURL(final_url));
+  EXPECT_EQ(GetMaximumActivationState(), driver()->activation_state());
+}
+
+}  // namespace subresource_filter