[subresource_filter] SB throttle can send multiple speculative requests.

components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.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 "components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.h"
 
+#include <map>
 #include <memory>
+#include <set>
+#include <string>
+#include <tuple>
+#include <utility>
 
 #include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
+#include "base/run_loop.h"
 #include "base/test/histogram_tester.h"
+#include "base/timer/mock_timer.h"
 #include "components/safe_browsing_db/test_database_manager.h"
 #include "components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h"
 #include "components/subresource_filter/content/browser/subresource_filter_client.h"
+#include "components/subresource_filter/content/browser/subresource_filter_safe_browsing_client.h"
 #include "components/subresource_filter/core/browser/subresource_filter_features.h"
 #include "components/subresource_filter/core/browser/subresource_filter_features_test_support.h"
 #include "components/subresource_filter/core/common/test_ruleset_creator.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents_observer.h"
+#include "content/public/test/cancelling_navigation_throttle.h"
 #include "content/public/test/navigation_simulator.h"
 #include "content/public/test/test_renderer_host.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace subresource_filter {
 
 namespace {
 
 char kURL[] = "http://example.test/";
-char kRedirectURL[] = "http://foo.test/";
+char kRedirectURL[] = "http://redirect.test/";
 
 // Names of navigation chain patterns histogram.
 const char kMatchesPatternHistogramNameSubresourceFilterSuffix[] =
     "SubresourceFilter.PageLoad.RedirectChainMatchPattern."
     "SubresourceFilterOnly";
 const char kNavigationChainSizeSubresourceFilterSuffix[] =
     "SubresourceFilter.PageLoad.RedirectChainLength.SubresourceFilterOnly";
+const char kSafeBrowsingNavigationDelay[] =
+    "SubresourceFilter.SafeBrowsing.NavigationDelay";
 
 // Human readable representation of expected redirect chain match patterns.
 // The explanations for the buckets given for the following redirect chain:
 // A->B->C->D, where A is initial URL and D is a final URL.
 enum RedirectChainMatchPattern {
   EMPTY,             // No histograms were recorded.
   F0M0L1,            // D is a Safe Browsing match.
   F0M1L0,            // B or C, or both are Safe Browsing matches.
   F0M1L1,            // B or C, or both and D are Safe Browsing matches.
   F1M0L0,            // A is Safe Browsing match
@@ skipping 16 matching lines @@
                          safe_browsing::SBThreatType threat_type) {
     url_to_threat_type_[url] = threat_type;
   }
 
   void SimulateTimeout() { simulate_timeout_ = true; }
 
  protected:
   ~FakeSafeBrowsingDatabaseManager() override {}
 
   bool CheckUrlForSubresourceFilter(const GURL& url, Client* client) override {
+    // Maintain the invariant that each client only makes a single request.
+    DCHECK(checks_.find(client) == checks_.end());
     if (simulate_timeout_)
       return false;
     if (!url_to_threat_type_.count(url))
       return true;
 
+    checks_.insert(client);
+
     content::BrowserThread::PostTask(
         content::BrowserThread::IO, FROM_HERE,
-        base::Bind(&Client::OnCheckBrowseUrlResult, base::Unretained(client),
-                   url, url_to_threat_type_[url],
-                   safe_browsing::ThreatMetadata()));
+        base::Bind(&FakeSafeBrowsingDatabaseManager::
+                       OnCheckUrlForSubresourceFilterComplete,
+                   base::Unretained(this), base::Unretained(client), url));
     return false;
   }
 
+  void OnCheckUrlForSubresourceFilterComplete(Client* client, const GURL& url) {
+    if (checks_.find(client) == checks_.end())
+      return;
+    client->OnCheckBrowseUrlResult(url, url_to_threat_type_[url],
+                                   safe_browsing::ThreatMetadata());
+  }
+
   bool CheckResourceUrl(const GURL& url, Client* client) override {
     return true;
   }
 
   bool IsSupported() const override { return true; }
   bool ChecksAreAlwaysAsync() const override { return false; }
   bool CanCheckResourceType(
       content::ResourceType /* resource_type */) const override {
     return true;
   }
 
   safe_browsing::ThreatSource GetThreatSource() const override {
     return safe_browsing::ThreatSource::LOCAL_PVER4;
   }
 
   bool CheckExtensionIDs(const std::set<std::string>& extension_ids,
                          Client* client) override {
     return true;
   }
 
