Revert "Fix sequence checker dcheck assumptions for optimization guide filters"

This reverts commit 3132a551.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 838729 as the
culprit for flakes in the build cycles as shown on:
https://analysis.chromium.org/p/chromium/flake-portal/analysis/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vMzEzMmE1NTFiZTUzNGNiMTJlZTlkYzA1ZjdkOWRlYzZlMDMzZmIzMQw

Sample Failed Build: https://ci.chromium.org/b/8860248290928956432

Sample Failed Step: bf_cache_browser_tests

Sample Flaky Test: OptimizationGuideKeyedServiceBrowserTest.CheckForBlocklistFilter

Original change's description:
> 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/+/2600040
> Reviewed-by: Michael Crouse <mcrouse@chromium.org>
> Commit-Queue: Sophie Chang <sophiechang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#838729}


Change-Id: If61e0c71d69fd4d7ef865229c986bd890ea66527
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1160660
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2600537
Cr-Commit-Position: refs/heads/master@{#838781}

chrome/browser/optimization_guide/optimization_guide_keyed_service_browsertest.cc

@@ -483,6 +483,7 @@ 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());
@@ -504,6 +505,7 @@ 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/"));
@@ -524,49 +526,6 @@ 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,7 +51,9 @@ BloomFilter::BloomFilter(uint32_t num_hash_functions,
   memcpy(&bytes_[0], filter_data.data(), filter_data.size());
 }
 
-BloomFilter::~BloomFilter() = default;
+BloomFilter::~BloomFilter() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
 
 bool BloomFilter::Contains(const std::string& str) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

components/optimization_guide/optimization_filter.cc

@@ -28,10 +28,11 @@ OptimizationFilter::OptimizationFilter(
   DETACH_FROM_SEQUENCE(sequence_checker_);
 }
 
-OptimizationFilter::~OptimizationFilter() = default;
+OptimizationFilter::~OptimizationFilter() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
 
 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,7 +11,6 @@
 #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 {
@@ -59,6 +58,7 @@ 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,37 +73,6 @@ 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);
 }