DNR: Support specifying no static rulesets.

This CL:

- Allows extensions to specify no static Rulesets as part of the
"rule_resources" manifest key or optionally to omit the
"declarative_net_request" manifest key. Since we support dynamic rules,
we should allow this. Also, we plan to implement the ability to toggle
the set of enabled static rulesets, under which a state with no enabled
static rulesets should be allowed.

- Since we now allow extensions to not specify any static rulesets,
don't fail if an extension calls updateDynamicRules or getDynamicRules
if the extension doesn't have any registered rulesets.

- Also, move the book-keeping for the set of extensions with active
rulesets to RulesetManager from RulesMonitorService. RulesetManager
already maintains the set of active rulesets, so it should be the source
of truth for this info.

BUG=754526, 953894, 931967

Change-Id: I3d9c6eae5f4d0691b8272ffa4e81c6791f273c65
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2125691Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756095}

chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc

@@ -325,21 +325,20 @@ class DeclarativeNetRequestBrowserTest
 
   void set_config_flags(unsigned flags) { flags_ = flags; }
 
-  // Loads an extension with the given declarative |rules| in the given
-  // |directory|. Generates a fatal failure if the extension failed to load.
-  // |hosts| specifies the host permissions, the extensions should
-  // have.
-  void LoadExtensionWithRules(const std::vector<TestRule>& rules,
-                              const std::string& directory,
-                              const std::vector<std::string>& hosts) {
+  // Loads an extension with the given |rulesets| in the given |directory|.
+  // Generates a fatal failure if the extension failed to load. |hosts|
+  // specifies the host permissions the extensions should have.
+  void LoadExtensionWithRulesets(const std::vector<TestRulesetInfo>& rulesets,
+                                 const std::string& directory,
+                                 const std::vector<std::string>& hosts,
+                                 size_t expected_rules_count) {
     base::ScopedAllowBlockingForTesting scoped_allow_blocking;
     base::HistogramTester tester;
 
     base::FilePath extension_dir = temp_dir_.GetPath().AppendASCII(directory);
     EXPECT_TRUE(base::CreateDirectory(extension_dir));
 
-    TestRulesetInfo info = {kJSONRulesFilename, std::move(*ToListValue(rules))};
-    WriteManifestAndRuleset(extension_dir, info, hosts, flags_);
+    WriteManifestAndRulesets(extension_dir, rulesets, hosts, flags_);
 
     background_page_ready_listener_->Reset();
     const Extension* extension = nullptr;
@@ -358,18 +357,22 @@ class DeclarativeNetRequestBrowserTest
     // Ensure the ruleset is also loaded on the IO thread.
     content::RunAllTasksUntilIdle();
 
+    size_t expected_histogram_counts = rulesets.empty() ? 0 : 1;
+
     // The histograms below are not logged for unpacked extensions.
     if (GetParam() == ExtensionLoadType::PACKED) {
       tester.ExpectTotalCount(kIndexAndPersistRulesTimeHistogram,
-                              1 /* count */);
+                              expected_histogram_counts);
       tester.ExpectBucketCount(kManifestRulesCountHistogram,
-                               rules.size() /*sample*/, 1 /* count */);
+                               expected_rules_count /*sample*/,
+                               expected_histogram_counts);
     }
     tester.ExpectTotalCount(
-        "Extensions.DeclarativeNetRequest.CreateVerifiedMatcherTime", 1);
+        "Extensions.DeclarativeNetRequest.CreateVerifiedMatcherTime",
+        expected_histogram_counts);
     tester.ExpectUniqueSample(
         "Extensions.DeclarativeNetRequest.LoadRulesetResult",
-        RulesetMatcher::kLoadSuccess /*sample*/, 1 /*count*/);
+        RulesetMatcher::kLoadSuccess /*sample*/, expected_histogram_counts);
 
     EXPECT_TRUE(AreAllIndexedStaticRulesetsValid(*extension, profile()));
 
@@ -381,6 +384,16 @@ class DeclarativeNetRequestBrowserTest
     EXPECT_TRUE(LoadErrorReporter::GetInstance()->GetErrors()->empty());
   }
 
+  // Specialization of LoadExtensionWithRulesets above for an extension with a
+  // single static ruleset.
+  void LoadExtensionWithRules(const std::vector<TestRule>& rules,
+                              const std::string& directory,
+                              const std::vector<std::string>& hosts) {
+    std::vector<TestRulesetInfo> rulesets;
+    rulesets.push_back({kJSONRulesFilename, std::move(*ToListValue(rules))});
+    LoadExtensionWithRulesets(rulesets, directory, hosts, rules.size());
+  }
+
   void LoadExtensionWithRules(const std::vector<TestRule>& rules) {
     LoadExtensionWithRules(rules, "test_extension", {} /* hosts */);
   }
@@ -1597,6 +1610,44 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest_Packed,
       "example.com", "/pages_with_script/page2.html")));
 }
 
