DNR: Simplify WriteManifestAndRuleset.

The |json_rules_filepath| argument is redundant, remove it. Also,
introduce a new struct TestRulesetInfo, since we anticipate extending
WriteManifestAndRuleset to support multiple rulesets.

BUG=754526

Change-Id: I85204f8c0dfabdfde6318e7513d6d590964987f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2120097
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Kelvin Jiang <kelvinjiang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753949}

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

@@ -30,8 +30,6 @@ namespace dnr_api = api::declarative_net_request;
 namespace {
 
 constexpr char kJSONRulesFilename[] = "rules_file.json";
-const base::FilePath::CharType kJSONRulesetFilepath[] =
-    FILE_PATH_LITERAL("rules_file.json");
 
 constexpr int64_t kNavigationId = 1;
 
@@ -57,9 +55,9 @@ class ActionTrackerTest : public DNRTestBase {
 
     // Create extension directory.
     ASSERT_TRUE(base::CreateDirectory(extension_dir));
+    TestRulesetInfo info = {kJSONRulesFilename, base::ListValue()};
     WriteManifestAndRuleset(
-        extension_dir, kJSONRulesetFilepath, kJSONRulesFilename,
-        std::vector<TestRule>(),
+        extension_dir, info,
         std::vector<std::string>({URLPattern::kAllUrlsPattern}), flags);
 
     last_loaded_extension_ =

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

@@ -117,8 +117,6 @@ namespace {
 namespace dnr_api = api::declarative_net_request;
 
 constexpr char kJSONRulesFilename[] = "rules_file.json";
-const base::FilePath::CharType kJSONRulesetFilepath[] =
-    FILE_PATH_LITERAL("rules_file.json");
 
 // Returns true if |window.scriptExecuted| is true for the given frame.
 bool WasFrameWithScriptLoaded(content::RenderFrameHost* rfh) {
@@ -359,8 +357,8 @@ class DeclarativeNetRequestBrowserTest
     base::FilePath extension_dir = temp_dir_.GetPath().AppendASCII(directory);
     EXPECT_TRUE(base::CreateDirectory(extension_dir));
 
-    WriteManifestAndRuleset(extension_dir, kJSONRulesetFilepath,
-                            kJSONRulesFilename, rules, hosts, flags_);
+    TestRulesetInfo info = {kJSONRulesFilename, std::move(*ToListValue(rules))};
+    WriteManifestAndRuleset(extension_dir, info, hosts, flags_);
 
     background_page_ready_listener_->Reset();
     const Extension* extension = nullptr;

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

@@ -38,8 +38,6 @@ namespace declarative_net_request {
 namespace {
 
 constexpr char kJSONRulesFilename[] = "rules_file.json";
-const base::FilePath::CharType kJSONRulesetFilepath[] =
-    FILE_PATH_LITERAL("rules_file.json");
 
 std::string GetParseError(ParseResult result, int rule_id) {
   ParseInfo info;
@@ -156,20 +154,17 @@ class RuleIndexingTest : public DNRTestBase {
     // Create extension directory.
     EXPECT_TRUE(base::CreateDirectory(extension_dir_));
 
-    if (rules_value_) {
-      WriteManifestAndRuleset(extension_dir_, kJSONRulesetFilepath,
-                              kJSONRulesFilename, *rules_value_,
-                              {} /* hosts */);
-    } else {
-      WriteManifestAndRuleset(extension_dir_, kJSONRulesetFilepath,
-                              kJSONRulesFilename, rules_list_, {} /* hosts */);
-    }
+    if (!rules_value_)
+      rules_value_ = ToListValue(rules_list_);
+
+    TestRulesetInfo info = {kJSONRulesFilename, std::move(*rules_value_)};
+    WriteManifestAndRuleset(extension_dir_, info, {} /* hosts */);
 
     // Overwrite the JSON rules file with some invalid json.
     if (persist_invalid_json_file_) {
       std::string data = "invalid json";
       ASSERT_EQ(static_cast<int>(data.size()),
-                base::WriteFile(extension_dir_.Append(kJSONRulesetFilepath),
+                base::WriteFile(extension_dir_.AppendASCII(kJSONRulesFilename),
                                 data.c_str(), data.size()));
     }
 

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

@@ -43,8 +43,6 @@ namespace dnr_api = api::declarative_net_request;
 namespace {
 
 constexpr char kJSONRulesFilename[] = "rules_file.json";
-const base::FilePath::CharType kJSONRulesetFilepath[] =
-    FILE_PATH_LITERAL("rules_file.json");
 
 class RulesetManagerTest : public DNRTestBase {
  public:
@@ -73,8 +71,9 @@ class RulesetManagerTest : public DNRTestBase {
     ConfigFlag flags = has_background_script
                            ? ConfigFlag::kConfig_HasBackgroundScript
                            : ConfigFlag::kConfig_None;
-    WriteManifestAndRuleset(extension_dir, kJSONRulesetFilepath,
-                            kJSONRulesFilename, rules, host_permissions, flags);
+
+    TestRulesetInfo info = {kJSONRulesFilename, std::move(*ToListValue(rules))};
+    WriteManifestAndRuleset(extension_dir, info, host_permissions, flags);
 
     last_loaded_extension_ =
         CreateExtensionLoader()->LoadExtension(extension_dir);

extensions/browser/api/declarative_net_request/filter_list_converter/converter.cc

@@ -34,8 +34,6 @@ using ElementTypeMap =
     base::flat_map<proto::ElementType, dnr_api::ResourceType>;
 
 constexpr char kJSONRulesFilename[] = "rules.json";
-const base::FilePath::CharType kJSONRulesetFilepath[] =
-    FILE_PATH_LITERAL("rules.json");
 
 // Utility class to convert the proto::UrlRule format to the JSON format
 // supported by Declarative Net Request.
@@ -493,11 +491,12 @@ class DNRJsonRuleOutputStream : public subresource_filter::RuleOutputStream {
 
   bool Finish() override {
     switch (write_type_) {
-      case filter_list_converter::kExtension:
-        WriteManifestAndRuleset(output_path_, kJSONRulesetFilepath,
-                                kJSONRulesFilename, output_rules_list_,
-                                {} /* hosts */);
+      case filter_list_converter::kExtension: {
+        TestRulesetInfo info = {kJSONRulesFilename,
+                                std::move(output_rules_list_)};
+        WriteManifestAndRuleset(output_path_, info, {} /* hosts */);
         break;
+      }
       case filter_list_converter::kJSONRuleset:
         JSONFileValueSerializer(output_path_).Serialize(output_rules_list_);
         break;

extensions/common/api/declarative_net_request/test_utils.cc

@@ -6,7 +6,6 @@
 
 #include "base/files/file_util.h"
 #include "base/json/json_file_value_serializer.h"
-#include "base/values.h"
 #include "extensions/common/api/declarative_net_request.h"
 #include "extensions/common/api/declarative_net_request/constants.h"
 #include "extensions/common/constants.h"
@@ -234,30 +233,20 @@ std::unique_ptr<base::ListValue> ToListValue(
   return builder.Build();
 }
 
-void WriteManifestAndRuleset(
-    const base::FilePath& extension_dir,
-    const base::FilePath::CharType* json_rules_filepath,
-    const std::string& json_rules_filename,
-    const std::vector<TestRule>& rules,
-    const std::vector<std::string>& hosts,
-    unsigned flags) {
+std::unique_ptr<base::ListValue> ToListValue(const std::vector<TestRule>& vec) {
   ListBuilder builder;
-  for (const auto& rule : rules)
+  for (const TestRule& rule : vec)
     builder.Append(rule.ToValue());
-  WriteManifestAndRuleset(extension_dir, json_rules_filepath,
-                          json_rules_filename, *builder.Build(), hosts, flags);
+  return builder.Build();
 }
 
-void WriteManifestAndRuleset(
-    const base::FilePath& extension_dir,
-    const base::FilePath::CharType* json_rules_filepath,
-    const std::string& json_rules_filename,
-    const base::Value& rules,
-    const std::vector<std::string>& hosts,
-    unsigned flags) {
+void WriteManifestAndRuleset(const base::FilePath& extension_dir,
+                             const TestRulesetInfo& info,
+                             const std::vector<std::string>& hosts,
+                             unsigned flags) {
   // Persist JSON rules file.
-  JSONFileValueSerializer(extension_dir.Append(json_rules_filepath))
-      .Serialize(rules);
+  JSONFileValueSerializer(extension_dir.AppendASCII(info.relative_file_path))
+      .Serialize(info.rules_value);
 
   // Persists a background script if needed.
   if (flags & ConfigFlag::kConfig_HasBackgroundScript) {
@@ -269,7 +258,7 @@ void WriteManifestAndRuleset(
 
   // Persist manifest file.
   JSONFileValueSerializer(extension_dir.Append(kManifestFilename))
-      .Serialize(*CreateManifest(json_rules_filename, hosts, flags));
+      .Serialize(*CreateManifest(info.relative_file_path, hosts, flags));
 }
 
 }  // namespace declarative_net_request

extensions/common/api/declarative_net_request/test_utils.h

@@ -11,12 +11,11 @@
 
 #include "base/files/file_path.h"
 #include "base/optional.h"
+#include "base/values.h"
 #include "extensions/common/url_pattern.h"
 
 namespace base {
 class DictionaryValue;
-class ListValue;
-class Value;
 }  // namespace base
 
 namespace extensions {
@@ -166,23 +165,24 @@ std::unique_ptr<base::DictionaryValue> CreateManifest(
 std::unique_ptr<base::ListValue> ToListValue(
     const std::vector<std::string>& vec);
 
-// Writes the declarative |rules| in the given |extension_dir| together with the
-// manifest file. |hosts| specifies the host permissions, the extensions should
-// have. |flags| is a bitmask of ConfigFlag to configure the extension.
-void WriteManifestAndRuleset(
-    const base::FilePath& extension_dir,
-    const base::FilePath::CharType* json_rules_filepath,
-    const std::string& json_rules_filename,
-    const std::vector<TestRule>& rules,
-    const std::vector<std::string>& hosts,
-    unsigned flags = ConfigFlag::kConfig_None);
-void WriteManifestAndRuleset(
-    const base::FilePath& extension_dir,
-    const base::FilePath::CharType* json_rules_filepath,
-    const std::string& json_rules_filename,
-    const base::Value& rules,
-    const std::vector<std::string>& hosts,
-    unsigned flags = ConfigFlag::kConfig_None);
+// Returns a ListValue corresponding to a vector of TestRules.
+std::unique_ptr<base::ListValue> ToListValue(
+    const std::vector<TestRule>& rules);
+
+// Describes a single extension ruleset.
+struct TestRulesetInfo {
+  std::string relative_file_path;
+  base::Value rules_value;
+};
+
+// Writes the declarative rules specified in |ruleset_info| in the given
+// |extension_dir| together with the manifest file. |hosts| specifies the host
+// permissions, the extensions should have. |flags| is a bitmask of ConfigFlag
+// to configure the extension.
+void WriteManifestAndRuleset(const base::FilePath& extension_dir,
+                             const TestRulesetInfo& ruleset_info,
+                             const std::vector<std::string>& hosts,
+                             unsigned flags = ConfigFlag::kConfig_None);
 
 }  // namespace declarative_net_request
 }  // namespace extensions