Commit 1b476301 authored by Karandeep Bhatia's avatar Karandeep Bhatia Committed by Commit Bot

DNR: Modify FlatRulesetIndexer to release data buffer.

FlatBufferBuilder now supports returning the released data buffer.
Modify FlatRulesetIndexer to use it. This is more safer and ergonomical
as the lifetime of the buffer is not tied to FlatRulesetIndexer now.

BUG=1043200

Change-Id: I983531ebd93820c9c28876c424302135e96651e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485841
Commit-Queue: Kelvin Jiang <kelvinjiang@chromium.org>
Auto-Submit: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: default avatarKelvin Jiang <kelvinjiang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819025}
parent 9fafd090
...@@ -255,7 +255,7 @@ void FlatRulesetIndexer::AddUrlRule(const IndexedRule& indexed_rule) { ...@@ -255,7 +255,7 @@ void FlatRulesetIndexer::AddUrlRule(const IndexedRule& indexed_rule) {
transform_offset, request_headers_offset, response_headers_offset)); transform_offset, request_headers_offset, response_headers_offset));
} }
void FlatRulesetIndexer::Finish() { flatbuffers::DetachedBuffer FlatRulesetIndexer::FinishAndReleaseBuffer() {
DCHECK(!finished_); DCHECK(!finished_);
finished_ = true; finished_ = true;
...@@ -280,11 +280,8 @@ void FlatRulesetIndexer::Finish() { ...@@ -280,11 +280,8 @@ void FlatRulesetIndexer::Finish() {
regex_rules_offset, regex_rules_offset,
extension_metadata_offset); extension_metadata_offset);
flat::FinishExtensionIndexedRulesetBuffer(builder_, root_offset); flat::FinishExtensionIndexedRulesetBuffer(builder_, root_offset);
}
base::span<const uint8_t> FlatRulesetIndexer::GetData() { return builder_.Release();
DCHECK(finished_);
return base::make_span(builder_.GetBufferPointer(), builder_.GetSize());
} }
std::vector<FlatRulesetIndexer::UrlPatternIndexBuilder*> std::vector<FlatRulesetIndexer::UrlPatternIndexBuilder*>
......
...@@ -10,7 +10,6 @@ ...@@ -10,7 +10,6 @@
#include <set> #include <set>
#include <vector> #include <vector>
#include "base/containers/span.h"
#include "base/macros.h" #include "base/macros.h"
#include "components/url_pattern_index/url_pattern_index.h" #include "components/url_pattern_index/url_pattern_index.h"
#include "extensions/browser/api/declarative_net_request/flat/extension_ruleset_generated.h" #include "extensions/browser/api/declarative_net_request/flat/extension_ruleset_generated.h"
...@@ -35,12 +34,9 @@ class FlatRulesetIndexer { ...@@ -35,12 +34,9 @@ class FlatRulesetIndexer {
// Returns the number of rules added till now. // Returns the number of rules added till now.
size_t indexed_rules_count() const { return indexed_rules_count_; } size_t indexed_rules_count() const { return indexed_rules_count_; }
// Finishes the ruleset construction. // Finishes the ruleset construction and releases the underlying indexed data
void Finish(); // buffer.
flatbuffers::DetachedBuffer FinishAndReleaseBuffer();
// Returns the data buffer, which is still owned by FlatRulesetIndexer.
// Finish() must be called prior to calling this.
base::span<const uint8_t> GetData();
private: private:
using UrlPatternIndexBuilder = url_pattern_index::UrlPatternIndexBuilder; using UrlPatternIndexBuilder = url_pattern_index::UrlPatternIndexBuilder;
......
...@@ -351,18 +351,18 @@ void VerifyExtensionMetadata( ...@@ -351,18 +351,18 @@ void VerifyExtensionMetadata(
const flat::ExtensionIndexedRuleset* AddRuleAndGetRuleset( const flat::ExtensionIndexedRuleset* AddRuleAndGetRuleset(
const std::vector<IndexedRule>& rules_to_index, const std::vector<IndexedRule>& rules_to_index,
FlatRulesetIndexer* indexer) { flatbuffers::DetachedBuffer* buffer) {
FlatRulesetIndexer indexer;
for (const auto& rule : rules_to_index) for (const auto& rule : rules_to_index)
indexer->AddUrlRule(rule); indexer.AddUrlRule(rule);
indexer->Finish(); *buffer = indexer.FinishAndReleaseBuffer();
base::span<const uint8_t> data = indexer->GetData(); EXPECT_EQ(rules_to_index.size(), indexer.indexed_rules_count());
EXPECT_EQ(rules_to_index.size(), indexer->indexed_rules_count()); flatbuffers::Verifier verifier(buffer->data(), buffer->size());
flatbuffers::Verifier verifier(data.data(), data.size());
if (!flat::VerifyExtensionIndexedRulesetBuffer(verifier)) if (!flat::VerifyExtensionIndexedRulesetBuffer(verifier))
return nullptr; return nullptr;
return flat::GetExtensionIndexedRuleset(data.data()); return flat::GetExtensionIndexedRuleset(buffer->data());
} }
// Helper which: // Helper which:
...@@ -374,9 +374,9 @@ const flat::ExtensionIndexedRuleset* AddRuleAndGetRuleset( ...@@ -374,9 +374,9 @@ const flat::ExtensionIndexedRuleset* AddRuleAndGetRuleset(
void AddRulesAndVerifyIndex(const std::vector<IndexedRule>& rules_to_index, void AddRulesAndVerifyIndex(const std::vector<IndexedRule>& rules_to_index,
const std::vector<const IndexedRule*> const std::vector<const IndexedRule*>
expected_index_lists[flat::IndexType_count]) { expected_index_lists[flat::IndexType_count]) {
FlatRulesetIndexer indexer; flatbuffers::DetachedBuffer buffer;
const flat::ExtensionIndexedRuleset* ruleset = const flat::ExtensionIndexedRuleset* ruleset =
AddRuleAndGetRuleset(rules_to_index, &indexer); AddRuleAndGetRuleset(rules_to_index, &buffer);
ASSERT_TRUE(ruleset); ASSERT_TRUE(ruleset);
for (size_t i = 0; i < flat::IndexType_count; ++i) { for (size_t i = 0; i < flat::IndexType_count; ++i) {
...@@ -573,9 +573,9 @@ TEST_F(FlatRulesetIndexerTest, RegexRules) { ...@@ -573,9 +573,9 @@ TEST_F(FlatRulesetIndexerTest, RegexRules) {
dnr_api::RULE_ACTION_TYPE_MODIFYHEADERS, nullptr, base::nullopt, dnr_api::RULE_ACTION_TYPE_MODIFYHEADERS, nullptr, base::nullopt,
std::move(request_headers), {})); std::move(request_headers), {}));
FlatRulesetIndexer indexer; flatbuffers::DetachedBuffer buffer;
const flat::ExtensionIndexedRuleset* ruleset = const flat::ExtensionIndexedRuleset* ruleset =
AddRuleAndGetRuleset(rules_to_index, &indexer); AddRuleAndGetRuleset(rules_to_index, &buffer);
ASSERT_TRUE(ruleset); ASSERT_TRUE(ruleset);
// All the indices should be empty, since we only have regex rules. // All the indices should be empty, since we only have regex rules.
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/callback_helpers.h" #include "base/callback_helpers.h"
#include "base/check_op.h" #include "base/check_op.h"
#include "base/containers/span.h"
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/files/file_util.h" #include "base/files/file_util.h"
#include "base/json/json_reader.h" #include "base/json/json_reader.h"
...@@ -470,9 +471,9 @@ ParseInfo RulesetSource::IndexAndPersistRules( ...@@ -470,9 +471,9 @@ ParseInfo RulesetSource::IndexAndPersistRules(
} }
} }
} }
indexer.Finish(); flatbuffers::DetachedBuffer buffer = indexer.FinishAndReleaseBuffer();
if (!PersistIndexedRuleset(indexed_path_,
if (!PersistIndexedRuleset(indexed_path_, indexer.GetData(), base::make_span(buffer.data(), buffer.size()),
&ruleset_checksum)) { &ruleset_checksum)) {
return ParseInfo(ParseResult::ERROR_PERSISTING_RULESET, return ParseInfo(ParseResult::ERROR_PERSISTING_RULESET,
nullptr /* rule_id */); nullptr /* rule_id */);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment