Declarative Net Request: Use base::span to pass indexed ruleset data buffer.

This CL consolidates the code to use base::span to pass indexed ruleset data
buffer, doing away with FlatRulesetIndexer::SerializedData. It provides for more
readable code.

BUG=696822

Change-Id: I382eec8328fb82af74bbd49e483304329d8c4bf7
Reviewed-on: https://chromium-review.googlesource.com/1152869Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584621}

extensions/browser/api/declarative_net_request/flat_ruleset_indexer.cc

@@ -109,10 +109,9 @@ void FlatRulesetIndexer::Finish() {
   flat::FinishExtensionIndexedRulesetBuffer(builder_, root_offset);
 }
 
-FlatRulesetIndexer::SerializedData FlatRulesetIndexer::GetData() {
+base::span<const uint8_t> FlatRulesetIndexer::GetData() {
   DCHECK(finished_);
-  return SerializedData(builder_.GetBufferPointer(),
-                        base::strict_cast<size_t>(builder_.GetSize()));
+  return base::make_span(builder_.GetBufferPointer(), builder_.GetSize());
 }
 
 FlatRulesetIndexer::UrlPatternIndexBuilder* FlatRulesetIndexer::GetBuilder(

extensions/browser/api/declarative_net_request/flat_ruleset_indexer.h

@@ -6,9 +6,9 @@
 #define EXTENSIONS_BROWSER_API_DECLARATIVE_NET_REQUEST_FLAT_RULESET_INDEXER_H_
 
 #include <stddef.h>
-#include <utility>
 #include <vector>
 
+#include "base/containers/span.h"
 #include "base/macros.h"
 #include "components/url_pattern_index/url_pattern_index.h"
 #include "extensions/browser/api/declarative_net_request/flat/extension_ruleset_generated.h"
@@ -24,9 +24,6 @@ struct IndexedRule;
 // Request API.
 class FlatRulesetIndexer {
  public:
-  // Represents the address and the size of the buffer storing the ruleset.
-  using SerializedData = std::pair<const uint8_t*, size_t>;
-
   FlatRulesetIndexer();
   ~FlatRulesetIndexer();
 
@@ -41,7 +38,7 @@ class FlatRulesetIndexer {
 
   // Returns the data buffer, which is still owned by FlatRulesetIndexer.
   // Finish() must be called prior to calling this.
-  SerializedData GetData();
+  base::span<const uint8_t> GetData();
 
  private:
   using UrlPatternIndexBuilder = url_pattern_index::UrlPatternIndexBuilder;

extensions/browser/api/declarative_net_request/flat_ruleset_indexer_unittest.cc

@@ -195,15 +195,15 @@ void AddRulesAndVerifyIndex(const std::vector<IndexedRule>& blocking_rules,
     indexer.AddUrlRule(rule);
 
   indexer.Finish();
-  FlatRulesetIndexer::SerializedData data = indexer.GetData();
+  base::span<const uint8_t> data = indexer.GetData();
   EXPECT_EQ(
       blocking_rules.size() + allowing_rules.size() + redirect_rules.size(),
       indexer.indexed_rules_count());
-  flatbuffers::Verifier verifier(data.first, data.second);
+  flatbuffers::Verifier verifier(data.data(), data.size());
   ASSERT_TRUE(flat::VerifyExtensionIndexedRulesetBuffer(verifier));
 
   const flat::ExtensionIndexedRuleset* ruleset =
-      flat::GetExtensionIndexedRuleset(data.first);
+      flat::GetExtensionIndexedRuleset(data.data());
   ASSERT_TRUE(ruleset);
 
   VerifyIndexEquality(blocking_rules, ruleset->blocking_index());

extensions/browser/api/declarative_net_request/ruleset_matcher.cc

@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/containers/span.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
@@ -47,8 +48,10 @@ RulesetMatcher::LoadRulesetResult RulesetMatcher::CreateVerifiedMatcher(
     return kLoadErrorFileRead;
 
   // This guarantees that no memory access will end up outside the buffer.
-  if (!IsValidRulesetData(reinterpret_cast<const uint8_t*>(ruleset_data.data()),
-                          ruleset_data.size(), expected_ruleset_checksum)) {
+  if (!IsValidRulesetData(
+          base::make_span(reinterpret_cast<const uint8_t*>(ruleset_data.data()),
+                          ruleset_data.size()),
+          expected_ruleset_checksum)) {
     return kLoadErrorRulesetVerification;
   }
 

extensions/browser/api/declarative_net_request/utils.cc

@@ -40,8 +40,8 @@ namespace {
 namespace dnr_api = extensions::api::declarative_net_request;
 
 // Returns the checksum of the given serialized |data|.
-int GetChecksum(const FlatRulesetIndexer::SerializedData& data) {
-  uint32_t hash = base::PersistentHash(data.first, data.second);
+int GetChecksum(base::span<const uint8_t> data) {
+  uint32_t hash = base::PersistentHash(data.data(), data.size());
 
   // Strip off the sign bit since this needs to be persisted in preferences
   // which don't support unsigned ints.
@@ -50,7 +50,7 @@ int GetChecksum(const FlatRulesetIndexer::SerializedData& data) {
 
 // Helper function to persist the indexed ruleset |data| for |extension|.
 bool PersistRuleset(const Extension& extension,
-                    const FlatRulesetIndexer::SerializedData& data,
+                    base::span<const uint8_t> data,
                     int* ruleset_checksum) {
   DCHECK(ruleset_checksum);
 
@@ -59,10 +59,12 @@ bool PersistRuleset(const Extension& extension,
 
   // Create the directory corresponding to |path| if it does not exist and then
   // persist the ruleset.
-  const int data_size = base::checked_cast<int>(data.second);
+  if (!base::IsValueInRangeForNumericType<int>(data.size()))
+    return false;
+  const int data_size = static_cast<int>(data.size());
   const bool success =
       base::CreateDirectory(path.DirName()) &&
-      base::WriteFile(path, reinterpret_cast<const char*>(data.first),
+      base::WriteFile(path, reinterpret_cast<const char*>(data.data()),
                       data_size) == data_size;
 
   if (success)
@@ -130,9 +132,7 @@ ParseInfo IndexAndPersistRulesImpl(const base::Value& rules,
   indexer.Finish();
   UMA_HISTOGRAM_TIMES(kIndexRulesTimeHistogram, timer.Elapsed());
 
-  // The actual data buffer is still owned by |indexer|.
-  const FlatRulesetIndexer::SerializedData data = indexer.GetData();
-  if (!PersistRuleset(extension, data, ruleset_checksum))
+  if (!PersistRuleset(extension, indexer.GetData(), ruleset_checksum))
     return ParseInfo(ParseResult::ERROR_PERSISTING_RULESET);
 
   if (!all_rules_parsed) {
@@ -268,12 +268,9 @@ void IndexAndPersistRules(service_manager::Connector* connector,
   }
 }
 
-bool IsValidRulesetData(const uint8_t* data,
-                        size_t size,
-                        int expected_checksum) {
-  flatbuffers::Verifier verifier(data, size);
-  FlatRulesetIndexer::SerializedData serialized_data(data, size);
-  return expected_checksum == GetChecksum(serialized_data) &&
+bool IsValidRulesetData(base::span<const uint8_t> data, int expected_checksum) {
+  flatbuffers::Verifier verifier(data.data(), data.size());
+  return expected_checksum == GetChecksum(data) &&
          flat::VerifyExtensionIndexedRulesetBuffer(verifier);
 }
 

extensions/browser/api/declarative_net_request/utils.h

@@ -11,6 +11,7 @@
 #include <vector>
 
 #include "base/callback_forward.h"
+#include "base/containers/span.h"
 #include "base/macros.h"
 #include "base/optional.h"
 
@@ -76,11 +77,9 @@ void IndexAndPersistRules(service_manager::Connector* connector,
                           const Extension& extension,
                           IndexAndPersistRulesCallback callback);
 
-// Returns true if |data| and |size| represent a valid data buffer containing
-// indexed ruleset data with |expected_checksum|.
-bool IsValidRulesetData(const uint8_t* data,
-                        size_t size,
-                        int expected_checksum);
+// Returns true if |data| represents a valid data buffer containing indexed
+// ruleset data with |expected_checksum|.
+bool IsValidRulesetData(base::span<const uint8_t> data, int expected_checksum);
 
 }  // namespace declarative_net_request
 }  // namespace extensions