UrlPatternIndex: Introduce format version and add correctness checks.

This CL
  - Introduces url_pattern_index::kUrlPatternIndexFormatVersion to keep a track
    of the current format version of the UrlPatternIndex schema.
  - Adds checks to url_pattern_index clients (Declarative net request and
    subresource_filter) to ensure that their ruleset format versions are updated
    whenever kUrlPatternIndexFormatVersion is changed.
  - Adds presubmit checks to components/url_pattern_index to ensure that
    kUrlPatternIndexFormatVersion is kept updated.

BUG=877214, 755717

Change-Id: Ib82b775e3da2112d5f17a2040ec495a7ff8e18d9
Reviewed-on: https://chromium-review.googlesource.com/1187706
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586098}

components/subresource_filter/core/common/indexed_ruleset.cc

@@ -35,6 +35,13 @@ int LocalGetChecksum(const uint8_t* data, size_t size) {
 // tools/perf/core/default_local_state.json.
 const int RulesetIndexer::kIndexedFormatVersion = 20;
 
+// This static assert is meant to catch cases where
+// url_pattern_index::kUrlPatternIndexFormatVersion is incremented without
+// updating RulesetIndexer::kIndexedFormatVersion.
+static_assert(url_pattern_index::kUrlPatternIndexFormatVersion == 1,
+              "kUrlPatternIndexFormatVersion has changed, make sure you've "
+              "also updated RulesetIndexer::kIndexedFormatVersion above.");
+
 RulesetIndexer::RulesetIndexer()
     : blacklist_(&builder_), whitelist_(&builder_), deactivation_(&builder_) {}
 

components/url_pattern_index/PRESUBMIT.py

+# Copyright 2018 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Presubmit script for components/url_pattern_index directory.
+
+See https://www.chromium.org/developers/how-tos/depottools/presubmit-scripts
+for more details about the presubmit API built into depot_tools.
+"""
+
+def CheckUrlPatternIndexFormatVersion(input_api, output_api):
+  """ Checks the kUrlPatternIndexFormatVersion is modified when necessary.
+
+  Whenever any of the following files is changed:
+    - components/url_pattern_index/flat/url_pattern_index.fbs
+    - components/url_pattern_index/url_pattern_index.cc
+  and kUrlPatternIndexFormatVersion stays intact, this check returns a
+  presubmit warning to make sure the value is updated if necessary.
+  """
+
+  url_pattern_index_files_changed = False
+  url_pattern_index_version_changed = False
+
+  for affected_file in input_api.AffectedFiles():
+    basename = input_api.basename(affected_file.LocalPath())
+
+    if (basename == 'url_pattern_index.fbs' or
+        basename == 'url_pattern_index.cc'):
+      url_pattern_index_files_changed = True
+
+    if basename == 'url_pattern_index.h':
+      for (_, line) in affected_file.ChangedContents():
+        if 'constexpr int kUrlPatternIndexFormatVersion' in line:
+          url_pattern_index_version_changed = True
+          break
+
+  out = []
+  if url_pattern_index_files_changed and not url_pattern_index_version_changed:
+    out.append(output_api.PresubmitPromptWarning(
+        'Please make sure that url_pattern_index::kUrlPatternIndexFormatVersion'
+        ' is modified if necessary.'))
+
+  return out
+
+def CheckChangeOnUpload(input_api, output_api):
+  return CheckUrlPatternIndexFormatVersion(input_api, output_api)

components/url_pattern_index/flat/url_pattern_index.fbs

@@ -4,6 +4,9 @@
 
 namespace url_pattern_index.flat;
 
+// NOTE: Increment url_pattern_index::kUrlPatternIndexFormatVersion whenever
+// making a breaking change to this schema.
+
 // Corresponds to url_pattern_index::proto::UrlPatternType.
 enum UrlPatternType : ubyte {
   SUBSTRING,

components/url_pattern_index/url_pattern_index.h

@@ -62,6 +62,12 @@ UrlRuleOffset SerializeUrlRule(const proto::UrlRule& rule,
 // value if |lhs_domain| should be ordered after |rhs_domain|.
 int CompareDomains(base::StringPiece lhs_domain, base::StringPiece rhs_domain);
 
+// The current format version of UrlPatternIndex.
+// Increase this value when introducing an incompatible change to the
+// UrlPatternIndex schema (flat/url_pattern_index.fbs). url_pattern_index
+// clients can use this as a signal to rebuild rulesets.
+constexpr int kUrlPatternIndexFormatVersion = 1;
+
 // The class used to construct an index over the URL patterns of a set of URL
 // rules. The rules themselves need to be converted to FlatBuffers format by the
 // client of this class, as well as persisted into the |flat_builder| that is

extensions/browser/api/declarative_net_request/utils.cc

@@ -20,6 +20,7 @@
 #include "base/threading/thread_restrictions.h"
 #include "base/timer/elapsed_timer.h"
 #include "base/values.h"
+#include "components/url_pattern_index/url_pattern_index.h"
 #include "extensions/browser/api/declarative_net_request/constants.h"
 #include "extensions/browser/api/declarative_net_request/flat/extension_ruleset_generated.h"
 #include "extensions/browser/api/declarative_net_request/flat_ruleset_indexer.h"
@@ -50,6 +51,13 @@ namespace dnr_api = extensions::api::declarative_net_request;
 // necessary.
 constexpr int kIndexedRulesetFormatVersion = 1;
 
+// This static assert is meant to catch cases where
+// url_pattern_index::kUrlPatternIndexFormatVersion is incremented without
+// updating kIndexedRulesetFormatVersion.
+static_assert(url_pattern_index::kUrlPatternIndexFormatVersion == 1,
+              "kUrlPatternIndexFormatVersion has changed, make sure you've "
+              "also updated kIndexedRulesetFormatVersion above.");
+
 constexpr int kInvalidIndexedRulesetFormatVersion = -1;
 
 int g_indexed_ruleset_format_version_for_testing =