Add default Filter Provider Manager (#18876)

* Add default filter provider manager
* Register ComponentFiltersProvider with FPM
* Use single AdblockFilterProviderMgr
* Move Add/RemoveProvider to constructor/destructor
* Remove extraneous Add/RemoveProvider calls
* Add methods to help with debugging
* Rm localhost_filters_provider()

browser/permissions/localhost_access_permission_browsertest.cc

@@ -433,13 +433,28 @@ IN_PROC_BROWSER_TEST_F(LocalhostAccessBrowserTest, AdblockRule) {
   // Add adblock rule to block localhost.
   std::string test_domain = "localhost";
   embedding_url_ = https_server_->GetURL(kTestEmbeddingDomain, kSimplePage);
-  const auto& target_url = localhost_server_->GetURL(test_domain, "/logo.png");
+  const auto& target_url =
+      localhost_server_->GetURL(test_domain, kTestTargetPath);
   auto rule = base::StrCat({"||", test_domain, "^"});
   AddAdblockRule(rule);
   // The image won't show up because of adblock rule.
   CheckNoPromptFlow(false, target_url);
 }
 
+// Test that badfiltering a localhost adblock rule makes permission come up.
+IN_PROC_BROWSER_TEST_F(LocalhostAccessBrowserTest, AdblockRuleBadfilter) {
+  std::string test_domain = "localhost";
+  embedding_url_ = https_server_->GetURL(kTestEmbeddingDomain, kSimplePage);
+  const auto& target_url =
+      localhost_server_->GetURL(test_domain, kTestTargetPath);
+
+  auto adblock_rule = base::StrCat({"||", test_domain, "^"});
+  auto badfilter_rule = base::StrCat({"\n", adblock_rule, "$badfilter"});
+  auto rules = adblock_rule + badfilter_rule;
+  AddAdblockRule(rules);
+  CheckAskAndAcceptFlow(target_url);
+}
+
 // Test that localhost connections from website not on allowlist
 // are blocked without permission prompt.
 IN_PROC_BROWSER_TEST_F(LocalhostAccessBrowserTest, WebsiteNotOnAllowlist) {
@@ -449,7 +464,8 @@ IN_PROC_BROWSER_TEST_F(LocalhostAccessBrowserTest, WebsiteNotOnAllowlist) {
   localhost_permission_component_->SetAllowedDomainsForTesting(
       {base::StrCat({kTestEmbeddingDomain, "\n", "#b.com"})});
   embedding_url_ = https_server_->GetURL("b.com", kSimplePage);
-  const auto& target_url = localhost_server_->GetURL(test_domain, "/logo.png");
+  const auto& target_url =
+      localhost_server_->GetURL(test_domain, kTestTargetPath);
   CheckNoPromptFlow(false, target_url);
 }
 
@@ -461,7 +477,8 @@ IN_PROC_BROWSER_TEST_F(LocalhostAccessBrowserTest,
   // Clear out the allowlist.
   localhost_permission_component_->SetAllowedDomainsForTesting({""});
   embedding_url_ = https_server_->GetURL(kTestEmbeddingDomain, kSimplePage);
-  const auto& target_url = localhost_server_->GetURL(test_domain, "/logo.png");
+  const auto& target_url =
+      localhost_server_->GetURL(test_domain, kTestTargetPath);
   SetCurrentStatus(ContentSetting::CONTENT_SETTING_ALLOW);
   // Load subresource, should succeed.
   InsertImage(target_url.spec(), true);
@@ -474,7 +491,8 @@ IN_PROC_BROWSER_TEST_F(LocalhostAccessBrowserTest, WebsitePartOfETLDP1) {
   std::string test_domain = "localhost";
   embedding_url_ = https_server_->GetURL(
       base::StrCat({"test1.", kTestEmbeddingDomain}), kSimplePage);
-  const auto& target_url = localhost_server_->GetURL(test_domain, "/logo.png");
+  const auto& target_url =
+      localhost_server_->GetURL(test_domain, kTestTargetPath);
   CheckAskAndAcceptFlow(target_url);
   embedding_url_ = https_server_->GetURL(
       base::StrCat({"test2.", kTestEmbeddingDomain}), kSimplePage);
@@ -487,7 +505,8 @@ IN_PROC_BROWSER_TEST_F(LocalhostAccessBrowserTest, AdblockRuleException) {
   // Add adblock rule to block localhost.
   std::string test_domain = "localhost";
   embedding_url_ = https_server_->GetURL(kTestEmbeddingDomain, kSimplePage);
-  const auto& target_url = localhost_server_->GetURL(test_domain, "/logo.png");
+  const auto& target_url =
+      localhost_server_->GetURL(test_domain, kTestTargetPath);
   auto original_rule = base::StrCat({"||", test_domain, "^", "\n"});
   auto exception_rule = base::StrCat({"@@||", test_domain, "^"});
   auto rules = original_rule + exception_rule;

components/brave_shields/browser/BUILD.gn

@@ -20,6 +20,8 @@ if (!is_ios) {
       "ad_block_filters_provider.h",
       "ad_block_filters_provider_manager.cc",
       "ad_block_filters_provider_manager.h",
+      "ad_block_localhost_filters_provider.cc",
+      "ad_block_localhost_filters_provider.h",
       "ad_block_pref_service.cc",
       "ad_block_pref_service.h",
       "ad_block_regional_service_manager.cc",

components/brave_shields/browser/ad_block_component_filters_provider.cc

@@ -12,6 +12,8 @@
 #include "base/files/file_path.h"
 #include "base/task/thread_pool.h"
 #include "brave/components/brave_shields/browser/ad_block_component_installer.h"
+#include "brave/components/brave_shields/browser/ad_block_filters_provider.h"
+#include "brave/components/brave_shields/browser/ad_block_filters_provider_manager.h"
 #include "brave/components/brave_shields/browser/filter_list_catalog_entry.h"
 #include "components/component_updater/component_updater_service.h"
 
@@ -23,8 +25,11 @@ AdBlockComponentFiltersProvider::AdBlockComponentFiltersProvider(
     component_updater::ComponentUpdateService* cus,
     std::string component_id,
     std::string base64_public_key,
-    std::string title)
-    : component_id_(component_id), component_updater_service_(cus) {
+    std::string title,
+    bool is_default_engine)
+    : AdBlockFiltersProvider(is_default_engine),
+      component_id_(component_id),
+      component_updater_service_(cus) {
   // Can be nullptr in unit tests
   if (cus) {
     RegisterAdBlockFiltersComponent(
@@ -34,15 +39,21 @@ AdBlockComponentFiltersProvider::AdBlockComponentFiltersProvider(
   }
 }
 
+std::string AdBlockComponentFiltersProvider::GetNameForDebugging() {
+  return "AdBlockComponentFiltersProvider";
+}
+
 AdBlockComponentFiltersProvider::AdBlockComponentFiltersProvider(
     component_updater::ComponentUpdateService* cus,
-    const FilterListCatalogEntry& catalog_entry)
+    const FilterListCatalogEntry& catalog_entry,
+    bool is_default_engine)
     : AdBlockComponentFiltersProvider(cus,
                                       catalog_entry.component_id,
                                       catalog_entry.base64_public_key,
-                                      catalog_entry.title) {}
+                                      catalog_entry.title,
+                                      is_default_engine) {}
 
-AdBlockComponentFiltersProvider::~AdBlockComponentFiltersProvider() = default;
+AdBlockComponentFiltersProvider::~AdBlockComponentFiltersProvider() {}
 
 void AdBlockComponentFiltersProvider::UnregisterComponent() {
   // Can be nullptr in unit tests

components/brave_shields/browser/ad_block_component_filters_provider.h

@@ -37,11 +37,13 @@ class AdBlockComponentFiltersProvider : public AdBlockFiltersProvider {
       component_updater::ComponentUpdateService* cus,
       std::string component_id,
       std::string base64_public_key,
-      std::string title);
+      std::string title,
+      bool is_default_engine = true);
   // Helper to build a particular adblock component from a catalog entry
   AdBlockComponentFiltersProvider(
       component_updater::ComponentUpdateService* cus,
-      const FilterListCatalogEntry& catalog_entry);
+      const FilterListCatalogEntry& catalog_entry,
+      bool is_default_engine = true);
   ~AdBlockComponentFiltersProvider() override;
   AdBlockComponentFiltersProvider(const AdBlockComponentFiltersProvider&) =
       delete;
@@ -56,6 +58,8 @@ class AdBlockComponentFiltersProvider : public AdBlockFiltersProvider {
   // is registered.
   void UnregisterComponent();
 
+  std::string GetNameForDebugging() override;
+
  private:
   friend class ::AdBlockServiceTest;
   friend class ::DebounceBrowserTest;

components/brave_shields/browser/ad_block_custom_filters_provider.cc

@@ -17,13 +17,9 @@ namespace brave_shields {
 
 AdBlockCustomFiltersProvider::AdBlockCustomFiltersProvider(
     PrefService* local_state)
-    : local_state_(local_state) {
-  AdBlockFiltersProviderManager::GetInstance()->AddProvider(this);
-}
+    : AdBlockFiltersProvider(false), local_state_(local_state) {}
 
-AdBlockCustomFiltersProvider::~AdBlockCustomFiltersProvider() {
-  AdBlockFiltersProviderManager::GetInstance()->RemoveProvider(this);
-}
+AdBlockCustomFiltersProvider::~AdBlockCustomFiltersProvider() {}
 
 void AdBlockCustomFiltersProvider::HideElementOnHost(
     const std::string& css_selector,
@@ -33,6 +29,10 @@ void AdBlockCustomFiltersProvider::HideElementOnHost(
                       '\n');
 }
 
+std::string AdBlockCustomFiltersProvider::GetNameForDebugging() {
+  return "AdBlockCustomFiltersProvider";
+}
+
 void AdBlockCustomFiltersProvider::CreateSiteExemption(
     const std::string& host) {
   std::string custom_filters = GetCustomFilters();

components/brave_shields/browser/ad_block_custom_filters_provider.h

@@ -42,6 +42,8 @@ class AdBlockCustomFiltersProvider : public AdBlockFiltersProvider {
   // AdBlockFiltersProvider
   void AddObserver(AdBlockFiltersProvider::Observer* observer);
 
+  std::string GetNameForDebugging() override;
+
  private:
   const raw_ptr<PrefService> local_state_;
 

components/brave_shields/browser/ad_block_engine.cc

@@ -92,7 +92,9 @@ std::string ResourceTypeToString(blink::mojom::ResourceType resource_type) {
 
 namespace brave_shields {
 
-AdBlockEngine::AdBlockEngine() : ad_block_client_(new adblock::Engine()) {
+AdBlockEngine::AdBlockEngine(bool is_default_engine)
+    : ad_block_client_(new adblock::Engine()),
+      is_default_engine_(is_default_engine) {
   DETACH_FROM_SEQUENCE(sequence_checker_);
 }
 

components/brave_shields/browser/ad_block_engine.h

@@ -42,11 +42,13 @@ class AdBlockEngine : public base::SupportsWeakPtr<AdBlockEngine> {
   using GetDATFileDataResult =
       brave_component_updater::LoadDATFileDataResult<adblock::Engine>;
 
-  AdBlockEngine();
+  explicit AdBlockEngine(bool is_default_engine);
   AdBlockEngine(const AdBlockEngine&) = delete;
   AdBlockEngine& operator=(const AdBlockEngine&) = delete;
   ~AdBlockEngine();
 
+  bool IsDefaultEngine() { return is_default_engine_; }
+
   void ShouldStartRequest(const GURL& url,
                           blink::mojom::ResourceType resource_type,
                           const std::string& tab_host,
@@ -111,6 +113,8 @@ class AdBlockEngine : public base::SupportsWeakPtr<AdBlockEngine> {
 
   raw_ptr<TestObserver> test_observer_ = nullptr;
 
+  bool is_default_engine_;
+
   SEQUENCE_CHECKER(sequence_checker_);
 };
 

components/brave_shields/browser/ad_block_filters_provider.cc

@@ -7,11 +7,22 @@
 
 #include <utility>
 
+#include "brave/components/brave_shields/browser/ad_block_filters_provider_manager.h"
+
 namespace brave_shields {
 
+AdBlockFiltersProvider::AdBlockFiltersProvider(bool engine_is_default)
+    : engine_is_default_(engine_is_default) {
+  AdBlockFiltersProviderManager::GetInstance()->AddProvider(this,
+                                                            engine_is_default_);
+}
+
 AdBlockFiltersProvider::AdBlockFiltersProvider() = default;
 
-AdBlockFiltersProvider::~AdBlockFiltersProvider() = default;
+AdBlockFiltersProvider::~AdBlockFiltersProvider() {
+  AdBlockFiltersProviderManager::GetInstance()->RemoveProvider(
+      this, engine_is_default_);
+}
 
 void AdBlockFiltersProvider::AddObserver(
     AdBlockFiltersProvider::Observer* observer) {

components/brave_shields/browser/ad_block_filters_provider.h

@@ -6,6 +6,8 @@
 #ifndef BRAVE_COMPONENTS_BRAVE_SHIELDS_BROWSER_AD_BLOCK_FILTERS_PROVIDER_H_
 #define BRAVE_COMPONENTS_BRAVE_SHIELDS_BROWSER_AD_BLOCK_FILTERS_PROVIDER_H_
 
+#include <string>
+
 #include "base/functional/callback.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
@@ -25,6 +27,8 @@ class AdBlockFiltersProvider {
     virtual void OnChanged() = 0;
   };
 
+  explicit AdBlockFiltersProvider(bool engine_is_default);
+  // Used by AdblockFiltersProviderManager
   AdBlockFiltersProvider();
   AdBlockFiltersProvider(const AdBlockFiltersProvider&) = delete;
   AdBlockFiltersProvider& operator=(const AdBlockFiltersProvider&) = delete;
@@ -38,11 +42,14 @@ class AdBlockFiltersProvider {
 
   base::WeakPtr<AdBlockFiltersProvider> AsWeakPtr();
 
+  virtual std::string GetNameForDebugging() = 0;
+
  protected:
+  bool engine_is_default_;
+
   virtual void LoadDATBuffer(
       base::OnceCallback<void(bool deserialize,
                               const DATFileDataBuffer& dat_buf)>) = 0;
-
   void NotifyObservers();
 
  private:

components/brave_shields/browser/ad_block_filters_provider_manager.cc

@@ -13,6 +13,7 @@
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/no_destructor.h"
+#include "base/notreached.h"
 #include "base/task/sequenced_task_runner.h"
 
 namespace brave_shields {
@@ -44,38 +45,54 @@ AdBlockFiltersProviderManager::AdBlockFiltersProviderManager() = default;
 AdBlockFiltersProviderManager::~AdBlockFiltersProviderManager() = default;
 
 void AdBlockFiltersProviderManager::AddProvider(
-    AdBlockFiltersProvider* provider) {
-  auto rv = filters_providers_.insert(provider);
+    AdBlockFiltersProvider* provider,
+    bool is_for_default_engine) {
+  auto rv = is_for_default_engine
+                ? default_engine_filters_providers_.insert(provider)
+                : additional_engine_filters_providers_.insert(provider);
   DCHECK(rv.second);
   provider->AddObserver(this);
 }
 
 void AdBlockFiltersProviderManager::RemoveProvider(
-    AdBlockFiltersProvider* provider) {
-  auto it = filters_providers_.find(provider);
-  DCHECK(it != filters_providers_.end());
-  (*it)->RemoveObserver(this);
-  filters_providers_.erase(it);
+    AdBlockFiltersProvider* provider,
+    bool is_for_default_engine) {
+  auto& filters_providers = is_for_default_engine
+                                ? default_engine_filters_providers_
+                                : additional_engine_filters_providers_;
+  auto it = filters_providers.find(provider);
+  DCHECK(it != filters_providers.end());
+  filters_providers.erase(it);
   NotifyObservers();
 }
 
+std::string AdBlockFiltersProviderManager::GetNameForDebugging() {
+  return "AdBlockCustomFiltersProvider";
+}
+
 void AdBlockFiltersProviderManager::OnChanged() {
   NotifyObservers();
 }
 
+// Use LoadDATBufferForEngine instead, for Filter Provider Manager.
 void AdBlockFiltersProviderManager::LoadDATBuffer(
     base::OnceCallback<void(bool deserialize, const DATFileDataBuffer& dat_buf)>
         cb) {
-  if (task_tracker_.HasTrackedTasks()) {
-    // There's already an in-progress load, cancel it.
-    task_tracker_.TryCancelAll();
-  }
+  NOTREACHED();
+}
 
+void AdBlockFiltersProviderManager::LoadDATBufferForEngine(
+    bool is_for_default_engine,
+    base::OnceCallback<void(bool deserialize, const DATFileDataBuffer& dat_buf)>
+        cb) {
+  auto& filters_providers = is_for_default_engine
+                                ? default_engine_filters_providers_
+                                : additional_engine_filters_providers_;
   const auto collect_and_merge = base::BarrierCallback<DATFileDataBuffer>(
-      filters_providers_.size(),
+      filters_providers.size(),
       base::BindOnce(&AdBlockFiltersProviderManager::FinishCombinating,
                      weak_factory_.GetWeakPtr(), std::move(cb)));
-  for (auto* provider : filters_providers_) {
+  for (auto* const provider : filters_providers) {
     task_tracker_.PostTask(
         base::SequencedTaskRunner::GetCurrentDefault().get(), FROM_HERE,
         base::BindOnce(