+// Tests than an extension can omit the "declarative_net_request" manifest key
+// but can still use dynamic rules.
+IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest, ZeroRulesets) {
+  set_config_flags(ConfigFlag::kConfig_HasBackgroundScript |
+                   ConfigFlag::kConfig_OmitDeclarativeNetRequestKey);
+
+  // Set-up an observer for RulesetMatcher to monitor the number of extension
+  // rulesets.
+  RulesetCountWaiter ruleset_count_waiter(ruleset_manager());
+
+  ASSERT_NO_FATAL_FAILURE(
+      LoadExtensionWithRulesets({} /* rulesets */, "extension_directory",
+                                {} /* hosts */, 0 /* expected_rules_count */));
+  const ExtensionId extension_id = last_loaded_extension_id();
+  EXPECT_EQ(0u, ruleset_manager()->GetMatcherCountForTest());
+
+  const GURL url = embedded_test_server()->GetURL(
+      "example.com", "/pages_with_script/page.html");
+  EXPECT_FALSE(IsNavigationBlocked(url));
+
+  // Add dynamic rule to block main-frame requests to "page.html".
+  TestRule rule = CreateGenericRule();
+  rule.condition->url_filter = std::string("page.html");
+  rule.condition->resource_types = std::vector<std::string>({"main_frame"});
+  ASSERT_NO_FATAL_FAILURE(AddDynamicRules(extension_id, {rule}));
+  ruleset_count_waiter.WaitForRulesetCount(1);
+
+  EXPECT_TRUE(IsNavigationBlocked(url));
+
+  DisableExtension(extension_id);
+  ruleset_count_waiter.WaitForRulesetCount(0);
+  EXPECT_FALSE(IsNavigationBlocked(url));
+
+  EnableExtension(extension_id);
+  ruleset_count_waiter.WaitForRulesetCount(1);
+  EXPECT_TRUE(IsNavigationBlocked(url));
+}
+
 // Ensure that Blink's in-memory cache is cleared on adding/removing rulesets.
 IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest, RendererCacheCleared) {
   // Set-up an observer for RulesetMatcher to monitor requests to
@@ -1978,9 +2029,7 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest,
   ASSERT_NO_FATAL_FAILURE(LoadExtensionWithRules({rule}));
 
   const ExtensionId extension_id = last_loaded_extension_id();
-  const auto* rules_monitor_service =
-      declarative_net_request::RulesMonitorService::Get(profile());
-  EXPECT_TRUE(rules_monitor_service->HasRegisteredRuleset(extension_id));
+  EXPECT_TRUE(ruleset_manager()->GetMatcherForExtension(extension_id));
 
   const Extension* extension = extension_registry()->GetExtensionById(
       last_loaded_extension_id(), ExtensionRegistry::ENABLED);
@@ -1999,7 +2048,7 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest,
       ExtensionRegistry::Get(profile()), extension_id);
   DisableExtension(extension_id);
   ASSERT_TRUE(registry_observer.WaitForExtensionUnloaded());
-  EXPECT_FALSE(rules_monitor_service->HasRegisteredRuleset(extension_id));
+  EXPECT_FALSE(ruleset_manager()->GetMatcherForExtension(extension_id));
 
   // Both loading the indexed ruleset and reindexing the ruleset should fail
   // now.
@@ -2013,7 +2062,7 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest,
   EXPECT_THAT(warning_service->GetWarningTypesAffectingExtension(extension_id),
               ::testing::ElementsAre(Warning::kRulesetFailedToLoad));
 
