DNR: Change where the static indexed ruleset is stored.

Currently the single static indexed ruleset for an extension is stored
at $extension_path/_metadata/generated_indexed_ruleset. Since we plan to
support multiple static rulesets, we need to store multiple indexed
rulesets.

This CL changes the static indexed ruleset location to
$extension_path/_metadata/generated_indexed_rulesets/_ruleset$ruleset_id.
This helps us move the code away from the assumption of a single static
ruleset.

As a result of this existing extensions with a DNR ruleset will
initially fail to load the DNR ruleset. However the re-indexing logic
should step in and ensure the extension works seamlessly. A side-effect
for these extensions is that the old indexed ruleset will remain on
disk. However this is ok since the API is still on beta and doesn't have
significant usage yet. Furthermore, we do periodically clean up
extension folders after extension updates.

BUG=754526

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

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

@@ -165,15 +165,18 @@ class RuleIndexingTest : public DNRTestBase {
     // Overwrite the JSON rules file with some invalid json.
     if (persist_invalid_json_file_) {
       std::string data = "invalid json";
-      base::WriteFile(extension_dir_.Append(kJSONRulesetFilepath), data.c_str(),
-                      data.size());
+      ASSERT_EQ(static_cast<int>(data.size()),
+                base::WriteFile(extension_dir_.Append(kJSONRulesetFilepath),
+                                data.c_str(), data.size()));
     }
 
     if (persist_initial_indexed_ruleset_) {
       std::string data = "user ruleset";
-      base::WriteFile(
-          extension_dir_.Append(file_util::GetIndexedRulesetRelativePath()),
-          data.c_str(), data.size());
+      base::FilePath ruleset_path = extension_dir_.Append(
+          file_util::GetIndexedRulesetRelativePath(kMinValidStaticRulesetID));
+      ASSERT_TRUE(base::CreateDirectory(ruleset_path.DirName()));
+      ASSERT_EQ(static_cast<int>(data.size()),
+                base::WriteFile(ruleset_path, data.c_str(), data.size()));
     }
   }
 

chrome/browser/extensions/unpacked_installer.cc

@@ -65,7 +65,7 @@ void MaybeCleanupMetadataFolder(const base::FilePath& extension_path) {
   const std::vector<base::FilePath> reserved_filepaths =
       file_util::GetReservedMetadataFilePaths(extension_path);
   for (const auto& file : reserved_filepaths)
-    base::DeleteFile(file, false /*recursive*/);
+    base::DeleteFile(file, true /*recursive*/);
 
   const base::FilePath& metadata_dir = extension_path.Append(kMetadataFolder);
   if (base::IsDirectoryEmpty(metadata_dir))

extensions/browser/api/declarative_net_request/ruleset_source.cc

@@ -290,11 +290,11 @@ RulesetSource RulesetSource::CreateStatic(const Extension& extension) {
       declarative_net_request::DNRManifestData::GetRuleset(extension);
 
   DCHECK_GE(info.id, kMinValidStaticRulesetID);
-  return RulesetSource(
-      extension.path().Append(info.relative_path),
-      extension.path().Append(file_util::GetIndexedRulesetRelativePath()),
-      info.id, dnr_api::SOURCE_TYPE_MANIFEST, dnr_api::MAX_NUMBER_OF_RULES,
-      extension.id());
+  return RulesetSource(extension.path().Append(info.relative_path),
+                       extension.path().Append(
+                           file_util::GetIndexedRulesetRelativePath(info.id)),
+                       info.id, dnr_api::SOURCE_TYPE_MANIFEST,
+                       dnr_api::MAX_NUMBER_OF_RULES, extension.id());
 }
 
 // static

extensions/browser/content_verifier.cc

@@ -27,6 +27,7 @@
 #include "extensions/browser/content_verifier/content_verifier_utils.h"
 #include "extensions/browser/content_verifier_delegate.h"
 #include "extensions/browser/extension_file_task_runner.h"
+#include "extensions/common/api/declarative_net_request/dnr_manifest_data.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension_l10n_util.h"
 #include "extensions/common/file_util.h"
@@ -125,9 +126,19 @@ 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)) {
+    const DNRManifestData::RulesetInfo& info =
+        DNRManifestData::GetRuleset(*extension);
+    indexed_ruleset_paths->insert(
+        canonicalize_path(file_util::GetIndexedRulesetRelativePath(info.id)));
+  }
+
   return std::make_unique<ContentVerifierIOData::ExtensionData>(
       std::move(image_paths), std::move(background_or_content_paths),
-      extension->version(), source_type);
+      std::move(indexed_ruleset_paths), extension->version(), source_type);
 }
 
 // Returns all locales, possibly with lowercasing them for case-insensitive OS.
