DNR: Update updateEnabledRulesets schema.

Change it to accept a dictionary instead of individual arguments for
better future compatibility. Note that this is a breaking change but
we'll reach out to the current users to make sure that they update their
extensions.

Skipping presubmit since this seems to be hitting crbug.com/956368.

BUG=1131746

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

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

@@ -422,7 +422,11 @@ class DeclarativeNetRequestBrowserTest
       const std::vector<std::string>& ruleset_ids_to_remove,
       const std::vector<std::string>& ruleset_ids_to_add) {
     static constexpr char kScript[] = R"(
-      chrome.declarativeNetRequest.updateEnabledRulesets($1, $2, () => {
+      let params = {
+        disableRulesetIds: $1,
+        enableRulesetIds: $2
+      };
+      chrome.declarativeNetRequest.updateEnabledRulesets(params, () => {
         window.domAutomationController.send(chrome.runtime.lastError ?
             chrome.runtime.lastError.message : 'success');
       });

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

@@ -12,7 +12,6 @@
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/json/json_reader.h"
-#include "base/json/json_writer.h"
 #include "base/location.h"
 #include "base/memory/ref_counted.h"
 #include "base/optional.h"
@@ -225,14 +224,19 @@ class DeclarativeNetRequestUnittest : public DNRTestBase {
       const std::vector<std::string>& ruleset_ids_to_remove,
       const std::vector<std::string>& ruleset_ids_to_add,
       base::Optional<std::string> expected_error) {
-    std::unique_ptr<base::Value> args =
-        ListBuilder()
-            .Append(ToListValue(ruleset_ids_to_remove))
-            .Append(ToListValue(ruleset_ids_to_add))
-            .Build();
-    std::string json_args;
-    base::JSONWriter::WriteWithOptions(
-        *args, base::JSONWriter::OPTIONS_PRETTY_PRINT, &json_args);
+    std::unique_ptr<base::Value> ids_to_remove_value =
+        ToListValue(ruleset_ids_to_remove);
+    std::unique_ptr<base::Value> ids_to_add_value =
+        ToListValue(ruleset_ids_to_add);
+
+    constexpr const char kParams[] = R"(
+      [{
+        "disableRulesetIds": $1,
+        "enableRulesetIds": $2
+      }]
+    )";
+    const std::string json_args = content::JsReplace(
+        kParams, std::move(*ids_to_remove_value), std::move(*ids_to_add_value));
 
     auto function = base::MakeRefCounted<
         DeclarativeNetRequestUpdateEnabledRulesetsFunction>();

extensions/browser/api/declarative_net_request/declarative_net_request_api.cc

@@ -174,43 +174,51 @@ DeclarativeNetRequestUpdateEnabledRulesetsFunction::Run() {
   EXTENSION_FUNCTION_VALIDATE(params);
   EXTENSION_FUNCTION_VALIDATE(error.empty());
 
-  auto* rules_monitor_service =
-      declarative_net_request::RulesMonitorService::Get(browser_context());
-  DCHECK(rules_monitor_service);
-  DCHECK(extension());
-
   std::set<RulesetID> ids_to_disable;
   std::set<RulesetID> ids_to_enable;
   const DNRManifestData::ManifestIDToRulesetMap& public_id_map =
       DNRManifestData::GetManifestIDToRulesetMap(*extension());
 
-  for (const std::string& public_id_to_enable : params->ruleset_ids_to_enable) {
-    auto it = public_id_map.find(public_id_to_enable);
-    if (it == public_id_map.end()) {
-      return RespondNow(Error(ErrorUtils::FormatErrorMessage(
-          declarative_net_request::kInvalidRulesetIDError,
-          public_id_to_enable)));
+  if (params->options.enable_ruleset_ids) {
+    for (const std::string& public_id_to_enable :
+         *params->options.enable_ruleset_ids) {
+      auto it = public_id_map.find(public_id_to_enable);
+      if (it == public_id_map.end()) {
+        return RespondNow(Error(ErrorUtils::FormatErrorMessage(
+            declarative_net_request::kInvalidRulesetIDError,
+            public_id_to_enable)));
+      }
+
+      ids_to_enable.insert(it->second->id);
     }
-
-    ids_to_enable.insert(it->second->id);
   }
 
-  for (const std::string& public_id_to_disable :
-       params->ruleset_ids_to_disable) {
-    auto it = public_id_map.find(public_id_to_disable);
-    if (it == public_id_map.end()) {
-      return RespondNow(Error(ErrorUtils::FormatErrorMessage(
-          declarative_net_request::kInvalidRulesetIDError,
-          public_id_to_disable)));
+  if (params->options.disable_ruleset_ids) {
+    for (const std::string& public_id_to_disable :
+         *params->options.disable_ruleset_ids) {
+      auto it = public_id_map.find(public_id_to_disable);
+      if (it == public_id_map.end()) {
+        return RespondNow(Error(ErrorUtils::FormatErrorMessage(
+            declarative_net_request::kInvalidRulesetIDError,
+            public_id_to_disable)));
+      }
+
+      // |ruleset_ids_to_enable| takes priority over |ruleset_ids_to_disable|.
+      RulesetID id = it->second->id;
+      if (base::Contains(ids_to_enable, id))
+        continue;
+
+      ids_to_disable.insert(id);
     }
+  }
 
-    // |ruleset_ids_to_enable| takes priority over |ruleset_ids_to_disable|.
-    RulesetID id = it->second->id;
-    if (base::Contains(ids_to_enable, id))
-      continue;
+  if (ids_to_enable.empty() && ids_to_disable.empty())
+    return RespondNow(NoArguments());
 
-    ids_to_disable.insert(id);
-  }
+  auto* rules_monitor_service =
+      declarative_net_request::RulesMonitorService::Get(browser_context());
+  DCHECK(rules_monitor_service);
+  DCHECK(extension());
 
   rules_monitor_service->UpdateEnabledStaticRulesets(
       *extension(), std::move(ids_to_disable), std::move(ids_to_enable),

extensions/common/api/declarative_net_request.idl

@@ -419,6 +419,15 @@ namespace declarativeNetRequest {
     Rule[]? addRules;
   };
 
+  dictionary UpdateRulesetOptions {
+    // The set of ids corresponding to a static $(ref:Ruleset) that should be
+    // disabled.
+    DOMString[]? disableRulesetIds;
+    // The set of ids corresponding to a static $(ref:Ruleset) that should be
+    // enabled.
+    DOMString[]? enableRulesetIds;
+  };
+
   callback EmptyCallback = void();
   callback GetAllowedPagesCallback = void(DOMString[] result);
   callback GetRulesCallback = void(Rule[] rules);
@@ -456,24 +465,19 @@ namespace declarativeNetRequest {
     static void getDynamicRules(GetRulesCallback callback);
 
     // Updates the set of enabled static rulesets for the extension. The
-    // rulesets with IDs listed in <code>rulesetIDsToDisable</code> are first
-    // removed, and then the rulesets listed in <code>rulesetIDsToEnable</code>
-    // are added.
+    // rulesets with IDs listed in <code>options.disableRulesetIds</code> are
+    // first removed, and then the rulesets listed in
+    // <code>options.enableRulesetIds</code> are added.<br/>
     // Note that the set of enabled static rulesets is persisted across sessions
     // but not across extension updates, i.e. the <code>rule_resources</code>
     // manifest key will determine the set of enabled static rulesets on each
     // extension update.
-    // |rulesetIdsToDisable|: The set of ids corresponding to a static
-    // $(ref:Ruleset) that should be disabled.
-    // |rulesetIdsToEnable|: The set of ids corresponding to a static
-    // $(ref:Ruleset) that should be enabled.
     // |callback|: Called once the update is complete. In case of an error,
     // $(ref:runtime.lastError) will be set and no change will be made to set of
     // enabled rulesets. This can happen for multiple reasons, such as invalid
     // ruleset IDs, rule count limit exceeded, or internal errors.
-    static void updateEnabledRulesets(
-        DOMString[] rulesetIdsToDisable, DOMString[] rulesetIdsToEnable,
-        EmptyCallback callback);
+    static void updateEnabledRulesets(UpdateRulesetOptions options,
+                                      optional EmptyCallback callback);
 
     // Returns the ids for the current set of enabled static rulesets.
     // |callback|: Called with a list of ids, where each id corresponds to an