Migrate EngineTestObserver -> AdBlockServiceTestObserver (#35854)

EngineTestObserver runs on the adblock task runner so it's unsafe to get callbacks on the main thread for the test runner

browser/brave_shields/ad_block_browser_test_helper.cc

@@ -14,8 +14,8 @@
 #include "base/test/thread_test_helper.h"
 #include "brave/browser/brave_browser_process.h"
 #include "brave/components/brave_shields/content/browser/ad_block_service.h"
+#include "brave/components/brave_shields/content/test/ad_block_service_test_observer.h"
 #include "brave/components/brave_shields/content/test/ad_block_unit_test_helper.h"
-#include "brave/components/brave_shields/content/test/engine_test_observer.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 
 namespace brave_shields {
@@ -39,23 +39,22 @@ AdBlockBrowserTestHelper::AdBlockBrowserTestHelper(
 AdBlockBrowserTestHelper::~AdBlockBrowserTestHelper() = default;
 
 void AdBlockBrowserTestHelper::WaitForAdBlockEngineInitialLoad() {
-  if (initial_engine_observer_) {
-    initial_engine_observer_->Wait();
-    initial_engine_observer_.reset();
+  if (initial_observer_) {
+    initial_observer_->WaitForDefault();
+    initial_observer_.reset();
   }
 }
 
 void AdBlockBrowserTestHelper::SetUpAdBlockService(
     content::BrowserContext* context) {
-  if (!initial_engine_observer_) {
+  if (!initial_observer_) {
     // Attach before SetupAdBlockServiceForTesting posts
     // SetFilterListCatalog({}) so the observer is guaranteed to catch the
-    // resulting OnEngineUpdated. Tests that want to wait on the initial build
-    // call WaitForAdBlockEngineInitialLoad() before registering any
+    // resulting OnFilterListLoaded. Tests that want to wait on the initial
+    // build call WaitForAdBlockEngineInitialLoad() before registering any
     // TestFiltersProvider; other tests simply ignore it.
-    initial_engine_observer_ = std::make_unique<EngineTestObserver>(
-        &g_brave_browser_process->ad_block_service()
-             ->GetDefaultEngineForTesting());
+    initial_observer_ = std::make_unique<AdBlockServiceTestObserver>(
+        g_brave_browser_process->ad_block_service());
   }
   SetupAdBlockServiceForTesting(g_brave_browser_process->ad_block_service());
   callback_.Run();

browser/brave_shields/ad_block_browser_test_helper.h

@@ -12,14 +12,14 @@
 #include "base/functional/callback_forward.h"
 #include "base/functional/callback_helpers.h"
 
-class EngineTestObserver;
-
 namespace content {
 class BrowserContext;
 }
 
 namespace brave_shields {
 
+class AdBlockServiceTestObserver;
+
 // Runs the ad block service's task runner until idle.
 bool WaitForAdBlockServiceThreads();
 
@@ -35,10 +35,10 @@ class AdBlockBrowserTestHelper {
   AdBlockBrowserTestHelper& operator=(const AdBlockBrowserTestHelper&) = delete;
   ~AdBlockBrowserTestHelper();
 
-  // Blocks until the initial empty-catalog engine update fires. Idempotent —
-  // calling more than once is a no-op. Call this before registering any
-  // TestFiltersProvider so the subsequent EngineTestObserver only wakes for
-  // the rule load (not for a stale initial update).
+  // Blocks until the initial empty-catalog default-engine filter list load
+  // fires. Idempotent — calling more than once is a no-op. Call this before
+  // registering any TestFiltersProvider so the subsequent observer only wakes
+  // for the rule load (not for a stale initial update).
   //
   // NOT safe with the DAT cache feature enabled — a cached DAT load may
   // suppress the initial filter set build, making this wait hang. DAT cache
@@ -49,7 +49,7 @@ class AdBlockBrowserTestHelper {
   void SetUpAdBlockService(content::BrowserContext* context);
 
   base::RepeatingClosure callback_;
-  std::unique_ptr<EngineTestObserver> initial_engine_observer_;
+  std::unique_ptr<AdBlockServiceTestObserver> initial_observer_;
   base::CallbackListSubscription create_services_subscription_;
 };
 

browser/brave_shields/ad_block_service_browsertest.cc

@@ -36,7 +36,7 @@
 #include "brave/components/brave_shields/content/browser/ad_block_service.h"
 #include "brave/components/brave_shields/content/browser/ad_block_subscription_service_manager.h"
 #include "brave/components/brave_shields/content/browser/ad_block_subscription_service_manager_observer.h"
-#include "brave/components/brave_shields/content/test/engine_test_observer.h"
+#include "brave/components/brave_shields/content/test/ad_block_service_test_observer.h"
 #include "brave/components/brave_shields/content/test/test_filters_provider.h"
 #include "brave/components/brave_shields/core/browser/ad_block_component_service_manager.h"
 #include "brave/components/brave_shields/core/browser/ad_block_default_resource_provider.h"
@@ -185,6 +185,7 @@ void AdBlockServiceTest::PreRunTestOnMainThread() {
 }
 
 void AdBlockServiceTest::TearDownOnMainThread() {
+  ad_block_test_helper_.reset();
   source_providers_.clear();
   temp_dirs_.clear();
   // Unset the host resolver so as not to interfere with later tests.
@@ -230,11 +231,8 @@ void AdBlockServiceTest::AddNewRules(const std::string& rules,
   source_provider->RegisterAsSourceProvider(ad_block_service);
   source_providers_.push_back(std::move(source_provider));
 
-  auto& engine = first_party_protections
-                     ? ad_block_service->GetDefaultEngineForTesting()
-                     : ad_block_service->GetAdditionalFiltersEngineForTesting();
-  EngineTestObserver engine_observer(&engine);
-  engine_observer.Wait();
+  brave_shields::AdBlockServiceTestObserver observer(ad_block_service);
+  observer.Wait(first_party_protections);
 }
 
 // Returns the path of the new directory, not the file. Intended for use with
@@ -299,9 +297,8 @@ void AdBlockServiceTest::UpdateAdBlockInstanceWithRules(
   EXPECT_TRUE(provider);
   provider->OnComponentReady(component_path);
 
-  auto& engine = service->GetDefaultEngineForTesting();
-  EngineTestObserver engine_observer(&engine);
-  engine_observer.Wait();
+  brave_shields::AdBlockServiceTestObserver observer(service);
+  observer.WaitForDefault();
 }
 
 void AdBlockServiceTest::EnableDeveloperMode(bool enabled) {
@@ -315,9 +312,8 @@ void AdBlockServiceTest::UpdateCustomAdBlockInstanceWithRules(
       g_brave_browser_process->ad_block_service();
   ad_block_service->custom_filters_provider()->UpdateCustomFilters(rules);
 
-  auto& engine = ad_block_service->GetAdditionalFiltersEngineForTesting();
-  EngineTestObserver engine_observer(&engine);
-  engine_observer.Wait();
+  brave_shields::AdBlockServiceTestObserver observer(ad_block_service);
+  observer.WaitForAdditional();
 }
 
 void AdBlockServiceTest::AssertTagExists(const std::string& tag,
@@ -378,11 +374,8 @@ void AdBlockServiceTest::InstallComponent(
     EXPECT_TRUE(provider);
     provider->OnComponentReady(component_path);
 
-    auto& engine = catalog_entry.first_party_protections
-                       ? service->GetDefaultEngineForTesting()
-                       : service->GetAdditionalFiltersEngineForTesting();
-    EngineTestObserver engine_observer(&engine);
-    engine_observer.Wait();
+    brave_shields::AdBlockServiceTestObserver observer(service);
+    observer.Wait(catalog_entry.first_party_protections);
   }
 }
 

browser/brave_shields/brave_shields_web_contents_observer_browsertest.cc

@@ -115,6 +115,7 @@ class BraveShieldsWebContentsObserverBrowserTest : public InProcessBrowserTest {
   }
 
   void TearDownOnMainThread() override {
+    helper_.reset();
     BraveShieldsWebContentsObserver::SetReceiverImplForTesting(nullptr);
   }
 

browser/debounce/debounce_browsertest.cc

@@ -14,7 +14,7 @@
 #include "brave/browser/brave_shields/ad_block_browser_test_helper.h"
 #include "brave/browser/extensions/brave_base_local_data_files_browsertest.h"
 #include "brave/components/brave_shields/content/browser/ad_block_service.h"
-#include "brave/components/brave_shields/content/test/engine_test_observer.h"
+#include "brave/components/brave_shields/content/test/ad_block_service_test_observer.h"
 #include "brave/components/brave_shields/content/test/test_filters_provider.h"
 #include "brave/components/brave_shields/core/browser/brave_shields_utils.h"
 #include "brave/components/debounce/core/browser/debounce_component_installer.h"
@@ -194,10 +194,9 @@ class DebounceBrowserTest : public BaseLocalDataFilesBrowserTest {
     source_provider->RegisterAsSourceProvider(
         g_brave_browser_process->ad_block_service());
     source_providers_.push_back(std::move(source_provider));
-    auto& engine = g_brave_browser_process->ad_block_service()
-                       ->GetDefaultEngineForTesting();
-    EngineTestObserver engine_observer(&engine);
-    engine_observer.Wait();
+    brave_shields::AdBlockServiceTestObserver observer(
+        g_brave_browser_process->ad_block_service());
+    observer.WaitForDefault();
   }
 
   void PostRunTestOnMainThread() override {
@@ -205,6 +204,13 @@ class DebounceBrowserTest : public BaseLocalDataFilesBrowserTest {
     BaseLocalDataFilesBrowserTest::PostRunTestOnMainThread();
   }
 
+  void TearDownOnMainThread() override {
+    // Reset before service shutdown so the helper's observer detaches while
+    // the service is still alive.
+    ad_block_test_helper_.reset();
+    BaseLocalDataFilesBrowserTest::TearDownOnMainThread();
+  }
+
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
   std::vector<std::unique_ptr<brave_shields::TestFiltersProvider>>

browser/ephemeral_storage/ephemeral_storage_1p_domain_block_browsertest.cc

@@ -16,7 +16,7 @@
 #include "brave/components/brave_component_updater/browser/local_data_files_service.h"
 #include "brave/components/brave_shields/content/browser/ad_block_engine.h"
 #include "brave/components/brave_shields/content/browser/ad_block_service.h"
-#include "brave/components/brave_shields/content/test/engine_test_observer.h"
+#include "brave/components/brave_shields/content/test/ad_block_service_test_observer.h"
 #include "brave/components/brave_shields/content/test/test_filters_provider.h"
 #include "brave/components/brave_shields/core/browser/brave_shields_utils.h"
 #include "brave/components/brave_shields/core/common/features.h"
@@ -45,6 +45,11 @@ class EphemeralStorage1pDomainBlockBrowserTest
     helper_ = std::make_unique<brave_shields::AdBlockBrowserTestHelper>();
   }
 
+  void TearDownOnMainThread() override {
+    helper_.reset();
+    EphemeralStorageBrowserTest::TearDownOnMainThread();
+  }
+
   void SetUpOnMainThread() override {
     EphemeralStorageBrowserTest::SetUpOnMainThread();
     a_site_simple_url_ = https_server_.GetURL("a.com", "/simple.html");
@@ -60,10 +65,8 @@ class EphemeralStorage1pDomainBlockBrowserTest
         g_brave_browser_process->ad_block_service();
     source_provider_->RegisterAsSourceProvider(ad_block_service);
 
-    auto& engine = g_brave_browser_process->ad_block_service()
-                       ->GetDefaultEngineForTesting();
-    EngineTestObserver engine_observer(&engine);
-    engine_observer.Wait();
+    brave_shields::AdBlockServiceTestObserver observer(ad_block_service);
+    observer.WaitForDefault();
   }
 
   void BlockDomainByURL(const GURL& url) {

browser/permissions/localhost_access_permission_browsertest.cc

@@ -130,7 +130,11 @@ class LocalhostAccessBrowserTest
     InProcessBrowserTest::TearDownInProcessBrowserTestFixture();
   }
 
-  void TearDownOnMainThread() override { prompt_factory_.reset(); }
+  void TearDownOnMainThread() override {
+    ad_block_test_helper_.reset();
+    prompt_factory_.reset();
+    InProcessBrowserTest::TearDownOnMainThread();
+  }
 
   permissions::MockPermissionPromptFactory* prompt_factory() {
     return prompt_factory_.get();

components/brave_perf_predictor/browser/perf_predictor_tab_helper_browsertest.cc

@@ -75,6 +75,11 @@ class PerfPredictorTabHelperTest : public InProcessBrowserTest {
 
   void TearDown() override { InProcessBrowserTest::TearDown(); }
 
+  void TearDownOnMainThread() override {
+    ad_block_test_helper_.reset();
+    InProcessBrowserTest::TearDownOnMainThread();
+  }
+
   void InitEmbeddedTestServer() {
     base::FilePath test_data_dir;
     base::PathService::Get(brave::DIR_TEST_DATA, &test_data_dir);

components/brave_shields/content/browser/ad_block_engine.cc

@@ -270,9 +270,6 @@ void AdBlockEngine::UpdateAdBlockClient(
   }
   UseResources(storage);
   AddKnownTagsToAdBlockInstance();
-  if (test_observer_) {
-    test_observer_->OnEngineUpdated();
-  }
 }
 
 void AdBlockEngine::AddKnownTagsToAdBlockInstance() {
@@ -381,12 +378,4 @@ DATFileDataBuffer AdBlockEngine::Serialize() {
   return base::ToVector(ad_block_client_->serialize());
 }
 
-void AdBlockEngine::AddObserverForTest(AdBlockEngine::TestObserver* observer) {
-  test_observer_ = observer;
-}
-
-void AdBlockEngine::RemoveObserverForTest() {
-  test_observer_ = nullptr;
-}
-
 }  // namespace brave_shields

components/brave_shields/content/browser/ad_block_engine.h

@@ -15,7 +15,6 @@
 #include <utility>
 #include <vector>
 
-#include "base/observer_list_types.h"
 #include "base/sequence_checker.h"
 #include "base/values.h"
 #include "brave/components/brave_component_updater/browser/dat_file_util.h"
@@ -74,14 +73,6 @@ class AdBlockEngine {
 
   DATFileDataBuffer Serialize();
 
-  class TestObserver : public base::CheckedObserver {
-   public:
-    virtual void OnEngineUpdated() = 0;
-  };
-
-  void AddObserverForTest(TestObserver* observer);
-  void RemoveObserverForTest();
-
  protected:
   void AddKnownTagsToAdBlockInstance();
   void UpdateAdBlockClient(rust::Box<adblock::Engine> ad_block_client,
@@ -107,8 +98,6 @@ class AdBlockEngine {
   std::optional<adblock::RegexManagerDiscardPolicy> regex_discard_policy_
       GUARDED_BY_CONTEXT(sequence_checker_);
 
-  raw_ptr<TestObserver> test_observer_ = nullptr;
-
   bool is_default_engine_;
 
   SEQUENCE_CHECKER(sequence_checker_);

components/brave_shields/content/test/BUILD.gn

@@ -9,10 +9,10 @@ source_set("test_support") {
   testonly = true
 
   sources = [
+    "ad_block_service_test_observer.cc",
+    "ad_block_service_test_observer.h",
     "ad_block_unit_test_helper.cc",
     "ad_block_unit_test_helper.h",
-    "engine_test_observer.cc",
-    "engine_test_observer.h",
     "test_filters_provider.cc",
     "test_filters_provider.h",
   ]

components/brave_shields/content/test/ad_block_service_test_observer.cc

@@ -0,0 +1,39 @@
+// Copyright (c) 2026 The Brave Authors. All rights reserved.
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at https://mozilla.org/MPL/2.0/.
+
+#include "brave/components/brave_shields/content/test/ad_block_service_test_observer.h"
+
+namespace brave_shields {
+
+AdBlockServiceTestObserver::AdBlockServiceTestObserver(
+    AdBlockService* service) {
+  observation_.Observe(service);
+}
+
+AdBlockServiceTestObserver::~AdBlockServiceTestObserver() = default;
+
+void AdBlockServiceTestObserver::WaitForDefault() {
+  default_loop_.Run();
+}
+
+void AdBlockServiceTestObserver::WaitForAdditional() {
+  additional_loop_.Run();
+}
+
+void AdBlockServiceTestObserver::Wait(bool is_default_engine) {
+  if (is_default_engine) {
+    WaitForDefault();
+  } else {
+    WaitForAdditional();
+  }
+}
+
+void AdBlockServiceTestObserver::OnFilterListLoaded(
+    bool is_default_engine,
+    AdBlockService::FilterListLoadResult result) {
+  (is_default_engine ? default_loop_ : additional_loop_).Quit();
+}
+
+}  // namespace brave_shields

components/brave_shields/content/test/ad_block_service_test_observer.h

@@ -0,0 +1,49 @@
+// Copyright (c) 2026 The Brave Authors. All rights reserved.
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at https://mozilla.org/MPL/2.0/.
+
+#ifndef BRAVE_COMPONENTS_BRAVE_SHIELDS_CONTENT_TEST_AD_BLOCK_SERVICE_TEST_OBSERVER_H_
+#define BRAVE_COMPONENTS_BRAVE_SHIELDS_CONTENT_TEST_AD_BLOCK_SERVICE_TEST_OBSERVER_H_
+
+#include "base/run_loop.h"
+#include "base/scoped_observation.h"
+#include "brave/components/brave_shields/content/browser/ad_block_service.h"
+
+namespace brave_shields {
+
+// Blocking waiter for AdBlockService filter-list load events. Observes
+// AdBlockService::Observer::OnFilterListLoaded which fires on the UI sequence,
+// avoiding the cross-sequence race the engine-scoped test observer suffered
+// from. Single-use per engine.
+class AdBlockServiceTestObserver : public AdBlockService::Observer {
+ public:
+  explicit AdBlockServiceTestObserver(AdBlockService* service);
+  AdBlockServiceTestObserver(const AdBlockServiceTestObserver&) = delete;
+  AdBlockServiceTestObserver& operator=(const AdBlockServiceTestObserver&) =
+      delete;
+  ~AdBlockServiceTestObserver() override;
+
+  // Blocks until OnFilterListLoaded fires for the specified engine. Quit is
+  // latched so the event may fire before or after the call; Wait returns in
+  // either case.
+  void WaitForDefault();
+  void WaitForAdditional();
+
+  // Convenience for callers with a dynamic engine choice.
+  void Wait(bool is_default_engine);
+
+ private:
+  // AdBlockService::Observer:
+  void OnFilterListLoaded(bool is_default_engine,
+                          AdBlockService::FilterListLoadResult result) override;
+
+  base::RunLoop default_loop_;
+  base::RunLoop additional_loop_;
+  base::ScopedObservation<AdBlockService, AdBlockService::Observer>
+      observation_{this};
+};
+
+}  // namespace brave_shields
+
+#endif  // BRAVE_COMPONENTS_BRAVE_SHIELDS_CONTENT_TEST_AD_BLOCK_SERVICE_TEST_OBSERVER_H_

components/brave_shields/content/test/ad_block_unit_test_helper.cc

@@ -18,7 +18,7 @@ void SetupAdBlockServiceForTesting(AdBlockService* ad_block_service) {
   // gates are initialized synchronously, a test that registers a filter
   // provider immediately after this call ends up with two in-flight engine
   // builds (an empty one from the gate and a rule-containing one from the
-  // provider), and EngineTestObserver::Wait() can race and observe the empty
+  // provider), and the service-observer Wait can race and observe the empty
   // build. Posting defers the gate init until after the caller returns, so
   // the test's provider is already registered when the gate fires — the
   // manager then issues a single engine build containing the test's rules.

components/brave_shields/content/test/engine_test_observer.cc

@@ -1,24 +0,0 @@
-// Copyright (c) 2023 The Brave Authors. All rights reserved.
-// This Source Code Form is subject to the terms of the Mozilla Public
-// License, v. 2.0. If a copy of the MPL was not distributed with this file,
-// You can obtain one at https://mozilla.org/MPL/2.0/.
-
-#include "brave/components/brave_shields/content/test/engine_test_observer.h"
-
-EngineTestObserver::EngineTestObserver(brave_shields::AdBlockEngine* engine)
-    : engine_(engine) {
-  engine_->AddObserverForTest(this);
-}
-
-EngineTestObserver::~EngineTestObserver() {
-  engine_->RemoveObserverForTest();
-}
-
-// Blocks until the engine is updated
-void EngineTestObserver::Wait() {
-  run_loop_.Run();
-}
-
-void EngineTestObserver::OnEngineUpdated() {
-  run_loop_.Quit();
-}

components/brave_shields/content/test/engine_test_observer.h

@@ -1,34 +0,0 @@
-// Copyright (c) 2023 The Brave Authors. All rights reserved.
-// This Source Code Form is subject to the terms of the Mozilla Public
-// License, v. 2.0. If a copy of the MPL was not distributed with this file,
-// You can obtain one at https://mozilla.org/MPL/2.0/.
-
-#ifndef BRAVE_COMPONENTS_BRAVE_SHIELDS_CONTENT_TEST_ENGINE_TEST_OBSERVER_H_
-#define BRAVE_COMPONENTS_BRAVE_SHIELDS_CONTENT_TEST_ENGINE_TEST_OBSERVER_H_
-
-#include "base/run_loop.h"
-#include "brave/components/brave_shields/content/browser/ad_block_engine.h"
-
-// A test observer that allows blocking waits for an AdBlockEngine to be
-// updated with new rules.
-class EngineTestObserver : public brave_shields::AdBlockEngine::TestObserver {
- public:
-  // Constructs an EngineTestObserver which will observe the given adblock
-  // engine for filter data updates.
-  explicit EngineTestObserver(brave_shields::AdBlockEngine* engine);
-  ~EngineTestObserver() override;
-
-  EngineTestObserver(const EngineTestObserver& other);
-  EngineTestObserver& operator=(const EngineTestObserver& other);
-
-  // Blocks until the engine is updated
-  void Wait();
-
- private:
-  void OnEngineUpdated() override;
-
-  base::RunLoop run_loop_;
-  raw_ptr<brave_shields::AdBlockEngine> engine_ = nullptr;
-};
-
-#endif  // BRAVE_COMPONENTS_BRAVE_SHIELDS_CONTENT_TEST_ENGINE_TEST_OBSERVER_H_