-  EXPECT_FALSE(rules_monitor_service->HasRegisteredRuleset(extension_id));
+  EXPECT_FALSE(ruleset_manager()->GetMatcherForExtension(extension_id));
 
   // Verify that loading the ruleset failed due to checksum mismatch.
   EXPECT_EQ(1, tester.GetBucketCount(
@@ -2043,16 +2092,14 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest_Packed,
   ruleset_count_waiter.WaitForRulesetCount(1);
 
   const ExtensionId extension_id = last_loaded_extension_id();
-  const auto* rules_monitor_service =
-      declarative_net_request::RulesMonitorService::Get(profile());
-  EXPECT_TRUE(rules_monitor_service->HasRegisteredRuleset(extension_id));
+  EXPECT_TRUE(ruleset_manager()->GetMatcherForExtension(extension_id));
 
   // Add a dynamic rule.
   AddDynamicRules(extension_id, {rule});
 
   DisableExtension(extension_id);
   ruleset_count_waiter.WaitForRulesetCount(0);
-  EXPECT_FALSE(rules_monitor_service->HasRegisteredRuleset(extension_id));
+  EXPECT_FALSE(ruleset_manager()->GetMatcherForExtension(extension_id));
 
   // Now change the current indexed ruleset format version. This should cause a
   // version mismatch when the extension is loaded again, but reindexing should
@@ -2065,7 +2112,7 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest_Packed,
   base::HistogramTester tester;
   EnableExtension(extension_id);
   ruleset_count_waiter.WaitForRulesetCount(1);
-  EXPECT_TRUE(rules_monitor_service->HasRegisteredRuleset(extension_id));
+  EXPECT_TRUE(ruleset_manager()->GetMatcherForExtension(extension_id));
 
   // Verify that loading the static and dynamic rulesets would have failed
   // initially due to version header mismatch and later succeeded.

chrome/browser/extensions/api/declarative_net_request/rule_indexing_unittest.cc

@@ -86,7 +86,8 @@ class RuleIndexingTestBase : public DNRTestBase {
  protected:
   // Loads the extension and verifies the indexed ruleset location and histogram
   // counts.
-  void LoadAndExpectSuccess(size_t expected_indexed_rules_count) {
+  void LoadAndExpectSuccess(size_t expected_indexed_rules_count,
+                            bool expect_rulesets_indexed = true) {
     base::HistogramTester tester;
     WriteExtensionData();
 
@@ -105,7 +106,7 @@ class RuleIndexingTestBase : public DNRTestBase {
     EXPECT_TRUE(error_reporter()->GetErrors()->empty());
 
     // The histograms below are not logged for unpacked extensions.
-    if (GetParam() == ExtensionLoadType::PACKED) {
+    if (GetParam() == ExtensionLoadType::PACKED && expect_rulesets_indexed) {
       tester.ExpectTotalCount(kIndexAndPersistRulesTimeHistogram,
                               1 /* count */);
       tester.ExpectBucketCount(kManifestRulesCountHistogram,
@@ -701,6 +702,12 @@ TEST_P(MultipleRulesetsIndexingTest, Success) {
                        kRulesPerRuleset /* expected_indexed_rules_count */);
 }
 
+// Tests an extension with no static rulesets.
+TEST_P(MultipleRulesetsIndexingTest, ZeroRulesets) {
+  LoadAndExpectSuccess(0 /* expected_indexed_rules_count */,
+                       false /* expect_rulesets_indexed */);
+}
+
 // Tests an extension with multiple empty rulesets.
 TEST_P(MultipleRulesetsIndexingTest, EmptyRulesets) {
   size_t kNumRulesets = 7;

chrome/browser/extensions/unpacked_installer.cc

@@ -268,11 +268,6 @@ bool UnpackedInstaller::LoadExtension(Manifest::Location location,
 bool UnpackedInstaller::IndexAndPersistRulesIfNeeded(std::string* error) {
   DCHECK(extension());
 
-  if (!declarative_net_request::DNRManifestData::HasRuleset(*extension())) {
-    // The extension did not provide a ruleset.
-    return true;
-  }
-
   using RulesetSource = declarative_net_request::RulesetSource;
 
   // TODO(crbug.com/761107): Change this so that we don't need to parse JSON
@@ -280,6 +275,7 @@ bool UnpackedInstaller::IndexAndPersistRulesIfNeeded(std::string* error) {
   // TODO(crbug.com/754526): Impose a limit on the total number of rules across
   // all the rulesets for an extension. Also, limit the number of install
   // warnings across all rulesets.
+  // Note |sources| may be empty.
   std::vector<RulesetSource> sources =
       RulesetSource::CreateStatic(*extension());
 

extensions/browser/api/declarative_net_request/action_tracker.cc

@@ -13,6 +13,7 @@
 #include "base/values.h"
 #include "extensions/browser/api/declarative_net_request/request_action.h"
 #include "extensions/browser/api/declarative_net_request/rules_monitor_service.h"
+#include "extensions/browser/api/declarative_net_request/ruleset_manager.h"
 #include "extensions/browser/api/declarative_net_request/utils.h"
 #include "extensions/browser/api/extensions_api_client.h"
 #include "extensions/browser/api/web_request/web_request_info.h"
@@ -221,11 +222,14 @@ void ActionTracker::ResetTrackedInfoForTab(int tab_id, int64_t navigation_id) {
 
   DCHECK(rules_monitor_service);
 
-  // Use |extensions_with_rulesets| because there may not be an entry for some
+  // Use GetExtensionsWithRulesets() because there may not be an entry for some
   // extensions in |rules_tracked_|. However, the action count should still be
   // surfaced for those extensions if the preference is enabled.
+  // TODO(kelvinjiang): Investigate if calling UpdateActionCount for all
+  // extensions with rulesets is necessary now that we don't show the action
+  // count if it is zero.
   for (const auto& extension_id :
-       rules_monitor_service->extensions_with_rulesets()) {
+       rules_monitor_service->ruleset_manager()->GetExtensionsWithRulesets()) {
     ExtensionNavigationIdKey navigation_key(extension_id, navigation_id);
 
     TrackedInfo& tab_info = rules_tracked_[{extension_id, tab_id}];

extensions/browser/api/declarative_net_request/declarative_net_request_api.cc

@@ -35,25 +35,6 @@ namespace {
 
 namespace dnr_api = api::declarative_net_request;
 
-// Returns true if the given |extension| has a registered ruleset. If it
-// doesn't, returns false and populates |error|.
-// TODO(crbug.com/931967): Using HasRegisteredRuleset for PreRunValidation means
-// that the extension function will fail if the ruleset for the extension is
-// currently being indexed. Fix this.
-bool HasRegisteredRuleset(content::BrowserContext* context,
-                          const ExtensionId& extension_id,
-                          std::string* error) {
-  const auto* rules_monitor_service =
-      declarative_net_request::RulesMonitorService::Get(context);
-  DCHECK(rules_monitor_service);
-
-  if (rules_monitor_service->HasRegisteredRuleset(extension_id))
-    return true;
-
-  *error = "The extension must have a ruleset in order to call this function.";
-  return false;
-}
-
 // Returns whether |extension| can call getMatchedRules for the specified
 // |tab_id| and populates |error| if it can't. If no tab ID is specified, then
 // the API call is for all tabs.
@@ -108,12 +89,6 @@ DeclarativeNetRequestUpdateDynamicRulesFunction::Run() {
   return RespondLater();
 }
 
-bool DeclarativeNetRequestUpdateDynamicRulesFunction::PreRunValidation(
-    std::string* error) {
-  return ExtensionFunction::PreRunValidation(error) &&
-         HasRegisteredRuleset(browser_context(), extension_id(), error);
-}
-
 void DeclarativeNetRequestUpdateDynamicRulesFunction::OnDynamicRulesUpdated(
     base::Optional<std::string> error) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -129,12 +104,6 @@ DeclarativeNetRequestGetDynamicRulesFunction::
 DeclarativeNetRequestGetDynamicRulesFunction::
     ~DeclarativeNetRequestGetDynamicRulesFunction() = default;
 
-bool DeclarativeNetRequestGetDynamicRulesFunction::PreRunValidation(
-    std::string* error) {
-  return ExtensionFunction::PreRunValidation(error) &&
-         HasRegisteredRuleset(browser_context(), extension_id(), error);
-}
-
 ExtensionFunction::ResponseAction
 DeclarativeNetRequestGetDynamicRulesFunction::Run() {
   auto source = declarative_net_request::RulesetSource::CreateDynamic(

extensions/browser/api/declarative_net_request/declarative_net_request_api.h

@@ -27,7 +27,6 @@ class DeclarativeNetRequestUpdateDynamicRulesFunction
   ~DeclarativeNetRequestUpdateDynamicRulesFunction() override;
 
   // ExtensionFunction override:
-  bool PreRunValidation(std::string* error) override;
   ExtensionFunction::ResponseAction Run() override;
 
  private:
@@ -46,7 +45,6 @@ class DeclarativeNetRequestGetDynamicRulesFunction : public ExtensionFunction {
   ~DeclarativeNetRequestGetDynamicRulesFunction() override;
 
   // ExtensionFunction override:
-  bool PreRunValidation(std::string* error) override;
   ExtensionFunction::ResponseAction Run() override;
 
  private:

extensions/browser/api/declarative_net_request/rules_monitor_service.cc

@@ -113,22 +113,16 @@ RulesMonitorService* RulesMonitorService::Get(
       browser_context);
 }
 
-bool RulesMonitorService::HasAnyRegisteredRulesets() const {
-  return !extensions_with_rulesets_.empty();
-}
-
-bool RulesMonitorService::HasRegisteredRuleset(
-    const ExtensionId& extension_id) const {
-  return extensions_with_rulesets_.find(extension_id) !=
-         extensions_with_rulesets_.end();
-}
-
 void RulesMonitorService::UpdateDynamicRules(
     const Extension& extension,
     std::vector<int> rule_ids_to_remove,
     std::vector<api::declarative_net_request::Rule> rules_to_add,
     DynamicRuleUpdateUICallback callback) {
-  DCHECK(HasRegisteredRuleset(extension.id()));
+  // TODO(crbug.com/953894): Subtle: The extension might just have been enabled
+  // and the ruleset for that extension might still be loading. In that case,
+  // the ruleset loading might race with updating the dynamic rules. If updating
+  // the dynamic rules wins the race, the subsequent completion of load will
+  // cause a DCHECK in RulesetManager::AddRuleset.
 
   LoadRequestData data(extension.id());
 
@@ -153,7 +147,12 @@ RulesMonitorService::RulesMonitorService(
       context_(browser_context),
       ruleset_manager_(browser_context),
       action_tracker_(browser_context) {
-  registry_observer_.Add(extension_registry_);
+  // Don't monitor extension lifecycle if the API is not available. This is
+  // useful since we base some of our actions (like loading dynamic ruleset on
+  // extension load) on the presence of certain extension prefs. These may still
+  // be remaining from an earlier install on which the feature was available.
+  if (IsAPIAvailable())
+    registry_observer_.Add(extension_registry_);
 }
 
 RulesMonitorService::~RulesMonitorService() = default;
@@ -180,11 +179,6 @@ void RulesMonitorService::OnExtensionLoaded(
     const Extension* extension) {
   DCHECK_EQ(context_, browser_context);
 
-  if (!declarative_net_request::DNRManifestData::HasRuleset(*extension))
-    return;
-
-  DCHECK(IsAPIAvailable());
-
   LoadRequestData load_data(extension->id());
   int expected_ruleset_checksum;
 
@@ -193,19 +187,22 @@ void RulesMonitorService::OnExtensionLoaded(
     std::vector<RulesetSource> static_rulesets =
         RulesetSource::CreateStatic(*extension);
 
-    // TODO(crbug.com/754526): Load all static rulesets for the extension.
-    RulesetInfo static_ruleset(std::move(static_rulesets[0]));
-    bool has_checksum = prefs_->GetDNRStaticRulesetChecksum(
-        extension->id(), static_ruleset.source().id(),
-        &expected_ruleset_checksum);
-
-    if (!has_checksum) {
-      // This might happen on prefs corruption.
-      warning_service_->AddWarnings(
-          {Warning::CreateRulesetFailedToLoadWarning(load_data.extension_id)});
-    } else {
-      static_ruleset.set_expected_checksum(expected_ruleset_checksum);
-      load_data.rulesets.push_back(std::move(static_ruleset));
+    if (!static_rulesets.empty()) {
+      // TODO(crbug.com/754526): Load all static rulesets for the extension.
+      RulesetInfo static_ruleset(std::move(static_rulesets[0]));
+      bool has_checksum = prefs_->GetDNRStaticRulesetChecksum(
+          extension->id(), static_ruleset.source().id(),
+          &expected_ruleset_checksum);
+
+      if (!has_checksum) {
+        // This might happen on prefs corruption.
+        warning_service_->AddWarnings(
+            {Warning::CreateRulesetFailedToLoadWarning(
+                load_data.extension_id)});
+      } else {
+        static_ruleset.set_expected_checksum(expected_ruleset_checksum);
+        load_data.rulesets.push_back(std::move(static_ruleset));
+      }
     }
   }
 
@@ -234,12 +231,10 @@ void RulesMonitorService::OnExtensionUnloaded(
   DCHECK_EQ(context_, browser_context);
 
   // Return early if the extension does not have an active indexed ruleset.
-  if (!extensions_with_rulesets_.erase(extension->id()))
+  if (!ruleset_manager_.GetMatcherForExtension(extension->id()))
     return;
 
-  DCHECK(IsAPIAvailable());
-
-  UnloadRuleset(extension->id());
+  UnloadRulesets(extension->id());
 }
 
 void RulesMonitorService::OnExtensionUninstalled(
@@ -272,29 +267,25 @@ void RulesMonitorService::OnExtensionUninstalled(
 }
 
 void RulesMonitorService::OnRulesetLoaded(LoadRequestData load_data) {
-  // Currently we only support a single static and an optional dynamic ruleset
-  // per extension.
-  DCHECK(load_data.rulesets.size() == 1u || load_data.rulesets.size() == 2u);
-  RulesetInfo& static_ruleset = load_data.rulesets[0];
-  DCHECK_GE(static_ruleset.source().id(), kMinValidStaticRulesetID)
-      << static_ruleset.source().id();
-
-  RulesetInfo* dynamic_ruleset =
-      load_data.rulesets.size() == 2 ? &load_data.rulesets[1] : nullptr;
-  DCHECK(!dynamic_ruleset ||
-         dynamic_ruleset->source().id() == kDynamicRulesetID)
-      << dynamic_ruleset->source().id();
-
-  // Update the ruleset checksums if needed.
-  if (static_ruleset.new_checksum()) {
-    prefs_->SetDNRStaticRulesetChecksum(load_data.extension_id,
-                                        static_ruleset.source().id(),
-                                        *(static_ruleset.new_checksum()));
-  }
+  DCHECK(!load_data.rulesets.empty());
 
-  if (dynamic_ruleset && dynamic_ruleset->new_checksum()) {
-    prefs_->SetDNRDynamicRulesetChecksum(load_data.extension_id,
-                                         *(dynamic_ruleset->new_checksum()));
+  // Update checksums for all rulesets.
+  // TODO(crbug.com/754526): The extension may have been uninstalled by this
+  // point, in which case setting the prefs is incorrect.
+  for (const RulesetInfo& ruleset : load_data.rulesets) {
+    if (!ruleset.new_checksum())
+      continue;
+
+    const bool is_dynamic_ruleset = ruleset.source().id() == kDynamicRulesetID;
+
+    if (is_dynamic_ruleset) {
+      prefs_->SetDNRDynamicRulesetChecksum(load_data.extension_id,
+                                           *(ruleset.new_checksum()));
+    } else {
+      prefs_->SetDNRStaticRulesetChecksum(load_data.extension_id,
+                                          ruleset.source().id(),
+                                          *(ruleset.new_checksum()));
+    }
   }
 
   // It's possible that the extension has been disabled since the initial load
@@ -304,11 +295,11 @@ void RulesMonitorService::OnRulesetLoaded(LoadRequestData load_data) {
     return;
 
   CompositeMatcher::MatcherList matchers;
-  if (static_ruleset.did_load_successfully()) {
-    matchers.push_back(static_ruleset.TakeMatcher());
-  }
-  if (dynamic_ruleset && dynamic_ruleset->did_load_successfully()) {
-    matchers.push_back(dynamic_ruleset->TakeMatcher());
+  for (RulesetInfo& ruleset : load_data.rulesets) {
+    if (!ruleset.did_load_successfully())
+      continue;
+
+    matchers.push_back(ruleset.TakeMatcher());
   }
 
   // A ruleset failed to load. Notify the user.
@@ -320,9 +311,8 @@ void RulesMonitorService::OnRulesetLoaded(LoadRequestData load_data) {
   if (matchers.empty())
     return;
 
-  extensions_with_rulesets_.insert(load_data.extension_id);
-  LoadRuleset(load_data.extension_id,
-              std::make_unique<CompositeMatcher>(std::move(matchers)));
+  LoadRulesets(load_data.extension_id,
+               std::make_unique<CompositeMatcher>(std::move(matchers)));
 }
 
 void RulesMonitorService::OnDynamicRulesUpdated(
@@ -364,7 +354,7 @@ void RulesMonitorService::OnDynamicRulesUpdated(
   UpdateRuleset(load_data.extension_id, dynamic_ruleset.TakeMatcher());
 }
 
-void RulesMonitorService::UnloadRuleset(const ExtensionId& extension_id) {
+void RulesMonitorService::UnloadRulesets(const ExtensionId& extension_id) {
   bool had_extra_headers_matcher = ruleset_manager_.HasAnyExtraHeadersMatcher();
   ruleset_manager_.RemoveRuleset(extension_id);
   action_tracker_.ClearExtensionData(extension_id);
@@ -376,7 +366,7 @@ void RulesMonitorService::UnloadRuleset(const ExtensionId& extension_id) {
   }
 }
 
-void RulesMonitorService::LoadRuleset(
+void RulesMonitorService::LoadRulesets(
     const ExtensionId& extension_id,
     std::unique_ptr<CompositeMatcher> matcher) {
   bool increment_extra_headers =
@@ -397,7 +387,16 @@ void RulesMonitorService::UpdateRuleset(
 
   CompositeMatcher* matcher =
       ruleset_manager_.GetMatcherForExtension(extension_id);
-  DCHECK(matcher);
+
+  // The extension didn't have any active rulesets.
+  if (!matcher) {
+    CompositeMatcher::MatcherList matchers;
+    matchers.push_back(std::move(ruleset_matcher));
+    LoadRulesets(extension_id,
+                 std::make_unique<CompositeMatcher>(std::move(matchers)));
+    return;
+  }
+
   matcher->AddOrUpdateRuleset(std::move(ruleset_matcher));
 
   bool has_extra_headers_matcher = ruleset_manager_.HasAnyExtraHeadersMatcher();

extensions/browser/api/declarative_net_request/rules_monitor_service.h

@@ -56,16 +56,6 @@ class RulesMonitorService : public BrowserContextKeyedAPI,
   static BrowserContextKeyedAPIFactory<RulesMonitorService>*
   GetFactoryInstance();
 
-  bool HasAnyRegisteredRulesets() const;
-
-  // Returns true if there is registered declarative ruleset corresponding to
-  // the given |extension_id|.
-  bool HasRegisteredRuleset(const ExtensionId& extension_id) const;
-
-  const std::set<ExtensionId>& extensions_with_rulesets() const {
-    return extensions_with_rulesets_;
-  }
-
   // Updates the dynamic rules for the |extension| and then invokes
   // |callback| with an optional error.
   using DynamicRuleUpdateUICallback =
@@ -115,17 +105,20 @@ class RulesMonitorService : public BrowserContextKeyedAPI,
                              LoadRequestData load_data,
                              base::Optional<std::string> error);
 
-  void UnloadRuleset(const ExtensionId& extension_id);
-  void LoadRuleset(const ExtensionId& extension_id,
-                   std::unique_ptr<CompositeMatcher> matcher);
+  // Unloads all rulesets for the given |extension_id|.
+  void UnloadRulesets(const ExtensionId& extension_id);
+
+  // Loads the given |matcher| for the given |extension_id|.
+  void LoadRulesets(const ExtensionId& extension_id,
+                    std::unique_ptr<CompositeMatcher> matcher);
+
+  // Adds the given ruleset for the given |extension_id|.
   void UpdateRuleset(const ExtensionId& extension_id,
                      std::unique_ptr<RulesetMatcher> ruleset_matcher);
 
   ScopedObserver<ExtensionRegistry, ExtensionRegistryObserver>
       registry_observer_{this};
 
-  std::set<ExtensionId> extensions_with_rulesets_;
-
   // Helper to bridge tasks to a sequence which allows file IO.
   std::unique_ptr<const FileSequenceBridge> file_sequence_bridge_;
 

extensions/browser/api/declarative_net_request/ruleset_manager.cc

@@ -83,9 +83,11 @@ void RulesetManager::AddRuleset(const ExtensionId& extension_id,
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(IsAPIAvailable());
 
-  bool inserted;
-  std::tie(std::ignore, inserted) = rulesets_.emplace(
-      extension_id, prefs_->GetInstallTime(extension_id), std::move(matcher));
+  bool inserted =
+      rulesets_
+          .emplace(extension_id, prefs_->GetInstallTime(extension_id),
+                   std::move(matcher))
+          .second;
   DCHECK(inserted) << "AddRuleset called twice in succession for "
                    << extension_id;
 
@@ -105,13 +107,11 @@ void RulesetManager::RemoveRuleset(const ExtensionId& extension_id) {
         return ruleset_data.extension_id == extension_id;
       };
 
-  DCHECK(std::find_if(rulesets_.begin(), rulesets_.end(), compare_by_id) !=
-         rulesets_.end())
+  size_t erased_count = base::EraseIf(rulesets_, compare_by_id);
+  DCHECK_EQ(1u, erased_count)
       << "RemoveRuleset called without a corresponding AddRuleset for "
       << extension_id;
 
-  base::EraseIf(rulesets_, compare_by_id);
-
   if (test_observer_)
     test_observer_->OnRulesetCountChanged(rulesets_.size());
 
@@ -120,6 +120,13 @@ void RulesetManager::RemoveRuleset(const ExtensionId& extension_id) {
   ClearRendererCacheOnNavigation();
 }
 
+std::set<ExtensionId> RulesetManager::GetExtensionsWithRulesets() const {
+  std::set<ExtensionId> extension_ids;
+  for (const ExtensionRulesetData& data : rulesets_)
+    extension_ids.insert(data.extension_id);
+  return extension_ids;
+}
+
 CompositeMatcher* RulesetManager::GetMatcherForExtension(
     const ExtensionId& extension_id) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

extensions/browser/api/declarative_net_request/ruleset_manager.h

@@ -7,6 +7,7 @@
 
 #include <stddef.h>
 #include <memory>
+#include <set>
 #include <vector>
 
 #include "base/containers/flat_set.h"
@@ -63,6 +64,9 @@ class RulesetManager {
   // corresponding AddRuleset.
   void RemoveRuleset(const ExtensionId& extension_id);
 
+  // Returns the set of extensions which have active rulesets.
+  std::set<ExtensionId> GetExtensionsWithRulesets() const;
+
   // Returns the CompositeMatcher corresponding to the |extension_id| or null
   // if no matcher is present for the extension.
   CompositeMatcher* GetMatcherForExtension(const ExtensionId& extension_id);

extensions/browser/content_verifier.cc

@@ -129,12 +129,10 @@ std::unique_ptr<ContentVerifierIOData::ExtensionData> CreateIOData(
   auto indexed_ruleset_paths =
       std::make_unique<std::set<CanonicalRelativePath>>();
   using DNRManifestData = declarative_net_request::DNRManifestData;
-  if (DNRManifestData::HasRuleset(*extension)) {
-    for (const DNRManifestData::RulesetInfo& info :
-         DNRManifestData::GetRulesets(*extension)) {
-      indexed_ruleset_paths->insert(
-          canonicalize_path(file_util::GetIndexedRulesetRelativePath(info.id)));
-    }
+  for (const DNRManifestData::RulesetInfo& info :
+       DNRManifestData::GetRulesets(*extension)) {
+    indexed_ruleset_paths->insert(
+        canonicalize_path(file_util::GetIndexedRulesetRelativePath(info.id)));
   }
 
   return std::make_unique<ContentVerifierIOData::ExtensionData>(

extensions/browser/sandboxed_unpacker.cc

@@ -692,12 +692,6 @@ void SandboxedUnpacker::IndexAndPersistJSONRulesetsIfNeeded() {
   DCHECK(unpacker_io_task_runner_->RunsTasksInCurrentSequence());
   DCHECK(extension_);
 
-  if (!declarative_net_request::DNRManifestData::HasRuleset(*extension_)) {
-    // The extension did not provide a ruleset.
-    CheckComputeHashes();
-    return;
-  }
-
   declarative_net_request::IndexHelper::Start(
       declarative_net_request::RulesetSource::CreateStatic(*extension_),
       base::BindOnce(&SandboxedUnpacker::OnJSONRulesetsIndexed, this));
@@ -706,6 +700,12 @@ void SandboxedUnpacker::IndexAndPersistJSONRulesetsIfNeeded() {
 void SandboxedUnpacker::OnJSONRulesetsIndexed(
     std::vector<declarative_net_request::IndexAndPersistJSONRulesetResult>
         results) {
+  // Early return if there were no rulesets to index originally.
+  if (results.empty()) {
+    CheckComputeHashes();
+    return;
+  }
+
   base::TimeDelta total_index_and_persist_time;
   size_t total_rules_count = 0;
 
@@ -734,6 +734,7 @@ void SandboxedUnpacker::OnJSONRulesetsIndexed(
       total_index_and_persist_time);
   UMA_HISTOGRAM_COUNTS_100000(
       declarative_net_request::kManifestRulesCountHistogram, total_rules_count);
+
   CheckComputeHashes();
 }
 

extensions/common/api/declarative_net_request/dnr_manifest_data.cc

@@ -7,30 +7,28 @@
 #include <utility>
 
 #include "base/logging.h"
+#include "base/no_destructor.h"
 #include "extensions/common/manifest_constants.h"
 
 namespace extensions {
 namespace declarative_net_request {
 
 DNRManifestData::DNRManifestData(std::vector<RulesetInfo> rulesets)
-    : rulesets(std::move(rulesets)) {
-  // TODO(crbug.com/953894): Remove this DCHECK when we support specifying 0
-  // rulesets.
-  DCHECK(!(this->rulesets.empty()));
-}
+    : rulesets(std::move(rulesets)) {}
 DNRManifestData::~DNRManifestData() = default;
 
-// static
-bool DNRManifestData::HasRuleset(const Extension& extension) {
-  return extension.GetManifestData(manifest_keys::kDeclarativeNetRequestKey);
-}
-
 // static
 const std::vector<DNRManifestData::RulesetInfo>& DNRManifestData::GetRulesets(
     const Extension& extension) {
+  // Since we return a reference, use a function local static for the case where
+  // the extension didn't specify any rulesets.
+  static const base::NoDestructor<std::vector<DNRManifestData::RulesetInfo>>
+      empty_vector;
+
   Extension::ManifestData* data =
       extension.GetManifestData(manifest_keys::kDeclarativeNetRequestKey);
-  DCHECK(data);
+  if (!data)
+    return *empty_vector;
 
   return static_cast<DNRManifestData*>(data)->rulesets;
 }

extensions/common/api/declarative_net_request/dnr_manifest_data.h

@@ -31,12 +31,8 @@ struct DNRManifestData : Extension::ManifestData {
   explicit DNRManifestData(std::vector<RulesetInfo> ruleset);
   ~DNRManifestData() override;
 
-  // Returns true if the extension specified the kDeclarativeNetRequestKey
-  // manifest key.
-  static bool HasRuleset(const Extension& extension);
-
-  // Returns the RulesetInfo for the |extension|. This must be called only if
-  // HasRuleset returns true for the |extension|.
+  // Returns the RulesetInfo for the |extension|. For an extension, which didn't
+  // specify a static ruleset, an empty vector is returned.
   static const std::vector<RulesetInfo>& GetRulesets(
       const Extension& extension);
 

extensions/common/api/declarative_net_request/dnr_manifest_handler.cc

@@ -63,14 +63,6 @@ bool DNRManifestHandler::Parse(Extension* extension, base::string16* error) {
     return false;
   }
 
-  // TODO(crbug.com/953894): Extension should be able to specify zero rulesets.
-  if (rulesets.empty()) {
-    *error = ErrorUtils::FormatErrorMessageUTF16(
-        errors::kInvalidDeclarativeRulesFileKey,
-        keys::kDeclarativeNetRequestKey, keys::kDeclarativeRuleResourcesKey);
-    return false;
-  }
-
   if (rulesets.size() >
       static_cast<size_t>(dnr_api::MAX_NUMBER_OF_STATIC_RULESETS)) {
     *error = ErrorUtils::FormatErrorMessageUTF16(

extensions/common/api/declarative_net_request/dnr_manifest_unittest.cc

@@ -58,7 +58,7 @@ class DNRManifestTest : public testing::Test {
   }
 
   // Loads the extension and verifies that the manifest info is correctly set
-  // up.. Returns the loaded extension.
+  // up.
   void LoadAndExpectSuccess(const std::vector<base::FilePath>& expected_paths) {
     std::string error;
     scoped_refptr<Extension> extension = file_util::LoadExtension(
@@ -66,8 +66,6 @@ class DNRManifestTest : public testing::Test {
     ASSERT_TRUE(extension) << error;
     EXPECT_TRUE(error.empty());
 
-    ASSERT_TRUE(DNRManifestData::HasRuleset(*extension));
-
     const std::vector<DNRManifestData::RulesetInfo>& rulesets =
         DNRManifestData::GetRulesets(*extension);
     ASSERT_EQ(expected_paths.size(), rulesets.size());
@@ -131,6 +129,16 @@ TEST_F(DNRManifestTest, InvalidRulesFileKey) {
       keys::kDeclarativeRuleResourcesKey));
 }
 
+TEST_F(DNRManifestTest, ZeroRulesets) {
+  std::unique_ptr<base::DictionaryValue> manifest =
+      CreateManifest(kJSONRulesFilename);
+  manifest->SetList(GetRuleResourcesKey(), std::make_unique<base::ListValue>());
+
+  std::vector<base::FilePath> no_paths;
+  WriteManifestAndRuleset(*manifest, no_paths);
+  LoadAndExpectSuccess(no_paths);
+}
+
 TEST_F(DNRManifestTest, MultipleRulesFileSuccess) {
   std::unique_ptr<base::DictionaryValue> manifest =
       CreateManifest(kJSONRulesFilename);

extensions/common/api/declarative_net_request/test_utils.cc

@@ -90,13 +90,19 @@ std::unique_ptr<base::DictionaryValue> CreateManifest(
     rule_resources_builder.Append(ruleset.ToValue());
   }
 
-  return DictionaryBuilder()
-      .Set(keys::kName, "Test extension")
-      .Set(keys::kDeclarativeNetRequestKey,
-           DictionaryBuilder()
-               .Set(keys::kDeclarativeRuleResourcesKey,
-                    rule_resources_builder.Build())
-               .Build())
+  DictionaryBuilder manifest_builder;
+
+  if (flags & kConfig_OmitDeclarativeNetRequestKey) {
+    DCHECK(ruleset_info.empty());
+  } else {
+    manifest_builder.Set(keys::kDeclarativeNetRequestKey,
+                         DictionaryBuilder()
+                             .Set(keys::kDeclarativeRuleResourcesKey,
+                                  rule_resources_builder.Build())
+                             .Build());
+  }
+
+  return manifest_builder.Set(keys::kName, "Test extension")
       .Set(keys::kPermissions, ToListValue(permissions))
       .Set(keys::kVersion, "1.0")
       .Set(keys::kManifestVersion, 2)

extensions/common/api/declarative_net_request/test_utils.h

@@ -150,6 +150,9 @@ enum ConfigFlag {
 
   // Whether the extension has the activeTab permission.
   kConfig_HasActiveTab = 1 << 2,
+
+  // Whether the "declarative_net_request" manifest key should be omitted.
+  kConfig_OmitDeclarativeNetRequestKey = 1 << 3,
 };
 
 // Describes a single extension ruleset.

extensions/common/manifest_constants.cc

@@ -384,8 +384,8 @@ const char kInvalidCssList[] =
     "Required value 'content_scripts[*].css' is invalid.";
 const char kInvalidDeclarativeNetRequestKey[] = "Invalid value for '*' key";
 const char kInvalidDeclarativeRulesFileKey[] =
-    "Invalid value for '*.*' key. It must be a non-empty list containing "
-    "Ruleset dictionaries.";
+    "Invalid value for '*.*' key. It must be a list containing Ruleset "
+    "dictionaries.";
 const char kInvalidDefaultLocale[] =
     "Invalid value for default locale - locale name must be a string.";
 const char kInvalidDescription[] =