Change the logic how Subesource Filter propagates activation.

components/subresource_filter/content/browser/content_subresource_filter_driver_factory_unittest.cc

Index: components/subresource_filter/content/browser/content_subresource_filter_driver_factory_unittest.cc
diff --git a/components/subresource_filter/content/browser/content_subresource_filter_driver_factory_unittest.cc b/components/subresource_filter/content/browser/content_subresource_filter_driver_factory_unittest.cc
index 2b48bf66c373d7c4a34f3112bd2d006496e22e57..b025eb53c671282e11b458bac6cac5d9c2df69c3 100644
--- a/components/subresource_filter/content/browser/content_subresource_filter_driver_factory_unittest.cc
+++ b/components/subresource_filter/content/browser/content_subresource_filter_driver_factory_unittest.cc
@@ -155,6 +155,16 @@ class ContentSubresourceFilterDriverFactoryTest
         safe_browsing::ThreatPatternType::SOCIAL_ENGINEERING_ADS);
   }
 
+  void NavigateToUrlAndCommit(const GURL& url, bool should_activate) {
+    EXPECT_CALL(*client(), ToggleNotificationVisibility(false)).Times(1);
+    content::WebContentsTester::For(web_contents())->StartNavigation(url);
+    ::testing::Mock::VerifyAndClearExpectations(client());
+    ActivateAndExpectForFrameHostForUrl(driver(), main_rfh(), url,
+                                        should_activate);
+    content::RenderFrameHostTester::For(main_rfh())
+        ->SimulateNavigationCommit(url);
+  }
+
   void ActivateAndExpectForFrameHostForUrl(MockSubresourceFilterDriver* driver,
                                            content::RenderFrameHost* rfh,
                                            const GURL& url,
@@ -185,16 +195,7 @@ class ContentSubresourceFilterDriverFactoryTest
       const GURL& url,
       bool should_activate,
       ActivationState expected_activation_state) {
-    EXPECT_CALL(*driver(), ActivateForProvisionalLoad(::testing::_))
-        .Times(should_activate);
-    EXPECT_CALL(*client(), ToggleNotificationVisibility(false)).Times(1);
-    content::WebContentsTester::For(web_contents())->StartNavigation(url);
-    ::testing::Mock::VerifyAndClearExpectations(client());
-    factory()->ReadyToCommitMainFrameNavigation(main_rfh(), url);
-    content::RenderFrameHostTester::For(main_rfh())
-        ->SimulateNavigationCommit(url);
-
-    ::testing::Mock::VerifyAndClearExpectations(driver());
+    NavigateToUrlAndCommit(url, should_activate);
 
     NavigateToUrlAndExpectActivationAndHidingPromptSubFrame(
         GURL(kExampleLoginUrl), should_activate);
@@ -238,93 +239,10 @@ class ContentSubresourceFilterDriverFactoryThreatTypeTest
   ContentSubresourceFilterDriverFactoryThreatTypeTest() {}
   ~ContentSubresourceFilterDriverFactoryThreatTypeTest() override {}
 
-  void VerifyEntitiesNotInTheBlacklist(
-      const GURL& test_url,
-      const std::vector<GURL>& redirects,
-      const ActivationListTestData& test_data) {
-    factory()->OnMainResourceMatchedSafeBrowsingBlacklist(
-        test_url, std::vector<GURL>(), test_data.threat_type,
-        test_data.threat_type_metadata);
-    EXPECT_EQ(test_data.should_add ? 1 : 0U,
-              factory()->activation_set().size());
-    EXPECT_EQ(test_data.should_add,
-              factory()->ShouldActivateForURL(GURL(test_url)));
-    EXPECT_EQ(test_data.should_add, factory()->ShouldActivateForURL(
-                                        GURL(test_url.GetWithEmptyPath())));
-    EXPECT_EQ(test_data.should_add,
-              factory()->ShouldActivateForURL(
-                  GURL("http://" + test_url.host() + "/path?q=q")));
-    factory()->OnMainResourceMatchedSafeBrowsingBlacklist(
-        test_url, redirects, test_data.threat_type,
-        test_data.threat_type_metadata);
-    for (const auto& redirect : redirects) {
-      EXPECT_EQ(test_data.should_add,
-                factory()->ShouldActivateForURL(redirect));
-      EXPECT_EQ(test_data.should_add,
-                factory()->ShouldActivateForURL(redirect.GetWithEmptyPath()));
-      EXPECT_EQ(test_data.should_add, factory()->ShouldActivateForURL(
-                                          GURL("http://" + redirect.host())));
-      EXPECT_EQ(test_data.should_add,
-                factory()->ShouldActivateForURL(
-                    GURL("http://" + redirect.host() + "/path?q=q")));
-    }
-  }
-
  private:
   DISALLOW_COPY_AND_ASSIGN(ContentSubresourceFilterDriverFactoryThreatTypeTest);
 };
 