+  void CancelCheck(Client* client) override { checks_.erase(client); }
+
  private:
   std::map<GURL, safe_browsing::SBThreatType> url_to_threat_type_;
   bool simulate_timeout_;
 
+  std::set<Client*> checks_;
+
   DISALLOW_COPY_AND_ASSIGN(FakeSafeBrowsingDatabaseManager);
 };
 
 class MockSubresourceFilterClient
     : public subresource_filter::SubresourceFilterClient {
  public:
   MockSubresourceFilterClient() {}
 
   ~MockSubresourceFilterClient() override = default;
 
   MOCK_METHOD1(ToggleNotificationVisibility, void(bool));
   MOCK_METHOD1(IsWhitelistedByContentSettings, bool(const GURL&));
   MOCK_METHOD1(WhitelistByContentSettings, void(const GURL&));
   MOCK_METHOD0(GetRulesetDealer, VerifiedRulesetDealer::Handle*());
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockSubresourceFilterClient);
 };
 
 // Throttle to call WillProcessResponse on the factory, which is otherwise
 // called by the ThrottleManager.
 class TestForwardingNavigationThrottle : public content::NavigationThrottle {
  public:
-  TestForwardingNavigationThrottle(content::NavigationHandle* handle)
+  explicit TestForwardingNavigationThrottle(content::NavigationHandle* handle)
       : content::NavigationThrottle(handle) {}
   ~TestForwardingNavigationThrottle() override {}
 
   // content::NavigationThrottle:
   content::NavigationThrottle::ThrottleCheckResult WillProcessResponse()
       override {
     content::WebContents* web_contents = navigation_handle()->GetWebContents();
     ContentSubresourceFilterDriverFactory* factory =
         ContentSubresourceFilterDriverFactory::FromWebContents(web_contents);
     factory->WillProcessResponse(navigation_handle());
@@ skipping 21 matching lines @@
             base::FeatureList::OVERRIDE_ENABLE_FEATURE, kActivationLevelEnabled,
             kActivationScopeActivationList, kActivationListSubresourceFilter));
     auto client = base::MakeUnique<MockSubresourceFilterClient>();
     ContentSubresourceFilterDriverFactory::CreateForWebContents(
         RenderViewHostTestHarness::web_contents(), std::move(client));
     fake_safe_browsing_database_ = new FakeSafeBrowsingDatabaseManager();
     NavigateAndCommit(GURL("https://test.com"));
     Observe(RenderViewHostTestHarness::web_contents());
   }
 
+  void TearDown() override {
+    RunUntilIdle();
+    content::RenderViewHostTestHarness::TearDown();
+  }
+
   ContentSubresourceFilterDriverFactory* factory() {
     return ContentSubresourceFilterDriverFactory::FromWebContents(
         RenderViewHostTestHarness::web_contents());
   }
 
   // content::WebContentsObserver:
   void DidStartNavigation(
       content::NavigationHandle* navigation_handle) override {
     ASSERT_TRUE(navigation_handle->IsInMainFrame());
-    navigation_handle_ = navigation_handle;
+    auto mock_timer = base::MakeUnique<base::MockTimer>(
+        false /* retain_user_task */, false /* is_repeating */);
+    mock_timer_ = mock_timer.get();
     navigation_handle->RegisterThrottleForTesting(
         base::MakeUnique<SubresourceFilterSafeBrowsingActivationThrottle>(
-            navigation_handle, fake_safe_browsing_database_));
+            navigation_handle, std::move(mock_timer),
+            fake_safe_browsing_database_));
     navigation_handle->RegisterThrottleForTesting(
         base::MakeUnique<TestForwardingNavigationThrottle>(navigation_handle));
   }
 
