Distinguish between subresource filtering and dryrun matching.

components/subresource_filter/content/renderer/subresource_filter_agent_unittest.cc

Index: components/subresource_filter/content/renderer/subresource_filter_agent_unittest.cc
diff --git a/components/subresource_filter/content/renderer/subresource_filter_agent_unittest.cc b/components/subresource_filter/content/renderer/subresource_filter_agent_unittest.cc
index 7a8b9bd0c20343610ca1fab1e3bace2f9afd7cf0..4c68e477d630c0711675ecd071ddb2b840409885 100644
--- a/components/subresource_filter/content/renderer/subresource_filter_agent_unittest.cc
+++ b/components/subresource_filter/content/renderer/subresource_filter_agent_unittest.cc
@@ -170,11 +170,21 @@ class SubresourceFilterAgentTest : public ::testing::Test {
     EXPECT_CALL(*agent(), SendDocumentLoadStatistics(::testing::_));
   }
 
-  void ExpectLoadAllowed(base::StringPiece url_spec, bool allowed) {
+  void ExpectLoadPolicy(
+      base::StringPiece url_spec,
+      blink::WebDocumentSubresourceFilter::LoadPolicy expected_policy) {
     blink::WebURL url = GURL(url_spec);
     blink::WebURLRequest::RequestContext request_context =
         blink::WebURLRequest::RequestContextImage;
-    EXPECT_EQ(allowed, agent()->filter()->allowLoad(url, request_context));
+    blink::WebDocumentSubresourceFilter::LoadPolicy actual_policy =
+        agent()->filter()->getLoadPolicy(url, request_context);
+    EXPECT_EQ(expected_policy, actual_policy);
+
+    // If the load policy indicated the load was filtered, simulate a filtered
+    // load callback. In production, this will be called in FrameFetchContext,
+    // but we simulate the call here.
+    if (actual_policy == blink::WebDocumentSubresourceFilter::Filter)
+      agent()->filter()->reportFilteredLoad();
   }
 
   SubresourceFilterAgentUnderTest* agent() { return agent_.get(); }
@@ -273,8 +283,8 @@ TEST_F(SubresourceFilterAgentTest, Enabled_FilteringIsInEffectForOneLoad) {
   ASSERT_TRUE(::testing::Mock::VerifyAndClearExpectations(agent()));
 
   ExpectSignalAboutFirstSubresourceDisallowed();
-  ExpectLoadAllowed(kTestFirstURL, false);
-  ExpectLoadAllowed(kTestSecondURL, true);
+  ExpectLoadPolicy(kTestFirstURL, blink::WebDocumentSubresourceFilter::Filter);
+  ExpectLoadPolicy(kTestSecondURL, blink::WebDocumentSubresourceFilter::Allow);
   ExpectDocumentLoadStatisticsSent();
   FinishLoad();
 
@@ -282,8 +292,8 @@ TEST_F(SubresourceFilterAgentTest, Enabled_FilteringIsInEffectForOneLoad) {
   ExpectNoSubresourceFilterGetsInjected();
   ExpectNoSignalAboutFirstSubresourceDisallowed();
   PerformSamePageNavigationWithoutSettingActivationLevel();
-  ExpectLoadAllowed(kTestFirstURL, false);
-  ExpectLoadAllowed(kTestSecondURL, true);
+  ExpectLoadPolicy(kTestFirstURL, blink::WebDocumentSubresourceFilter::Filter);
+  ExpectLoadPolicy(kTestSecondURL, blink::WebDocumentSubresourceFilter::Allow);
 
   ExpectNoSubresourceFilterGetsInjected();
   StartLoadWithoutSettingActivationLevel();
@@ -313,11 +323,14 @@ TEST_F(SubresourceFilterAgentTest, Enabled_HistogramSamplesOverTwoLoads) {
     ASSERT_TRUE(::testing::Mock::VerifyAndClearExpectations(agent()));
 
     ExpectSignalAboutFirstSubresourceDisallowed();
-    ExpectLoadAllowed(kTestFirstURL, false);
+    ExpectLoadPolicy(kTestFirstURL,
+                     blink::WebDocumentSubresourceFilter::Filter);
     ExpectNoSignalAboutFirstSubresourceDisallowed();
-    ExpectLoadAllowed(kTestFirstURL, false);
+    ExpectLoadPolicy(kTestFirstURL,
+                     blink::WebDocumentSubresourceFilter::Filter);
     ExpectNoSignalAboutFirstSubresourceDisallowed();
-    ExpectLoadAllowed(kTestSecondURL, true);
+    ExpectLoadPolicy(kTestSecondURL,
+                     blink::WebDocumentSubresourceFilter::Allow);
     ExpectDocumentLoadStatisticsSent();
     FinishLoad();
 
@@ -327,9 +340,11 @@ TEST_F(SubresourceFilterAgentTest, Enabled_HistogramSamplesOverTwoLoads) {
     ASSERT_TRUE(::testing::Mock::VerifyAndClearExpectations(agent()));
 
     ExpectNoSignalAboutFirstSubresourceDisallowed();
-    ExpectLoadAllowed(kTestSecondURL, true);
+    ExpectLoadPolicy(kTestSecondURL,
+                     blink::WebDocumentSubresourceFilter::Allow);
     ExpectSignalAboutFirstSubresourceDisallowed();
-    ExpectLoadAllowed(kTestFirstURL, false);
+    ExpectLoadPolicy(kTestFirstURL,
+                     blink::WebDocumentSubresourceFilter::Filter);
     ExpectDocumentLoadStatisticsSent();
     FinishLoad();
 
@@ -369,8 +384,8 @@ TEST_F(SubresourceFilterAgentTest, Enabled_NewRulesetIsPickedUpAtNextLoad) {
       SetTestRulesetToDisallowURLsWithPathSuffix(kTestSecondURLPathSuffix));
 
   ExpectSignalAboutFirstSubresourceDisallowed();
-  ExpectLoadAllowed(kTestFirstURL, false);
-  ExpectLoadAllowed(kTestSecondURL, true);
+  ExpectLoadPolicy(kTestFirstURL, blink::WebDocumentSubresourceFilter::Filter);
+  ExpectLoadPolicy(kTestSecondURL, blink::WebDocumentSubresourceFilter::Allow);
   ExpectDocumentLoadStatisticsSent();
   FinishLoad();
 
@@ -379,8 +394,8 @@ TEST_F(SubresourceFilterAgentTest, Enabled_NewRulesetIsPickedUpAtNextLoad) {
   ASSERT_TRUE(::testing::Mock::VerifyAndClearExpectations(agent()));
 
   ExpectSignalAboutFirstSubresourceDisallowed();
-  ExpectLoadAllowed(kTestFirstURL, true);
-  ExpectLoadAllowed(kTestSecondURL, false);
+  ExpectLoadPolicy(kTestFirstURL, blink::WebDocumentSubresourceFilter::Allow);
+  ExpectLoadPolicy(kTestSecondURL, blink::WebDocumentSubresourceFilter::Filter);
   ExpectDocumentLoadStatisticsSent();
   FinishLoad();
 }
@@ -415,9 +430,11 @@ TEST_F(SubresourceFilterAgentTest, DryRun_ResourcesAreEvaluatedButNotFiltered) {
   // In dry-run mode, loads to the first URL should be recorded as
   // `MatchedRules`, but still be allowed to proceed and not recorded as
   // `Disallowed`.
-  ExpectLoadAllowed(kTestFirstURL, true);
-  ExpectLoadAllowed(kTestFirstURL, true);
-  ExpectLoadAllowed(kTestSecondURL, true);
+  ExpectLoadPolicy(kTestFirstURL,
+                   blink::WebDocumentSubresourceFilter::AllowWouldFilter);
+  ExpectLoadPolicy(kTestFirstURL,
+                   blink::WebDocumentSubresourceFilter::AllowWouldFilter);
+  ExpectLoadPolicy(kTestSecondURL, blink::WebDocumentSubresourceFilter::Allow);
   ExpectDocumentLoadStatisticsSent();
   FinishLoad();
 
@@ -445,10 +462,10 @@ TEST_F(SubresourceFilterAgentTest,
   ASSERT_TRUE(::testing::Mock::VerifyAndClearExpectations(agent()));
 
   ExpectSignalAboutFirstSubresourceDisallowed();
-  ExpectLoadAllowed(kTestFirstURL, false);
+  ExpectLoadPolicy(kTestFirstURL, blink::WebDocumentSubresourceFilter::Filter);
   ExpectNoSignalAboutFirstSubresourceDisallowed();
-  ExpectLoadAllowed(kTestFirstURL, false);
-  ExpectLoadAllowed(kTestSecondURL, true);
+  ExpectLoadPolicy(kTestFirstURL, blink::WebDocumentSubresourceFilter::Filter);
+  ExpectLoadPolicy(kTestSecondURL, blink::WebDocumentSubresourceFilter::Allow);
   ExpectDocumentLoadStatisticsSent();
   FinishLoad();
 
@@ -456,9 +473,9 @@ TEST_F(SubresourceFilterAgentTest,
   StartLoadAndSetActivationLevel(ActivationLevel::ENABLED);
   ASSERT_TRUE(::testing::Mock::VerifyAndClearExpectations(agent()));
 
-  ExpectLoadAllowed(kTestSecondURL, true);
+  ExpectLoadPolicy(kTestSecondURL, blink::WebDocumentSubresourceFilter::Allow);
   ExpectSignalAboutFirstSubresourceDisallowed();
-  ExpectLoadAllowed(kTestFirstURL, false);
+  ExpectLoadPolicy(kTestFirstURL, blink::WebDocumentSubresourceFilter::Filter);
   ExpectDocumentLoadStatisticsSent();
   FinishLoad();
 }
@@ -473,8 +490,13 @@ TEST_F(SubresourceFilterAgentTest,
 
   auto filter = agent()->TakeFilter();
   ResetAgent();
-  EXPECT_FALSE(filter->allowLoad(GURL(kTestFirstURL),
-                                 blink::WebURLRequest::RequestContextImage));
+
+  // The filter has been disconnected from the agent, so a call to
+  // reportFilteredLoad() should not signal a first resource disallowed call to
+  // the agent.

[engedy, 2017/02/06 16:56:32]

nit: ... nor should it cause a crash.

[Bryan McQuade, 2017/02/06 17:11:56]

Done

+  ExpectNoSignalAboutFirstSubresourceDisallowed();
+
+  filter->reportFilteredLoad();
 }
 
 }  // namespace subresource_filter