Verify the ruleset earlier on if AdsTagging is running

Bug: 817365
Change-Id: I106d6b49c78f66b47a37d7f429b62b506ea5a225
Reviewed-on: https://chromium-review.googlesource.com/951823
Commit-Queue: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542132}

chrome/browser/subresource_filter/subresource_filter_browsertest.cc

@@ -22,6 +22,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/histogram_tester.h"
+#include "base/test/scoped_feature_list.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/metrics/subprocess_metrics_provider.h"
 #include "chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer.h"
@@ -69,6 +70,10 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
+namespace subresource_filter {
+
+using subresource_filter::testing::TestRulesetPair;
+
 namespace {
 
 namespace proto = url_pattern_index::proto;
@@ -132,12 +137,46 @@ GURL GetURLWithFragment(const GURL& url, base::StringPiece fragment) {
   return url.ReplaceComponents(replacements);
 }
 
-}  // namespace
+void OpenAndPublishRuleset(ContentRulesetService* content_ruleset_service,
+                           const base::FilePath& path) {
+  base::File index_file;
+  base::RunLoop open_loop;
+  auto open_callback = base::BindRepeating(
+      [](base::OnceClosure quit_closure, base::File* out, base::File result) {
+        *out = std::move(result);
+        std::move(quit_closure).Run();
+      },
+      open_loop.QuitClosure(), &index_file);
+  content_ruleset_service->TryOpenAndSetRulesetFile(path,
+                                                    std::move(open_callback));
+  open_loop.Run();
+  ASSERT_TRUE(index_file.IsValid());
+  content_ruleset_service->PublishNewRulesetVersion(std::move(index_file));
+}
 
-namespace subresource_filter {
+RulesetVerificationStatus GetRulesetVerification() {
+  ContentRulesetService* service =
+      g_browser_process->subresource_filter_ruleset_service();
+  VerifiedRulesetDealer::Handle* dealer_handle = service->ruleset_dealer();
+
+  auto callback_method = [](base::OnceClosure quit_closure,
+                            RulesetVerificationStatus* status,
+                            VerifiedRulesetDealer* verified_dealer) {
+    *status = verified_dealer->status();
+    std::move(quit_closure).Run();
+  };
+
+  RulesetVerificationStatus status;
+  base::RunLoop run_loop;
+  auto callback =
+      base::BindRepeating(callback_method, run_loop.QuitClosure(), &status);
+
+  dealer_handle->GetDealerAsync(callback);
+  run_loop.Run();
+  return status;
+}
 
-using subresource_filter::testing::TestRulesetCreator;
-using subresource_filter::testing::TestRulesetPair;
+}  // namespace
 
 // Tests -----------------------------------------------------------------------
 
@@ -654,8 +693,7 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
   ContentRulesetService* service =
       g_browser_process->subresource_filter_ruleset_service();
   ASSERT_TRUE(service->ruleset_dealer());
-  service->PublishNewRulesetVersion(
-      testing::TestRuleset::Open(test_ruleset_pair.indexed));
+  OpenAndPublishRuleset(service, test_ruleset_pair.indexed.path);
 
   auto ruleset_handle =
       std::make_unique<VerifiedRuleset::Handle>(service->ruleset_dealer());
@@ -667,6 +705,27 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
                                         receiver.GetCallback());
   receiver.WaitForActivationDecision();
   receiver.ExpectReceivedOnce(ActivationState(ActivationLevel::DISABLED));
+  RulesetVerificationStatus dealer_status = GetRulesetVerification();
+  EXPECT_EQ(RulesetVerificationStatus::CORRUPT, dealer_status);
+}
+
+IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, LazyRulesetValidation) {
+  // The ruleset shouldn't be validated until it's used.
+  SetRulesetToDisallowURLsWithPathSuffix("included_script.js");
+  RulesetVerificationStatus dealer_status = GetRulesetVerification();
+  EXPECT_EQ(RulesetVerificationStatus::NOT_VERIFIED, dealer_status);
+}
+
+IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
+                       AdsTaggingImmediateRulesetValidation) {
+  // When Ads Tagging is enabled, the ruleset should be validated as soon as
+  // it's published.
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(subresource_filter::kAdTagging);
+
+  SetRulesetToDisallowURLsWithPathSuffix("included_script.js");
+  RulesetVerificationStatus dealer_status = GetRulesetVerification();
+  EXPECT_EQ(RulesetVerificationStatus::INTACT, dealer_status);
 }
 
 IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, PageLoadMetrics) {

components/subresource_filter/content/browser/content_ruleset_service.cc

@@ -6,12 +6,14 @@
 
 #include <utility>
 
+#include "base/feature_list.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/sequenced_task_runner.h"
 #include "base/task_scheduler/post_task.h"
 #include "components/subresource_filter/content/common/subresource_filter_messages.h"
 #include "components/subresource_filter/core/browser/ruleset_service.h"
+#include "components/subresource_filter/core/browser/subresource_filter_features.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
@@ -85,6 +87,14 @@ void ContentRulesetService::PublishNewRulesetVersion(base::File ruleset_data) {
   DCHECK(ruleset_data.IsValid());
   CloseFileOnFileThread(&ruleset_data_);
 
+  // If Ad Tagging is running, then every request does a lookup and it's
+  // important that we verify the ruleset early on.
+  if (base::FeatureList::IsEnabled(kAdTagging)) {
+    // Even though the handle will immediately be destroyed, it will still
+    // validate the ruleset on its task runner.
+    VerifiedRuleset::Handle ruleset_handle(ruleset_dealer_.get());
+  }
+
   ruleset_data_ = std::move(ruleset_data);
   for (auto it = content::RenderProcessHost::AllHostsIterator(); !it.IsAtEnd();
        it.Advance()) {