+  void DidFinishNavigation(
+      content::NavigationHandle* navigation_handle) override {
+    mock_timer_ = nullptr;
+  }
+
+  content::NavigationThrottle::ThrottleCheckResult SimulateStart() {
+    navigation_simulator_->Start();
+    auto result = navigation_simulator_->GetLastThrottleCheckResult();
+    if (result == content::NavigationThrottle::CANCEL)
+      navigation_simulator_.reset();
+    return result;
+  }
+
+  content::NavigationThrottle::ThrottleCheckResult SimulateRedirect(
+      const GURL& new_url) {
+    navigation_simulator_->Redirect(new_url);
+    auto result = navigation_simulator_->GetLastThrottleCheckResult();
+    if (result == content::NavigationThrottle::CANCEL)
+      navigation_simulator_.reset();
+    return result;
+  }
+
+  content::NavigationThrottle::ThrottleCheckResult SimulateCommit() {
+    navigation_simulator_->Commit();
+    auto result = navigation_simulator_->GetLastThrottleCheckResult();
+    if (result == content::NavigationThrottle::CANCEL)
+      navigation_simulator_.reset();
+    return result;
+  }
+
   void SimulateStartAndExpectProceed() {
-    navigation_simulator_->Start();
-    EXPECT_EQ(content::NavigationThrottle::PROCEED,
-              navigation_simulator_->GetLastThrottleCheckResult());
+    EXPECT_EQ(content::NavigationThrottle::PROCEED, SimulateStart());
   }
 
   void SimulateRedirectAndExpectProceed(const GURL& new_url) {
-    navigation_simulator_->Redirect(new_url);
-    EXPECT_EQ(content::NavigationThrottle::PROCEED,
-              navigation_simulator_->GetLastThrottleCheckResult());
+    EXPECT_EQ(content::NavigationThrottle::PROCEED, SimulateRedirect(new_url));
   }
 
   void SimulateCommitAndExpectProceed() {
-    navigation_simulator_->Commit();
-    EXPECT_EQ(content::NavigationThrottle::PROCEED,
-              navigation_simulator_->GetLastThrottleCheckResult());
+    EXPECT_EQ(content::NavigationThrottle::PROCEED, SimulateCommit());
   }
 
   void CreateTestNavigationForMainFrame(const GURL& first_url) {
     navigation_simulator_ =
         content::NavigationSimulator::CreateRendererInitiated(first_url,
                                                               main_rfh());
   }
 
   void ConfigureAsSubresourceFilterOnlyURL(const GURL& url) {
     fake_safe_browsing_database_->AddBlacklistedUrl(
         url, safe_browsing::SB_THREAT_TYPE_SUBRESOURCE_FILTER);
   }
 
   void SimulateTimeout() { fake_safe_browsing_database_->SimulateTimeout(); }
 
+  void RunUntilIdle() {
+    base::RunLoop().RunUntilIdle();
+    auto callback = []() { base::RunLoop().RunUntilIdle(); };
+    base::RunLoop run_loop;
+    content::BrowserThread::PostTaskAndReply(content::BrowserThread::IO,
+                                             FROM_HERE, base::Bind(callback),
+                                             run_loop.QuitClosure());
+    run_loop.Run();
+    base::RunLoop().RunUntilIdle();
+  }
+
   const base::HistogramTester& tester() const { return tester_; }
 
+  base::MockTimer* mock_timer() { return mock_timer_; }
+
  private:
   base::FieldTrialList field_trial_list_;
   std::unique_ptr<testing::ScopedSubresourceFilterFeatureToggle>
       scoped_feature_toggle_;
   std::unique_ptr<content::NavigationSimulator> navigation_simulator_;
   scoped_refptr<FakeSafeBrowsingDatabaseManager> fake_safe_browsing_database_;
   base::HistogramTester tester_;
-  content::NavigationHandle* navigation_handle_;
+
+  // Only valid during a navigation.
+  base::MockTimer* mock_timer_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(SubresourceFilterSafeBrowsingActivationThrottleTest);
 };
 