-TEST_F(ContentSubresourceFilterDriverFactoryTest, SocEngHitEmptyRedirects) {
-  base::FieldTrialList field_trial_list(nullptr);
-  testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
-      base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationStateEnabled,
-      kActivationScopeNoSites, kActivationListSocialEngineeringAdsInterstitial);
-
-  BlacklistURLWithRedirects(GURL(kExampleUrlWithParams), std::vector<GURL>());
-  EXPECT_EQ(1U, factory()->activation_set().size());
-
-  EXPECT_TRUE(factory()->ShouldActivateForURL(GURL(kExampleUrl)));
-  EXPECT_TRUE(factory()->ShouldActivateForURL(GURL("http://example.com")));
-  EXPECT_TRUE(
-      factory()->ShouldActivateForURL(GURL("http://example.com/42?q=42!")));
-  EXPECT_TRUE(
-      factory()->ShouldActivateForURL(GURL("https://example.com/42?q=42!")));
-  EXPECT_TRUE(
-      factory()->ShouldActivateForURL(GURL("http://example.com/awesomepath")));
-  const GURL whitelisted("http://example.com/page?q=whitelisted");
-  EXPECT_TRUE(factory()->ShouldActivateForURL(whitelisted));
-  factory()->AddHostOfURLToWhitelistSet(whitelisted);
-  EXPECT_FALSE(factory()->ShouldActivateForURL(whitelisted));
-}
-
-TEST_F(ContentSubresourceFilterDriverFactoryTest, SocEngHitWithRedirects) {
-  base::FieldTrialList field_trial_list(nullptr);
-  testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
-      base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationStateEnabled,
-      kActivationScopeNoSites, kActivationListSocialEngineeringAdsInterstitial);
-
-  std::vector<GURL> redirects;
-  redirects.push_back(GURL("https://example1.com"));
-  redirects.push_back(GURL("https://example2.com"));
-  redirects.push_back(GURL("https://example3.com"));
-  BlacklistURLWithRedirects(GURL(kExampleUrlWithParams), redirects);
-  EXPECT_EQ(4U, factory()->activation_set().size());
-  EXPECT_TRUE(factory()->ShouldActivateForURL(GURL(kExampleUrl)));
-
-  for (const auto& redirect : redirects) {
-    EXPECT_TRUE(factory()->ShouldActivateForURL(redirect));
-    EXPECT_TRUE(factory()->ShouldActivateForURL(redirect.GetWithEmptyPath()));
-    EXPECT_TRUE(
-        factory()->ShouldActivateForURL(GURL("http://" + redirect.host())));
-    EXPECT_TRUE(factory()->ShouldActivateForURL(
-        GURL("http://" + redirect.host() + "/path?q=q")));
-  }
-  const GURL whitelisted("http://example.com/page?q=42");
-  EXPECT_TRUE(factory()->ShouldActivateForURL(whitelisted));
-  factory()->AddHostOfURLToWhitelistSet(whitelisted);
-  EXPECT_FALSE(factory()->ShouldActivateForURL(whitelisted));
-}
-
 TEST_F(ContentSubresourceFilterDriverFactoryTest,
        ActivateForFrameHostNotNeeded) {
   base::FieldTrialList field_trial_list(nullptr);
@@ -334,9 +252,9 @@ TEST_F(ContentSubresourceFilterDriverFactoryTest,
   ActivateAndExpectForFrameHostForUrl(driver(), main_rfh(), GURL(kTestUrl),
                                       false /* should_activate */);
   BlacklistURLWithRedirects(GURL(kExampleUrlWithParams), std::vector<GURL>());
-  ActivateAndExpectForFrameHostForUrl(driver(), main_rfh(),
-                                      GURL("https://not-example.com"),
-                                      false /* should_activate */);
+  NavigateToUrlAndCommit(GURL(kExampleUrlWithParams), false);
+  NavigateToUrlAndCommit(GURL("https://not-example.com"),
+                         false /* should_activate */);
 }
 
 TEST_F(ContentSubresourceFilterDriverFactoryTest, ActivateForFrameHostNeeded) {
@@ -347,10 +265,9 @@ TEST_F(ContentSubresourceFilterDriverFactoryTest, ActivateForFrameHostNeeded) {
       kActivationListSocialEngineeringAdsInterstitial);
 
   BlacklistURLWithRedirects(GURL(kExampleUrlWithParams), std::vector<GURL>());
-  ActivateAndExpectForFrameHostForUrl(driver(), main_rfh(), GURL(kTestUrl),
-                                      false /* should_activate */);
-  ActivateAndExpectForFrameHostForUrl(driver(), main_rfh(), GURL(kExampleUrl),
-                                      true /* should_activate */);
+  NavigateToUrlAndCommit(GURL(kExampleUrlWithParams),
+                         true /* should_activate */);
+  NavigateToUrlAndCommit(GURL(kExampleUrl), false /* should_activate */);
 }
 
 TEST_P(ContentSubresourceFilterDriverFactoryThreatTypeTest, NonSocEngHit) {
@@ -366,7 +283,10 @@ TEST_P(ContentSubresourceFilterDriverFactoryThreatTypeTest, NonSocEngHit) {
   redirects.push_back(GURL("https://example3.com"));
 
   const GURL test_url("https://example.com/nonsoceng?q=engsocnon");
-  VerifyEntitiesNotInTheBlacklist(test_url, redirects, test_data);
+  factory()->OnMainResourceMatchedSafeBrowsingBlacklist(
+      test_url, redirects, test_data.threat_type,
+      test_data.threat_type_metadata);
+  NavigateToUrlAndCommit(test_url, false /* should_activate */);
 };
 
 TEST_F(ContentSubresourceFilterDriverFactoryTest,
@@ -480,6 +400,8 @@ TEST_F(ContentSubresourceFilterDriverFactoryTest,
 
 TEST_F(ContentSubresourceFilterDriverFactoryTest,
        SpecialCaseNavigationAllSitesEnabled) {
+  // Check when the experiment is enabled for all site, the activation signal is

[engedy, 2016/10/12 09:32:29]

nit: Check that when ...

[melandory, 2016/10/12 12:29:01]

Done.

+  // always sent.
   base::FieldTrialList field_trial_list(nullptr);
   testing::ScopedSubresourceFilterFeatureToggle scoped_feature_toggle(
       base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationStateEnabled,
@@ -494,7 +416,7 @@ TEST_F(ContentSubresourceFilterDriverFactoryTest,
       base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationStateEnabled,
       kActivationScopeActivationList,
       kActivationListSocialEngineeringAdsInterstitial);
-  BlacklistURLWithRedirects(GURL(kExampleUrlWithParams), std::vector<GURL>());
+  BlacklistURLWithRedirects(GURL(kExampleUrl), std::vector<GURL>());
   SpecialCaseNavSeq(true /* should_activate */, ActivationState::ENABLED);
 }