@@ -715,6 +726,8 @@ bool ContentVerifier::ShouldVerifyAnyPaths(
       *(data->canonical_browser_image_paths);
   const std::set<CanonicalRelativePath>& background_or_content_paths =
       *(data->canonical_background_or_content_paths);
+  const std::set<CanonicalRelativePath>& indexed_ruleset_paths =
+      *(data->canonical_indexed_ruleset_paths);
 
   base::Optional<std::set<std::string>> all_locale_candidates;
 
@@ -723,9 +736,6 @@ bool ContentVerifier::ShouldVerifyAnyPaths(
           base::FilePath(kManifestFilename));
   const base::FilePath messages_file(kMessagesFilename);
   const base::FilePath locales_relative_dir(kLocaleFolder);
-  const CanonicalRelativePath indexed_ruleset_path =
-      content_verifier_utils::CanonicalizeRelativePath(
-          file_util::GetIndexedRulesetRelativePath());
   for (const base::FilePath& relative_unix_path : relative_unix_paths) {
     if (relative_unix_path.empty())
       continue;
@@ -750,7 +760,8 @@ bool ContentVerifier::ShouldVerifyAnyPaths(
     if (base::Contains(browser_images, canonical_path_value))
       continue;
 
-    if (canonical_path_value == indexed_ruleset_path)
+    // Skip indexed rulesets since these are generated.
+    if (base::Contains(indexed_ruleset_paths, canonical_path_value))
       continue;
 
     const base::FilePath canonical_path(canonical_path_value.value());

extensions/browser/content_verifier_io_data.cc

@@ -15,11 +15,15 @@ ContentVerifierIOData::ExtensionData::ExtensionData(
         canonical_browser_image_paths,
     std::unique_ptr<std::set<CanonicalRelativePath>>
         canonical_background_or_content_paths,
+    std::unique_ptr<std::set<CanonicalRelativePath>>
+        canonical_indexed_ruleset_paths,
     const base::Version& version,
     ContentVerifierDelegate::VerifierSourceType source_type)
     : canonical_browser_image_paths(std::move(canonical_browser_image_paths)),
       canonical_background_or_content_paths(
           std::move(canonical_background_or_content_paths)),
+      canonical_indexed_ruleset_paths(
+          std::move(canonical_indexed_ruleset_paths)),
       version(version),
       source_type(source_type) {}
 

extensions/browser/content_verifier_io_data.h

@@ -32,6 +32,11 @@ class ContentVerifierIOData {
     // content scripts.
     std::unique_ptr<std::set<CanonicalRelativePath>>
         canonical_background_or_content_paths;
+
+    // Set of indexed ruleset paths used by the Declarative Net Request API.
+    std::unique_ptr<std::set<CanonicalRelativePath>>
+        canonical_indexed_ruleset_paths;
+
     base::Version version;
     ContentVerifierDelegate::VerifierSourceType source_type;
 
@@ -39,6 +44,8 @@ class ContentVerifierIOData {
                       canonical_browser_image_paths,
                   std::unique_ptr<std::set<CanonicalRelativePath>>
                       canonical_background_or_content_paths,
+                  std::unique_ptr<std::set<CanonicalRelativePath>>
+                      canonical_indexed_ruleset_paths,
                   const base::Version& version,
                   ContentVerifierDelegate::VerifierSourceType source_type);
     ~ExtensionData();

extensions/browser/content_verifier_unittest.cc

@@ -416,9 +416,6 @@ TEST_F(ContentVerifierTest, NeverVerifiedPaths) {
       //   - locales with mixedcase lang.
       FilePathVariants(
           base::FilePath(FILE_PATH_LITERAL("_locales/en_GB/messages.json"))),
-      // Indexed ruleset is never verified.
-      FilePathVariants(base::FilePath(
-          FILE_PATH_LITERAL("_metadata/generated_indexed_ruleset"))),
   };
 
   for (const auto& test_case : kNeverVerifiedTestCases) {

extensions/common/constants.cc

@@ -30,8 +30,8 @@ const base::FilePath::CharType kVerifiedContentsFilename[] =
     FILE_PATH_LITERAL("verified_contents.json");
 const base::FilePath::CharType kComputedHashesFilename[] =
     FILE_PATH_LITERAL("computed_hashes.json");
-const base::FilePath::CharType kIndexedRulesetFilename[] =
-    FILE_PATH_LITERAL("generated_indexed_ruleset");
+const base::FilePath::CharType kIndexedRulesetDirectory[] =
+    FILE_PATH_LITERAL("generated_indexed_rulesets");
 
 const char kInstallDirectoryName[] = "Extensions";
 

extensions/common/constants.h

@@ -45,8 +45,8 @@ extern const base::FilePath::CharType kVerifiedContentsFilename[];
 // Name of the computed hashes file within the metadata folder.
 extern const base::FilePath::CharType kComputedHashesFilename[];
 
-// Name of the indexed ruleset file for the Declarative Net Request API.
-extern const base::FilePath::CharType kIndexedRulesetFilename[];
+// Name of the indexed ruleset directory for the Declarative Net Request API.
+extern const base::FilePath::CharType kIndexedRulesetDirectory[];
 
 // The name of the directory inside the profile where extensions are
 // installed to.

extensions/common/file_util.cc

@@ -23,6 +23,7 @@
 #include "base/macros.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ -585,15 +586,21 @@ base::FilePath GetVerifiedContentsPath(const base::FilePath& extension_path) {
 base::FilePath GetComputedHashesPath(const base::FilePath& extension_path) {
   return extension_path.Append(kMetadataFolder).Append(kComputedHashesFilename);
 }
-base::FilePath GetIndexedRulesetRelativePath() {
-  return base::FilePath(kMetadataFolder).Append(kIndexedRulesetFilename);
+base::FilePath GetIndexedRulesetDirectoryRelativePath() {
+  return base::FilePath(kMetadataFolder).Append(kIndexedRulesetDirectory);
+}
+base::FilePath GetIndexedRulesetRelativePath(int static_ruleset_id) {
+  const char* kRulesetPrefix = "_ruleset";
+  std::string filename =
+      kRulesetPrefix + base::NumberToString(static_ruleset_id);
+  return GetIndexedRulesetDirectoryRelativePath().AppendASCII(filename);
 }
 
 std::vector<base::FilePath> GetReservedMetadataFilePaths(
     const base::FilePath& extension_path) {
   return {GetVerifiedContentsPath(extension_path),
           GetComputedHashesPath(extension_path),
-          extension_path.Append(GetIndexedRulesetRelativePath())};
+          extension_path.Append(GetIndexedRulesetDirectoryRelativePath())};
 }
 
 }  // namespace file_util

extensions/common/file_util.h

@@ -170,9 +170,15 @@ MessageBundle::SubstitutionMap* LoadMessageBundleSubstitutionMapFromPaths(
 base::FilePath GetVerifiedContentsPath(const base::FilePath& extension_path);
 base::FilePath GetComputedHashesPath(const base::FilePath& extension_path);
 
-// Helper function to get path used for the indexed ruleset by the Declarative
-// Net Request API.
-base::FilePath GetIndexedRulesetRelativePath();
+// Helper function to get the relative path for the directory containing static
+// indexed rulesets. Path is relative to the extension path. Used by the
+// Declarative Net Request API.
+base::FilePath GetIndexedRulesetDirectoryRelativePath();
+
+// Helper function to get the relative path for a given static indexed ruleset.
+// Path is relative to the extension path. This is used by the Declarative Net
+// Request API.
+base::FilePath GetIndexedRulesetRelativePath(int static_ruleset_id);
 
 // Returns the list of file-paths reserved for use by the Extension system in
 // the kMetadataFolder.