+class SubresourceFilterSafeBrowsingActivationThrottleTestWithCancelling
+    : public SubresourceFilterSafeBrowsingActivationThrottleTest,
+      public ::testing::WithParamInterface<
+          std::tuple<content::CancellingNavigationThrottle::CancelTime,
+                     content::CancellingNavigationThrottle::ResultSynchrony>> {
+ public:
+  SubresourceFilterSafeBrowsingActivationThrottleTestWithCancelling() {}
+  ~SubresourceFilterSafeBrowsingActivationThrottleTestWithCancelling()
+      override {}
+
+  void DidStartNavigation(content::NavigationHandle* handle) override {
+    std::tie(cancel_time_, result_sync_) = GetParam();
+    handle->RegisterThrottleForTesting(
+        base::MakeUnique<content::CancellingNavigationThrottle>(
+            handle, cancel_time_, result_sync_));
+    SubresourceFilterSafeBrowsingActivationThrottleTest::DidStartNavigation(
+        handle);
+  }
+
+  content::CancellingNavigationThrottle::CancelTime cancel_time() {
+    return cancel_time_;
+  }
+
+  content::CancellingNavigationThrottle::ResultSynchrony result_sync() {
+    return result_sync_;
+  }
+
+ private:
+  content::CancellingNavigationThrottle::CancelTime cancel_time_;
+  content::CancellingNavigationThrottle::ResultSynchrony result_sync_;
+
+  DISALLOW_COPY_AND_ASSIGN(
+      SubresourceFilterSafeBrowsingActivationThrottleTestWithCancelling);
+};
+
 TEST_F(SubresourceFilterSafeBrowsingActivationThrottleTest,
        ListNotMatched_NoActivation) {
   const GURL url(kURL);
   CreateTestNavigationForMainFrame(url);
   SimulateStartAndExpectProceed();
   SimulateCommitAndExpectProceed();
   EXPECT_EQ(ContentSubresourceFilterDriverFactory::ActivationDecision::
                 ACTIVATION_LIST_NOT_MATCHED,
             factory()->GetActivationDecisionForLastCommittedPageLoad());
   tester().ExpectTotalCount(kMatchesPatternHistogramNameSubresourceFilterSuffix,
@@ skipping 48 matching lines @@
   tester().ExpectUniqueSample(kNavigationChainSizeSubresourceFilterSuffix, 2,
                               1);
 }
 
 TEST_F(SubresourceFilterSafeBrowsingActivationThrottleTest,
        ListNotMatchedAndTimeout_NoActivation) {
   const GURL url(kURL);
   SimulateTimeout();
   CreateTestNavigationForMainFrame(url);
   SimulateStartAndExpectProceed();
+
+  // This call blocks the current task until the timeout occurs after a few

[Charlie Harrison, 2017/04/19 23:58:24]

Comment could be clarified.

+  // seconds. Get around this by posting a task which fires the timer.
+  auto fire_timer = [](base::MockTimer* timer) {
+    EXPECT_TRUE(timer->IsRunning());
+    EXPECT_EQ(SubresourceFilterSafeBrowsingClientRequest::kCheckURLTimeout,
+              timer->GetCurrentDelay());
+    timer->Fire();
+  };
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(fire_timer, base::Unretained(mock_timer())));
   SimulateCommitAndExpectProceed();
   EXPECT_EQ(ContentSubresourceFilterDriverFactory::ActivationDecision::
                 ACTIVATION_LIST_NOT_MATCHED,
             factory()->GetActivationDecisionForLastCommittedPageLoad());
   tester().ExpectTotalCount(kMatchesPatternHistogramNameSubresourceFilterSuffix,
                             0);
   tester().ExpectTotalCount(kNavigationChainSizeSubresourceFilterSuffix, 0);
+  tester().ExpectTotalCount(kSafeBrowsingNavigationDelay, 1);
+  tester().ExpectTimeBucketCount(kSafeBrowsingNavigationDelay,
+                                 base::TimeDelta::FromMilliseconds(0), 0);
 }
 
