Fix sequence checker dcheck assumptions for optimization guide filters

The bloom filter and optimization filter can get destructed on
background thread when a new component comes in after initial load

Bug: 1160660
Change-Id: Ia1dd5925d7513391148b677fb3e75b818a9fa190
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2600040Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838729}

chrome/browser/optimization_guide/optimization_guide_keyed_service_browsertest.cc

@@ -483,7 +483,6 @@ IN_PROC_BROWSER_TEST_F(
   PushHintsComponentAndWaitForCompletion();
   RegisterWithKeyedService();
 
-  ukm::TestAutoSetUkmRecorder ukm_recorder;
   base::HistogramTester histogram_tester;
 
   ui_test_utils::NavigateToURL(browser(), url_that_redirects_to_hints());
@@ -505,7 +504,6 @@ IN_PROC_BROWSER_TEST_F(OptimizationGuideKeyedServiceBrowserTest,
   PushHintsComponentAndWaitForCompletion();
   RegisterWithKeyedService();
 
-  ukm::TestAutoSetUkmRecorder ukm_recorder;
   base::HistogramTester histogram_tester;
 
   ui_test_utils::NavigateToURL(browser(), GURL("https://nohints.com/"));
@@ -526,6 +524,49 @@ IN_PROC_BROWSER_TEST_F(OptimizationGuideKeyedServiceBrowserTest,
       1);
 }
 
+IN_PROC_BROWSER_TEST_F(OptimizationGuideKeyedServiceBrowserTest,
+                       CheckForBlocklistFilter) {
+  PushHintsComponentAndWaitForCompletion();
+
+  OptimizationGuideKeyedService* ogks =
+      OptimizationGuideKeyedServiceFactory::GetForProfile(browser()->profile());
+
+  // Register an optimization type with an optimization filter.
+  ogks->RegisterOptimizationTypes({optimization_guide::proto::FAST_HOST_HINTS});
+  // Wait until filter is loaded.
+  base::RunLoop().RunUntilIdle();
+
+  base::HistogramTester histogram_tester;
+
+  EXPECT_EQ(optimization_guide::OptimizationGuideDecision::kFalse,
+            ogks->CanApplyOptimization(
+                GURL("https://blockedhost.com/whatever"),
+                optimization_guide::proto::FAST_HOST_HINTS, nullptr));
+  histogram_tester.ExpectUniqueSample(
+      "OptimizationGuide.ApplyDecision.FastHostHints",
+      static_cast<int>(optimization_guide::OptimizationTypeDecision::
+                           kNotAllowedByOptimizationFilter),
+      1);
+
+  // Register another type with optimization filter.
+  ogks->RegisterOptimizationTypes(
+      {optimization_guide::proto::LITE_PAGE_REDIRECT});
+  // Wait until filter is loaded.
+  base::RunLoop().RunUntilIdle();
+
+  // The previously loaded filter should still be loaded and give the same
+  // result.
+  EXPECT_EQ(optimization_guide::OptimizationGuideDecision::kFalse,
+            ogks->CanApplyOptimization(
+                GURL("https://blockedhost.com/whatever"),
+                optimization_guide::proto::FAST_HOST_HINTS, nullptr));
+  histogram_tester.ExpectUniqueSample(
+      "OptimizationGuide.ApplyDecision.FastHostHints",
+      static_cast<int>(optimization_guide::OptimizationTypeDecision::
+                           kNotAllowedByOptimizationFilter),
+      2);
+}
+
 class OptimizationGuideKeyedServiceDataSaverUserWithInfobarShownTest
     : public OptimizationGuideKeyedServiceBrowserTest {
  public:

components/optimization_guide/bloom_filter.cc

@@ -51,9 +51,7 @@ BloomFilter::BloomFilter(uint32_t num_hash_functions,
   memcpy(&bytes_[0], filter_data.data(), filter_data.size());
 }
 
-BloomFilter::~BloomFilter() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-}
+BloomFilter::~BloomFilter() = default;
 
 bool BloomFilter::Contains(const std::string& str) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

components/optimization_guide/optimization_filter.cc

@@ -28,11 +28,10 @@ OptimizationFilter::OptimizationFilter(
   DETACH_FROM_SEQUENCE(sequence_checker_);
 }
 
-OptimizationFilter::~OptimizationFilter() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-}
+OptimizationFilter::~OptimizationFilter() = default;
 
 bool OptimizationFilter::Matches(const GURL& url) const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return ContainsHostSuffix(url) || MatchesRegexp(url);
 }
 

components/optimization_guide/test_hints_component_creator.cc

@@ -11,6 +11,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/version.h"
+#include "components/optimization_guide/bloom_filter.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -58,7 +59,6 @@ TestHintsComponentCreator::CreateHintsComponentInfoWithPageHints(
     optimization_guide::proto::Optimization* optimization =
         page_hint->add_whitelisted_optimizations();
     optimization->set_optimization_type(optimization_type);
-
   }
 
   // Always stick something with no hint version in here.
@@ -73,6 +73,37 @@ TestHintsComponentCreator::CreateHintsComponentInfoWithPageHints(
   bad_version_hint->set_version("notaversion");
   bad_version_hint->add_page_hints()->set_page_pattern("*");
 
+  // Always stick an allowlist optimization filter in here.
+  optimization_guide::BloomFilter allowlist_bloom_filter(7, 511);
+  allowlist_bloom_filter.Add("allowedhost.com");
+  std::string allowlist_bloom_filter_data(
+      reinterpret_cast<const char*>(&allowlist_bloom_filter.bytes()[0]),
+      allowlist_bloom_filter.bytes().size());
+  optimization_guide::proto::OptimizationFilter* allowlist_optimization_filter =
+      config.add_optimization_allowlists();
+  allowlist_optimization_filter->set_optimization_type(
+      optimization_guide::proto::LITE_PAGE_REDIRECT);
+  allowlist_optimization_filter->mutable_bloom_filter()->set_num_hash_functions(
+      7);
+  allowlist_optimization_filter->mutable_bloom_filter()->set_num_bits(511);
+  allowlist_optimization_filter->mutable_bloom_filter()->set_data(
+      allowlist_bloom_filter_data);
+  // Always stick a blocklist optimization filter in here.
+  optimization_guide::BloomFilter blocklist_bloom_filter(7, 511);
+  blocklist_bloom_filter.Add("blockedhost.com");
+  std::string blocklist_bloom_filter_data(
+      reinterpret_cast<const char*>(&blocklist_bloom_filter.bytes()[0]),
+      blocklist_bloom_filter.bytes().size());
+  optimization_guide::proto::OptimizationFilter* blocklist_optimization_filter =
+      config.add_optimization_blacklists();
+  blocklist_optimization_filter->set_optimization_type(
+      optimization_guide::proto::FAST_HOST_HINTS);
+  blocklist_optimization_filter->mutable_bloom_filter()->set_num_hash_functions(
+      7);
+  blocklist_optimization_filter->mutable_bloom_filter()->set_num_bits(511);
+  blocklist_optimization_filter->mutable_bloom_filter()->set_data(
+      blocklist_bloom_filter_data);
+
   return WriteConfigToFileAndReturnHintsComponentInfo(config);
 }