-// TODO(melandory): Once non-defering check in WillStart is implemented add one
-// more test that destroys the Navigation along with corresponding throttles
-// while the SB check is pending? (To be run by ASAN bots to ensure
-// no use-after-free.)
+TEST_F(SubresourceFilterSafeBrowsingActivationThrottleTest,
+       ListMatchedOnStart_NoDelay) {
+  const GURL url(kURL);
+  ConfigureAsSubresourceFilterOnlyURL(url);
+  CreateTestNavigationForMainFrame(url);
+  SimulateStartAndExpectProceed();
+
+  // Get the database result back before commit.
+  RunUntilIdle();
+
+  SimulateCommitAndExpectProceed();
+  EXPECT_EQ(
+      ContentSubresourceFilterDriverFactory::ActivationDecision::ACTIVATED,
+      factory()->GetActivationDecisionForLastCommittedPageLoad());
+  tester().ExpectUniqueSample(
+      kMatchesPatternHistogramNameSubresourceFilterSuffix, NO_REDIRECTS_HIT, 1);
+  tester().ExpectUniqueSample(kNavigationChainSizeSubresourceFilterSuffix, 1,
+                              1);
+  tester().ExpectTimeBucketCount(kSafeBrowsingNavigationDelay,
+                                 base::TimeDelta::FromMilliseconds(0), 1);
+}
+
+TEST_F(SubresourceFilterSafeBrowsingActivationThrottleTest,
+       ListMatchedOnRedirect_NoDelay) {
+  const GURL url(kURL);
+  const GURL redirect_url(kRedirectURL);
+  ConfigureAsSubresourceFilterOnlyURL(redirect_url);
+  CreateTestNavigationForMainFrame(url);
+
+  SimulateStartAndExpectProceed();
+  SimulateRedirectAndExpectProceed(redirect_url);
+
+  // Get the database result back before commit.
+  RunUntilIdle();
+
+  SimulateCommitAndExpectProceed();
+  EXPECT_EQ(
+      ContentSubresourceFilterDriverFactory::ActivationDecision::ACTIVATED,
+      factory()->GetActivationDecisionForLastCommittedPageLoad());
+  tester().ExpectUniqueSample(
+      kMatchesPatternHistogramNameSubresourceFilterSuffix, F0M0L1, 1);
+  tester().ExpectUniqueSample(kNavigationChainSizeSubresourceFilterSuffix, 2,
+                              1);
+  tester().ExpectTimeBucketCount(kSafeBrowsingNavigationDelay,
+                                 base::TimeDelta::FromMilliseconds(0), 1);
+}
+
+TEST_F(SubresourceFilterSafeBrowsingActivationThrottleTest,
+       ListMatchedOnStartWithRedirect_NoActivation) {
+  const GURL url(kURL);
+  const GURL redirect_url(kRedirectURL);
+  ConfigureAsSubresourceFilterOnlyURL(url);
+  CreateTestNavigationForMainFrame(url);
+
+  // These two lines also test how the database client reacts to two requests
+  // happening one after another.
+  SimulateStartAndExpectProceed();
+  SimulateRedirectAndExpectProceed(redirect_url);
+
+  // Get the database result back before commit.
+  RunUntilIdle();
+
+  SimulateCommitAndExpectProceed();
+  EXPECT_EQ(ContentSubresourceFilterDriverFactory::ActivationDecision::
+                ACTIVATION_LIST_NOT_MATCHED,
+            factory()->GetActivationDecisionForLastCommittedPageLoad());
+  tester().ExpectTotalCount(kMatchesPatternHistogramNameSubresourceFilterSuffix,
+                            0);
+  tester().ExpectTotalCount(kNavigationChainSizeSubresourceFilterSuffix, 0);
+  tester().ExpectTimeBucketCount(kSafeBrowsingNavigationDelay,
+                                 base::TimeDelta::FromMilliseconds(0), 1);
+}
+
+TEST_P(SubresourceFilterSafeBrowsingActivationThrottleTestWithCancelling,
+       Cancel) {
+  const GURL url(kURL);
+  SCOPED_TRACE(::testing::Message() << "CancelTime: " << cancel_time()
+                                    << " ResultSynchrony: " << result_sync());
+  ConfigureAsSubresourceFilterOnlyURL(url);
+  CreateTestNavigationForMainFrame(url);
+
+  content::NavigationThrottle::ThrottleCheckResult result = SimulateStart();
+  if (cancel_time() ==
+      content::CancellingNavigationThrottle::WILL_START_REQUEST) {
+    EXPECT_EQ(content::NavigationThrottle::CANCEL, result);
+    tester().ExpectTotalCount(kSafeBrowsingNavigationDelay, 0);
+    return;
+  }
+  EXPECT_EQ(content::NavigationThrottle::PROCEED, result);
+
+  result = SimulateRedirect(GURL(kRedirectURL));
+  if (cancel_time() ==
+      content::CancellingNavigationThrottle::WILL_REDIRECT_REQUEST) {
+    EXPECT_EQ(content::NavigationThrottle::CANCEL, result);
+    tester().ExpectTotalCount(kSafeBrowsingNavigationDelay, 0);
+    return;
+  }
+  EXPECT_EQ(content::NavigationThrottle::PROCEED, result);
+
+  base::RunLoop().RunUntilIdle();
+
+  result = SimulateCommit();
+  EXPECT_EQ(content::NavigationThrottle::CANCEL, result);
+  tester().ExpectTotalCount(kSafeBrowsingNavigationDelay, 0);
+}
+
+INSTANTIATE_TEST_CASE_P(
+    CancelMethod,
+    SubresourceFilterSafeBrowsingActivationThrottleTestWithCancelling,
+    ::testing::Combine(
+        ::testing::Values(
+            content::CancellingNavigationThrottle::WILL_START_REQUEST,
+            content::CancellingNavigationThrottle::WILL_REDIRECT_REQUEST,
+            content::CancellingNavigationThrottle::WILL_PROCESS_RESPONSE),
+        ::testing::Values(
+            content::CancellingNavigationThrottle::SYNCHRONOUS,
+            content::CancellingNavigationThrottle::ASYNCHRONOUS)));
 
 }  // namespace subresource_filter