Remove linked_ptr from the extensions rules registry.

BUG=582001

Change-Id: Ia2abb1393718c8c0aa54a350ccc065077e362870
Reviewed-on: https://chromium-review.googlesource.com/c/1363797
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615702}

chrome/browser/extensions/api/declarative/declarative_apitest.cc

@@ -119,7 +119,7 @@ class DeclarativeApiTest : public ExtensionApiTest {
             RulesRegistryService::kDefaultRulesRegistryID,
             extensions::declarative_webrequest_constants::kOnRequest);
 
-    std::vector<linked_ptr<api::events::Rule>> rules;
+    std::vector<const api::events::Rule*> rules;
     base::PostTaskWithTraits(
         FROM_HERE, {BrowserThread::IO},
         base::BindOnce(&RulesRegistry::GetAllRules, rules_registry,

chrome/browser/extensions/api/declarative/rules_registry_service_unittest.cc

@@ -32,16 +32,16 @@ const char kExtensionId[] = "foo";
 
 void InsertRule(scoped_refptr<extensions::RulesRegistry> registry,
                 const std::string& id) {
-  std::vector<linked_ptr<extensions::api::events::Rule>> add_rules;
-  add_rules.push_back(make_linked_ptr(new extensions::api::events::Rule));
-  add_rules[0]->id.reset(new std::string(id));
-  std::string error = registry->AddRules(kExtensionId, add_rules);
+  std::vector<extensions::api::events::Rule> add_rules;
+  add_rules.emplace_back();
+  add_rules[0].id.reset(new std::string(id));
+  std::string error = registry->AddRules(kExtensionId, std::move(add_rules));
   EXPECT_TRUE(error.empty());
 }
 
 void VerifyNumberOfRules(scoped_refptr<extensions::RulesRegistry> registry,
                          size_t expected_number_of_rules) {
-  std::vector<linked_ptr<extensions::api::events::Rule>> get_rules;
+  std::vector<const extensions::api::events::Rule*> get_rules;
   registry->GetAllRules(kExtensionId, &get_rules);
   EXPECT_EQ(expected_number_of_rules, get_rules.size());
 }

chrome/browser/extensions/api/declarative/rules_registry_with_cache_unittest.cc

@@ -75,10 +75,10 @@ class RulesRegistryWithCacheTest : public testing::Test {
   std::string AddRule(const std::string& extension_id,
                       const std::string& rule_id,
                       TestRulesRegistry* registry) {
-    std::vector<linked_ptr<api::events::Rule>> add_rules;
-    add_rules.push_back(make_linked_ptr(new api::events::Rule));
-    add_rules[0]->id.reset(new std::string(rule_id));
-    return registry->AddRules(extension_id, add_rules);
+    std::vector<api::events::Rule> add_rules;
+    add_rules.emplace_back();
+    add_rules[0].id.reset(new std::string(rule_id));
+    return registry->AddRules(extension_id, std::move(add_rules));
   }
 
   std::string AddRule(const std::string& extension_id,
@@ -95,7 +95,7 @@ class RulesRegistryWithCacheTest : public testing::Test {
 
   int GetNumberOfRules(const std::string& extension_id,
                        TestRulesRegistry* registry) {
-    std::vector<linked_ptr<api::events::Rule>> get_rules;
+    std::vector<const api::events::Rule*> get_rules;
     registry->GetAllRules(extension_id, &get_rules);
     return get_rules.size();
   }
@@ -191,7 +191,7 @@ TEST_F(RulesRegistryWithCacheTest, GetRules) {
   std::vector<std::string> rules_to_get;
   rules_to_get.push_back(kRuleId);
   rules_to_get.push_back("unknown_rule");
-  std::vector<linked_ptr<api::events::Rule>> gotten_rules;
+  std::vector<const api::events::Rule*> gotten_rules;
   registry_->GetRules(extension1_->id(), rules_to_get, &gotten_rules);
   ASSERT_EQ(1u, gotten_rules.size());
   ASSERT_TRUE(gotten_rules[0]->id.get());
@@ -205,7 +205,7 @@ TEST_F(RulesRegistryWithCacheTest, GetAllRules) {
   EXPECT_EQ("", AddRule(extension2_->id(), kRuleId));
 
   // Check that we get the correct rules.
-  std::vector<linked_ptr<api::events::Rule>> gotten_rules;
+  std::vector<const api::events::Rule*> gotten_rules;
   registry_->GetAllRules(extension1_->id(), &gotten_rules);
   EXPECT_EQ(2u, gotten_rules.size());
   ASSERT_TRUE(gotten_rules[0]->id.get());

chrome/browser/extensions/api/declarative_content/chrome_content_rules_registry.cc

@@ -227,7 +227,7 @@ ChromeContentRulesRegistry::GetMatchingRules(content::WebContents* tab) const {
 
 std::string ChromeContentRulesRegistry::AddRulesImpl(
     const std::string& extension_id,
-    const std::vector<linked_ptr<api::events::Rule>>& api_rules) {
+    const std::vector<const api::events::Rule*>& api_rules) {
   EvaluationScope evaluation_scope(this);
   const Extension* extension = ExtensionRegistry::Get(browser_context())
       ->GetInstalledExtension(extension_id);
@@ -246,7 +246,7 @@ std::string ChromeContentRulesRegistry::AddRulesImpl(
         evaluator.get();
   }
 
-  for (const linked_ptr<api::events::Rule>& api_rule : api_rules) {
+  for (auto* api_rule : api_rules) {
     ExtensionIdRuleIdPair rule_id(extension_id, *api_rule->id);
     DCHECK(content_rules_.find(rule_id) == content_rules_.end());
 

chrome/browser/extensions/api/declarative_content/chrome_content_rules_registry.h

@@ -74,7 +74,7 @@ class ChromeContentRulesRegistry
   // RulesRegistry:
   std::string AddRulesImpl(
       const std::string& extension_id,
-      const std::vector<linked_ptr<api::events::Rule>>& rules) override;
+      const std::vector<const api::events::Rule*>& rules) override;
   std::string RemoveRulesImpl(
       const std::string& extension_id,
       const std::vector<std::string>& rule_identifiers) override;

chrome/browser/extensions/api/declarative_content/chrome_content_rules_registry_unittest.cc

@@ -161,7 +161,7 @@ TEST_F(DeclarativeChromeContentRulesRegistryTest, ActiveRulesDoesntGrow) {
   EXPECT_EQ(0u, registry->GetActiveRulesCountForTesting());
 
   // Add a rule.
-  linked_ptr<api::events::Rule> rule(new api::events::Rule);
+  api::events::Rule rule;
   api::events::Rule::Populate(
       *base::test::ParseJson(
           "{\n"
@@ -176,9 +176,8 @@ TEST_F(DeclarativeChromeContentRulesRegistryTest, ActiveRulesDoesntGrow) {
           "    { \"instanceType\": \"declarativeContent.ShowAction\" }\n"
           "  ]\n"
           "}"),
-      rule.get());
-  std::vector<linked_ptr<api::events::Rule>> rules;
-  rules.push_back(rule);
+      &rule);
+  std::vector<const api::events::Rule*> rules({&rule});
 
   const Extension* extension = env()->MakeExtension(*base::test::ParseJson(
       "{\"page_action\": {}}"));

chrome/test/data/extensions/api_test/declarative/api/background.js

@@ -111,7 +111,8 @@ chrome.test.runTests([
   function testGetRules() {
     var callback = function(rules) {
       chrome.test.assertNoLastError();
-      // We are not given any gurantee on the order in which rules are returned.
+      // We are not given any guarantee on the order in which rules are
+      // returned.
       chrome.test.assertTrue(
           chrome.test.checkDeepEq([outputRule0, outputRule1], rules) ||
           chrome.test.checkDeepEq([outputRule1, outputRule0], rules));
@@ -123,7 +124,8 @@ chrome.test.runTests([
   function testGetRules2() {
     var callback = function(rules) {
       chrome.test.assertNoLastError();
-      // We are not given any gurantee on the order in which rules are returned.
+      // We are not given any guarantee on the order in which rules are
+      // returned.
       chrome.test.assertTrue(
           chrome.test.checkDeepEq([outputRule0, outputRule1], rules) ||
           chrome.test.checkDeepEq([outputRule1, outputRule0], rules));
@@ -147,7 +149,7 @@ chrome.test.runTests([
   // function testGetRules4() {
   //   var callback = function(rules) {
   //     chrome.test.assertNoLastError();
-  //     // We are not given any gurantee on the order in which rules are
+  //     // We are not given any guarantee on the order in which rules are
   //     // returned.
   //     chrome.test.assertTrue(
   //         chrome.test.checkDeepEq([outputRule0, outputRule1], rules) ||

extensions/browser/api/declarative/declarative_api.cc

@@ -217,18 +217,14 @@ ExtensionFunction::ResponseValue
 EventsEventAddRulesFunction::RunAsyncOnCorrectThread() {
   ConvertBinaryListElementsToBase64(args_.get());
 
-  // TODO(devlin): Remove the dependency on linked_ptr here.
-  std::vector<linked_ptr<api::events::Rule>> linked_rules;
-  for (api::events::Rule& rule : params_->rules) {
-    linked_rules.push_back(
-        make_linked_ptr(new api::events::Rule(std::move(rule))));
-  }
-  std::string error = rules_registry_->AddRules(extension_id(), linked_rules);
+  std::vector<const api::events::Rule*> rules_out;
+  std::string error = rules_registry_->AddRules(
+      extension_id(), std::move(params_->rules), &rules_out);
   if (!error.empty())
     return Error(error);
 
   auto rules_value = std::make_unique<base::ListValue>();
-  for (const auto& rule : linked_rules)
+  for (const auto* rule : rules_out)
     rules_value->Append(rule->ToValue());
   return OneArgument(std::move(rules_value));
 }
@@ -306,7 +302,7 @@ bool EventsEventGetRulesFunction::CreateParams() {
 
 ExtensionFunction::ResponseValue
 EventsEventGetRulesFunction::RunAsyncOnCorrectThread() {
-  std::vector<linked_ptr<Rule> > rules;
+  std::vector<const Rule*> rules;
   if (params_->rule_identifiers.get()) {
     rules_registry_->GetRules(extension_id(), *params_->rule_identifiers,
                               &rules);
@@ -315,7 +311,7 @@ EventsEventGetRulesFunction::RunAsyncOnCorrectThread() {
   }
 
   auto rules_value = std::make_unique<base::ListValue>();
-  for (const auto& rule : rules)
+  for (const auto* rule : rules)
     rules_value->Append(rule->ToValue());
   return OneArgument(std::move(rules_value));
 }

extensions/browser/api/declarative/declarative_rule.h

@@ -234,7 +234,7 @@ class DeclarativeRule {
       content::BrowserContext* browser_context,
       const Extension* extension,
       base::Time extension_installation_time,
-      linked_ptr<JsonRule> rule,
+      const JsonRule& rule,
       ConsistencyChecker check_consistency,
       std::string* error);
 
@@ -448,20 +448,20 @@ DeclarativeRule<ConditionT, ActionT>::Create(
     content::BrowserContext* browser_context,
     const Extension* extension,
     base::Time extension_installation_time,
-    linked_ptr<JsonRule> rule,
+    const JsonRule& rule,
     ConsistencyChecker check_consistency,
     std::string* error) {
   std::unique_ptr<DeclarativeRule> error_result;
 
   std::unique_ptr<ConditionSet> conditions = ConditionSet::Create(
-      extension, url_matcher_condition_factory, rule->conditions, error);
+      extension, url_matcher_condition_factory, rule.conditions, error);
   if (!error->empty())
     return std::move(error_result);
   CHECK(conditions.get());
 
   bool bad_message = false;
   std::unique_ptr<ActionSet> actions = ActionSet::Create(
-      browser_context, extension, rule->actions, error, &bad_message);
+      browser_context, extension, rule.actions, error, &bad_message);
   if (bad_message) {
     // TODO(battre) Export concept of bad_message to caller, the extension
     // should be killed in case it is true.
@@ -479,11 +479,11 @@ DeclarativeRule<ConditionT, ActionT>::Create(
     return std::move(error_result);
   }
 
-  CHECK(rule->priority.get());
-  int priority = *(rule->priority);
+  CHECK(rule.priority.get());
+  int priority = *(rule.priority);
 
-  GlobalRuleId rule_id(extension->id(), *(rule->id));
-  Tags tags = rule->tags ? *rule->tags : Tags();
+  GlobalRuleId rule_id(extension->id(), *(rule.id));
+  Tags tags = rule.tags ? *rule.tags : Tags();
   return std::unique_ptr<DeclarativeRule>(
       new DeclarativeRule(rule_id, tags, extension_installation_time,
                           std::move(conditions), std::move(actions), priority));

extensions/browser/api/declarative/declarative_rule_unittest.cc

@@ -299,22 +299,22 @@ TEST(DeclarativeActionTest, ApplyActionSet) {
 
 TEST(DeclarativeRuleTest, Create) {
   typedef DeclarativeRule<FulfillableCondition, SummingAction> Rule;
-  linked_ptr<Rule::JsonRule> json_rule(new Rule::JsonRule);
-  ASSERT_TRUE(Rule::JsonRule::Populate(
-      *ParseJson("{ \n"
-                 "  \"id\": \"rule1\", \n"
-                 "  \"conditions\": [ \n"
-                 "    {\"url_id\": 1, \"max\": 3}, \n"
-                 "    {\"url_id\": 2, \"max\": 5}, \n"
-                 "  ], \n"
-                 "  \"actions\": [ \n"
-                 "    { \n"
-                 "      \"value\": 2 \n"
-                 "    } \n"
-                 "  ], \n"
-                 "  \"priority\": 200 \n"
-                 "}"),
-      json_rule.get()));
+  Rule::JsonRule json_rule;
+  ASSERT_TRUE(
+      Rule::JsonRule::Populate(*ParseJson("{ \n"
+                                          "  \"id\": \"rule1\", \n"
+                                          "  \"conditions\": [ \n"
+                                          "    {\"url_id\": 1, \"max\": 3}, \n"
+                                          "    {\"url_id\": 2, \"max\": 5}, \n"
+                                          "  ], \n"
+                                          "  \"actions\": [ \n"
+                                          "    { \n"
+                                          "      \"value\": 2 \n"
+                                          "    } \n"
+                                          "  ], \n"
+                                          "  \"priority\": 200 \n"
+                                          "}"),
+                               &json_rule));
 
   const char kExtensionId[] = "ext1";
   scoped_refptr<const Extension> extension = ExtensionBuilder()
@@ -369,53 +369,48 @@ TEST(DeclarativeRuleTest, CheckConsistency) {
   typedef DeclarativeRule<FulfillableCondition, SummingAction> Rule;
   URLMatcher matcher;
   std::string error;
-  linked_ptr<Rule::JsonRule> json_rule(new Rule::JsonRule);
+  Rule::JsonRule json_rule;
   const char kExtensionId[] = "ext1";
   scoped_refptr<const Extension> extension = ExtensionBuilder()
                                                  .SetManifest(SimpleManifest())
                                                  .SetID(kExtensionId)
                                                  .Build();
 
-  ASSERT_TRUE(Rule::JsonRule::Populate(
-      *ParseJson("{ \n"
-                 "  \"id\": \"rule1\", \n"
-                 "  \"conditions\": [ \n"
-                 "    {\"url_id\": 1, \"max\": 3}, \n"
-                 "    {\"url_id\": 2, \"max\": 5}, \n"
-                 "  ], \n"
-                 "  \"actions\": [ \n"
-                 "    { \n"
-                 "      \"value\": 2 \n"
-                 "    } \n"
-                 "  ], \n"
-                 "  \"priority\": 200 \n"
-                 "}"),
-      json_rule.get()));
+  ASSERT_TRUE(
+      Rule::JsonRule::Populate(*ParseJson("{ \n"
+                                          "  \"id\": \"rule1\", \n"
+                                          "  \"conditions\": [ \n"
+                                          "    {\"url_id\": 1, \"max\": 3}, \n"
+                                          "    {\"url_id\": 2, \"max\": 5}, \n"
+                                          "  ], \n"
+                                          "  \"actions\": [ \n"
+                                          "    { \n"
+                                          "      \"value\": 2 \n"
+                                          "    } \n"
+                                          "  ], \n"
+                                          "  \"priority\": 200 \n"
+                                          "}"),
+                               &json_rule));
   std::unique_ptr<Rule> rule(Rule::Create(
       matcher.condition_factory(), NULL, extension.get(), base::Time(),
       json_rule, base::Bind(AtLeastOneCondition), &error));
   EXPECT_TRUE(rule);
   EXPECT_EQ("", error);
 
-  ASSERT_TRUE(Rule::JsonRule::Populate(
-      *ParseJson("{ \n"
-                 "  \"id\": \"rule1\", \n"
-                 "  \"conditions\": [ \n"
-                 "  ], \n"
-                 "  \"actions\": [ \n"
-                 "    { \n"
-                 "      \"value\": 2 \n"
-                 "    } \n"
-                 "  ], \n"
-                 "  \"priority\": 200 \n"
-                 "}"),
-      json_rule.get()));
-  rule = Rule::Create(matcher.condition_factory(),
-                      NULL,
-                      extension.get(),
-                      base::Time(),
-                      json_rule,
-                      base::Bind(AtLeastOneCondition),
+  ASSERT_TRUE(Rule::JsonRule::Populate(*ParseJson("{ \n"
+                                                  "  \"id\": \"rule1\", \n"
+                                                  "  \"conditions\": [ \n"
+                                                  "  ], \n"
+                                                  "  \"actions\": [ \n"
+                                                  "    { \n"
+                                                  "      \"value\": 2 \n"
+                                                  "    } \n"
+                                                  "  ], \n"
+                                                  "  \"priority\": 200 \n"
+                                                  "}"),
+                                       &json_rule));
+  rule = Rule::Create(matcher.condition_factory(), NULL, extension.get(),
+                      base::Time(), json_rule, base::Bind(AtLeastOneCondition),
                       &error);
   EXPECT_FALSE(rule);
   EXPECT_EQ("No conditions", error);

extensions/browser/api/declarative/rules_registry.h

@@ -16,7 +16,6 @@
 #include "base/callback_forward.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
-#include "base/memory/linked_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_observer.h"
@@ -62,23 +61,24 @@ class RulesRegistry : public base::RefCountedThreadSafe<RulesRegistry> {
 
   // RulesRegistry implementation:
 
-  // Registers |rules|, owned by |extension_id| to this RulesRegistry.
-  // If a concrete RuleRegistry does not support some of the rules,
-  // it may ignore them.
+  // Registers |rules| in this RulesRegistry. If a concrete RuleRegistry does
+  // not support some of the rules, it may ignore them.
   //
   // |rules| is a list of Rule instances following the definition of the
   // declarative extension APIs. It is guaranteed that each rule in |rules| has
   // a unique name within the scope of |extension_id| that has not been
   // registered before, unless it has been removed again.
-  // The ownership of rules remains with the caller.
   //
-  // Returns an empty string if the function is successful or an error
-  // message otherwise.
+  // Returns an empty string if the function is successful or an error message
+  // otherwise. If the function is successful, and if the |rules_out| parameter
+  // is non-null, pointers to the added rules are returned.
   //
   // IMPORTANT: This function is atomic. Either all rules that are deemed
   // relevant are added or none.
-  std::string AddRules(const std::string& extension_id,
-                       const std::vector<linked_ptr<api::events::Rule>>& rules);
+  std::string AddRules(
+      const std::string& extension_id,
+      std::vector<api::events::Rule> rules_in,
+      std::vector<const api::events::Rule*>* rules_out = nullptr);
 
   // Unregisters all rules listed in |rule_identifiers| and owned by
   // |extension_id| from this RulesRegistry.
@@ -101,14 +101,14 @@ class RulesRegistry : public base::RefCountedThreadSafe<RulesRegistry> {
   // registered in this RuleRegistry. Entries in |rule_identifiers| that
   // are unknown are ignored.
   //
-  // The returned rules are stored in |out|. Ownership is passed to the caller.
+  // The returned rules are stored in |out|.
   void GetRules(const std::string& extension_id,
                 const std::vector<std::string>& rule_identifiers,
-                std::vector<linked_ptr<api::events::Rule>>* out);
+                std::vector<const api::events::Rule*>* out);
 
   // Same as GetRules but returns all rules owned by |extension_id|.
   void GetAllRules(const std::string& extension_id,
-                   std::vector<linked_ptr<api::events::Rule>>* out);
+                   std::vector<const api::events::Rule*>* out);
 
   // Called to notify the RulesRegistry that the extension availability has
   // changed, so that the registry can update which rules are active.
@@ -146,7 +146,7 @@ class RulesRegistry : public base::RefCountedThreadSafe<RulesRegistry> {
   // automatically cache the rules and re-call *Impl on browser startup.
   virtual std::string AddRulesImpl(
       const std::string& extension_id,
-      const std::vector<linked_ptr<api::events::Rule>>& rules) = 0;
+      const std::vector<const api::events::Rule*>& rules) = 0;
   virtual std::string RemoveRulesImpl(
       const std::string& extension_id,
       const std::vector<std::string>& rule_identifiers) = 0;
@@ -157,10 +157,14 @@ class RulesRegistry : public base::RefCountedThreadSafe<RulesRegistry> {
   friend class base::RefCountedThreadSafe<RulesRegistry>;
   friend class RulesCacheDelegate;
 
-  typedef std::string RuleId;
-  typedef std::pair<ExtensionId, RuleId> RulesDictionaryKey;
-  typedef std::map<RulesDictionaryKey, linked_ptr<api::events::Rule>>
-      RulesDictionary;
+  using RuleId = std::string;
+  using RulesDictionaryKey = std::pair<ExtensionId, RuleId>;
+
+  // NOTE: The property of stability of iterators of a map during insertion is
+  // relied upon here. If this type needs to change, beware that this will
+  // severely complicate returning valid pointers to callers of member functions
+  // of this class.
+  using RulesDictionary = std::map<RulesDictionaryKey, api::events::Rule>;
   enum ProcessChangedRulesState {
     // ProcessChangedRules can never be called, |cache_delegate_| is NULL.
     NEVER_PROCESS,
@@ -170,19 +174,22 @@ class RulesRegistry : public base::RefCountedThreadSafe<RulesRegistry> {
     // to schedule one.
     NOT_SCHEDULED_FOR_PROCESSING
   };
-  typedef std::map<ExtensionId, ProcessChangedRulesState> ProcessStateMap;
+  using ProcessStateMap = std::map<ExtensionId, ProcessChangedRulesState>;
 
   base::WeakPtr<RulesRegistry> GetWeakPtr() {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     return weak_ptr_factory_.GetWeakPtr();
   }
 
-  // Internal implementation of the AddRules interface which adds
-  // |from_manifest| which is true when the source is the manifest.
+  // Internal implementation of the AddRules interface which adds the rules to
+  // the |destination| RulesDictionary. If the function is successful, and if
+  // the |rules_out| parameter is non-null, pointers to the added rules are
+  // returned.
   std::string AddRulesInternal(
       const std::string& extension_id,
-      const std::vector<linked_ptr<api::events::Rule>>& rules,
-      RulesDictionary* out);
+      std::vector<api::events::Rule> rules_in,
+      RulesDictionary* destination,
+      std::vector<const api::events::Rule*>* rules_out);
 
   // The precondition for calling this method is that all rules have unique IDs.
   // AddRules establishes this precondition and calls into this method.
@@ -190,17 +197,19 @@ class RulesRegistry : public base::RefCountedThreadSafe<RulesRegistry> {
   // CheckAndFillInOptionalRules for improved performance.
   //
   // Returns an empty string if the function is successful or an error
-  // message otherwise.
-  std::string AddRulesNoFill(
-      const std::string& extension_id,
-      const std::vector<linked_ptr<api::events::Rule>>& rules,
-      RulesDictionary* out);
+  // message otherwise. If the function is successful, and if the
+  // |rules_out| parameter is non-null, pointers to the added rules are
+  // returned.
+  std::string AddRulesNoFill(const std::string& extension_id,
+                             std::vector<api::events::Rule> rules_in,
+                             RulesDictionary* destination,
+                             std::vector<const api::events::Rule*>* rules_out);
 
   // Same as GetRules but returns all rules owned by |extension_id| for a given
   // |rules| dictionary.
   void GetRules(const std::string& extension_id,
-                const RulesDictionary& rules,
-                std::vector<linked_ptr<api::events::Rule>>* out);
+                RulesDictionary* rules,
+                std::vector<const api::events::Rule*>* out);
 
   // Common processing after extension's rules have changed.
   void ProcessChangedRules(const std::string& extension_id);
@@ -266,11 +275,10 @@ class RulesRegistry : public base::RefCountedThreadSafe<RulesRegistry> {
   // returns a non-empty error message.
   std::string CheckAndFillInOptionalRules(
       const std::string& extension_id,
-      const std::vector<linked_ptr<api::events::Rule>>& rules);
+      std::vector<api::events::Rule>* rules);
 
   // Initializes the priority fields in case they have not been set.
-  void FillInOptionalPriorities(
-      const std::vector<linked_ptr<api::events::Rule>>& rules);
+  void FillInOptionalPriorities(std::vector<api::events::Rule>* rules);
 
   // Removes all |identifiers| of |extension_id| from |used_rule_identifiers_|.
   void RemoveUsedRuleIdentifiers(const std::string& extension_id,
@@ -280,9 +288,8 @@ class RulesRegistry : public base::RefCountedThreadSafe<RulesRegistry> {
   // |extension_id|.
   void RemoveAllUsedRuleIdentifiers(const std::string& extension_id);
 
-  typedef std::string RuleIdentifier;
-  typedef std::map<ExtensionId, std::set<RuleIdentifier> > RuleIdentifiersMap;
-  RuleIdentifiersMap used_rule_identifiers_;
+  using RuleIdentifier = std::string;
+  std::map<ExtensionId, std::set<RuleIdentifier>> used_rule_identifiers_;
   int last_generated_rule_identifier_id_;
 
   // |cache_delegate_| is owned by the registry service. If |cache_delegate_| is

extensions/browser/api/declarative/rules_registry_unittest.cc

@@ -36,37 +36,44 @@ TEST(RulesRegistryTest, FillOptionalIdentifiers) {
 
   // Add rules and check that their identifiers are filled and unique.
 
-  std::vector<linked_ptr<api::events::Rule>> add_rules;
-  add_rules.push_back(make_linked_ptr(new api::events::Rule));
-  add_rules.push_back(make_linked_ptr(new api::events::Rule));
-  error = registry->AddRules(kExtensionId, add_rules);
-  EXPECT_TRUE(error.empty()) << error;
-
-  std::vector<linked_ptr<api::events::Rule>> get_rules;
+  {
+    std::vector<api::events::Rule> add_rules;
+    add_rules.emplace_back();
+    add_rules.emplace_back();
+    error = registry->AddRules(kExtensionId, std::move(add_rules));
+    EXPECT_TRUE(error.empty()) << error;
+  }
+
+  std::vector<const api::events::Rule*> get_rules;
   registry->GetAllRules(kExtensionId, &get_rules);
 
   ASSERT_EQ(2u, get_rules.size());
 
   ASSERT_TRUE(get_rules[0]->id.get());
-  EXPECT_NE("", *get_rules[0]->id);
+  // Make a copy of the id that this rule was assigned so that we can try to
+  // reuse it later when the rule is gone.
+  std::string id0 = *get_rules[0]->id;
+  EXPECT_NE("", id0);
 
   ASSERT_TRUE(get_rules[1]->id.get());
   EXPECT_NE("", *get_rules[1]->id);
 
-  EXPECT_NE(*get_rules[0]->id, *get_rules[1]->id);
+  EXPECT_NE(id0, *get_rules[1]->id);
 
   EXPECT_EQ(1u /*extensions*/ + 2u /*rules*/,
             registry->GetNumberOfUsedRuleIdentifiersForTesting());
 
   // Check that we cannot add a new rule with the same ID.
 
-  std::vector<linked_ptr<api::events::Rule>> add_rules_2;
-  add_rules_2.push_back(make_linked_ptr(new api::events::Rule));
-  add_rules_2[0]->id.reset(new std::string(*get_rules[0]->id));
-  error = registry->AddRules(kExtensionId, add_rules_2);
-  EXPECT_FALSE(error.empty());
+  {
+    std::vector<api::events::Rule> add_rules;
+    add_rules.emplace_back();
+    add_rules[0].id.reset(new std::string(id0));
+    error = registry->AddRules(kExtensionId, std::move(add_rules));
+    EXPECT_FALSE(error.empty());
+  }
 
-  std::vector<linked_ptr<api::events::Rule>> get_rules_2;
+  std::vector<const api::events::Rule*> get_rules_2;
   registry->GetAllRules(kExtensionId, &get_rules_2);
   ASSERT_EQ(2u, get_rules_2.size());
   EXPECT_EQ(1u /*extensions*/ + 2u /*rules*/,
@@ -75,26 +82,28 @@ TEST(RulesRegistryTest, FillOptionalIdentifiers) {
   // Check that we can register the old rule IDs once they were unregistered.
 
   std::vector<std::string> remove_rules_3;
-  remove_rules_3.push_back(*get_rules[0]->id);
+  remove_rules_3.push_back(id0);
   error = registry->RemoveRules(kExtensionId, remove_rules_3);
   EXPECT_TRUE(error.empty()) << error;
 
   EXPECT_EQ(1u /*extensions*/ + 1u /*rules*/,
             registry->GetNumberOfUsedRuleIdentifiersForTesting());
 
-  std::vector<linked_ptr<api::events::Rule>> get_rules_3a;
+  std::vector<const api::events::Rule*> get_rules_3a;
   registry->GetAllRules(kExtensionId, &get_rules_3a);
   ASSERT_EQ(1u, get_rules_3a.size());
 
-  std::vector<linked_ptr<api::events::Rule>> add_rules_3;
-  add_rules_3.push_back(make_linked_ptr(new api::events::Rule));
-  add_rules_3[0]->id.reset(new std::string(*get_rules[0]->id));
-  error = registry->AddRules(kExtensionId, add_rules_3);
-  EXPECT_TRUE(error.empty()) << error;
-  EXPECT_EQ(1u /*extensions*/ + 2u /*rules*/,
-            registry->GetNumberOfUsedRuleIdentifiersForTesting());
-
-  std::vector<linked_ptr<api::events::Rule>> get_rules_3b;
+  {
+    std::vector<api::events::Rule> add_rules;
+    add_rules.emplace_back();
+    add_rules[0].id.reset(new std::string(id0));
+    error = registry->AddRules(kExtensionId, std::move(add_rules));
+    EXPECT_TRUE(error.empty()) << error;
+    EXPECT_EQ(1u /*extensions*/ + 2u /*rules*/,
+              registry->GetNumberOfUsedRuleIdentifiersForTesting());
+  }
+
+  std::vector<const api::events::Rule*> get_rules_3b;
   registry->GetAllRules(kExtensionId, &get_rules_3b);
   ASSERT_EQ(2u, get_rules_3b.size());
 
@@ -105,20 +114,22 @@ TEST(RulesRegistryTest, FillOptionalIdentifiers) {
   EXPECT_EQ(0u /*extensions*/ + 0u /*rules*/,
             registry->GetNumberOfUsedRuleIdentifiersForTesting());
 
-  std::vector<linked_ptr<api::events::Rule>> get_rules_4a;
+  std::vector<const api::events::Rule*> get_rules_4a;
   registry->GetAllRules(kExtensionId, &get_rules_4a);
   ASSERT_TRUE(get_rules_4a.empty());
 
-  std::vector<linked_ptr<api::events::Rule>> add_rules_4;
-  add_rules_4.push_back(make_linked_ptr(new api::events::Rule));
-  add_rules_4[0]->id.reset(new std::string(kRuleId));
-  error = registry->AddRules(kExtensionId, add_rules_4);
-  EXPECT_TRUE(error.empty()) << error;
+  {
+    std::vector<api::events::Rule> add_rules;
+    add_rules.emplace_back();
+    add_rules[0].id.reset(new std::string(kRuleId));
+    error = registry->AddRules(kExtensionId, std::move(add_rules));
+    EXPECT_TRUE(error.empty()) << error;
+  }
 
   EXPECT_EQ(1u /*extensions*/ + 1u /*rules*/,
             registry->GetNumberOfUsedRuleIdentifiersForTesting());
 
-  std::vector<linked_ptr<api::events::Rule>> get_rules_4b;
+  std::vector<const api::events::Rule*> get_rules_4b;
   registry->GetAllRules(kExtensionId, &get_rules_4b);
 
   ASSERT_EQ(1u, get_rules_4b.size());
@@ -147,14 +158,16 @@ TEST(RulesRegistryTest, FillOptionalPriority) {
 
   // Add rules and check that their priorities are filled if they are empty.
 
-  std::vector<linked_ptr<api::events::Rule>> add_rules;
-  add_rules.push_back(make_linked_ptr(new api::events::Rule));
-  add_rules[0]->priority.reset(new int(2));
-  add_rules.push_back(make_linked_ptr(new api::events::Rule));
-  error = registry->AddRules(kExtensionId, add_rules);
-  EXPECT_TRUE(error.empty()) << error;
+  {
+    std::vector<api::events::Rule> add_rules;
+    add_rules.emplace_back();
+    add_rules[0].priority.reset(new int(2));
+    add_rules.emplace_back();
+    error = registry->AddRules(kExtensionId, std::move(add_rules));
+    EXPECT_TRUE(error.empty()) << error;
+  }
 
-  std::vector<linked_ptr<api::events::Rule>> get_rules;
+  std::vector<const api::events::Rule*> get_rules;
   registry->GetAllRules(kExtensionId, &get_rules);
 
   ASSERT_EQ(2u, get_rules.size());
@@ -220,7 +233,7 @@ TEST(RulesRegistryTest, TwoRulesInManifest) {
   // Simulate what RulesRegistryService would do on extension load.
   registry->OnExtensionLoaded(extension.get());
 
-  std::vector<linked_ptr<api::events::Rule>> get_rules;
+  std::vector<const api::events::Rule*> get_rules;
   registry->GetAllRules(kExtensionId, &get_rules);
 
   ASSERT_EQ(2u, get_rules.size());
@@ -287,17 +300,20 @@ TEST(RulesRegistryTest, DeleteRuleInManifest) {
       content::BrowserThread::UI, "declarativeContent.onPageChanged", key);
   // Simulate what RulesRegistryService would do on extension load.
   registry->OnExtensionLoaded(extension.get());
-  // Add some extra rules outside of the manifest.
-  std::vector<linked_ptr<api::events::Rule>> add_rules;
-  linked_ptr<api::events::Rule> rule_1 = make_linked_ptr(new api::events::Rule);
-  rule_1->id.reset(new std::string("rule_1"));
-  linked_ptr<api::events::Rule> rule_2 = make_linked_ptr(new api::events::Rule);
-  rule_2->id.reset(new std::string("rule_2"));
-  add_rules.push_back(rule_1);
-  add_rules.push_back(rule_2);
-  registry->AddRules(kExtensionId, add_rules);
-
-  std::vector<linked_ptr<api::events::Rule>> get_rules;
+
+  {
+    // Add some extra rules outside of the manifest.
+    std::vector<api::events::Rule> add_rules;
+    api::events::Rule rule_1;
+    rule_1.id.reset(new std::string("rule_1"));
+    api::events::Rule rule_2;
+    rule_2.id.reset(new std::string("rule_2"));
+    add_rules.push_back(std::move(rule_1));
+    add_rules.push_back(std::move(rule_2));
+    registry->AddRules(kExtensionId, std::move(add_rules));
+  }
+
+  std::vector<const api::events::Rule*> get_rules;
   registry->GetAllRules(kExtensionId, &get_rules);
   ASSERT_EQ(3u, get_rules.size());
   EXPECT_EQ("manifest_rule_0", *(get_rules[0]->id));

extensions/browser/api/declarative/test_rules_registry.cc

@@ -32,7 +32,7 @@ TestRulesRegistry::TestRulesRegistry(content::BrowserContext* browser_context,
 
 std::string TestRulesRegistry::AddRulesImpl(
     const std::string& extension_id,
-    const std::vector<linked_ptr<api::events::Rule>>& rules) {
+    const std::vector<const api::events::Rule*>& rules) {
   return result_;
 }
 

extensions/browser/api/declarative/test_rules_registry.h

@@ -26,7 +26,7 @@ class TestRulesRegistry : public RulesRegistry {
   // RulesRegistry implementation:
   std::string AddRulesImpl(
       const std::string& extension_id,
-      const std::vector<linked_ptr<api::events::Rule>>& rules) override;
+      const std::vector<const api::events::Rule*>& rules) override;
   std::string RemoveRulesImpl(
       const std::string& extension_id,
       const std::vector<std::string>& rule_identifiers) override;

extensions/browser/api/declarative_webrequest/webrequest_rules_registry.cc

@@ -151,7 +151,7 @@ std::list<LinkedPtrEventResponseDelta> WebRequestRulesRegistry::CreateDeltas(
 
 std::string WebRequestRulesRegistry::AddRulesImpl(
     const std::string& extension_id,
-    const std::vector<linked_ptr<api::events::Rule>>& rules) {
+    const std::vector<const api::events::Rule*>& rules) {
   typedef std::pair<WebRequestRule::RuleId, linked_ptr<const WebRequestRule>>
       IdRulePair;
   typedef std::vector<IdRulePair> RulesVector;
@@ -166,13 +166,13 @@ std::string WebRequestRulesRegistry::AddRulesImpl(
       extension_info_map_->extensions().GetByID(extension_id);
   RulesMap& registered_rules = webrequest_rules_[extension_id];
 
-  for (const linked_ptr<api::events::Rule>& rule : rules) {
+  for (auto* rule : rules) {
     const WebRequestRule::RuleId& rule_id(*rule->id);
     DCHECK(registered_rules.find(rule_id) == registered_rules.end());
 
     std::unique_ptr<WebRequestRule> webrequest_rule(WebRequestRule::Create(
         url_matcher_.condition_factory(), browser_context(), extension,
-        extension_installation_time, rule,
+        extension_installation_time, *rule,
         base::Bind(&Checker, base::Unretained(extension)), &error));
     if (!error.empty()) {
       // We don't return here, because we want to clear temporary

extensions/browser/api/declarative_webrequest/webrequest_rules_registry.h

@@ -91,7 +91,7 @@ class WebRequestRulesRegistry : public RulesRegistry {
   // Implementation of RulesRegistry:
   std::string AddRulesImpl(
       const std::string& extension_id,
-      const std::vector<linked_ptr<api::events::Rule>>& rules) override;
+      const std::vector<const api::events::Rule*>& rules) override;
   std::string RemoveRulesImpl(
       const std::string& extension_id,
       const std::vector<std::string>& rule_identifiers) override;

extensions/common/api/declarative/declarative_manifest_data.cc

@@ -143,23 +143,35 @@ std::unique_ptr<DeclarativeManifestData> DeclarativeManifestData::FromValue(
       return std::unique_ptr<DeclarativeManifestData>();
     }
 
-    linked_ptr<Rule> rule(new Rule());
-    if (!Rule::Populate(*dict, rule.get())) {
+    Rule rule;
+    if (!Rule::Populate(*dict, &rule)) {
       error_builder.Append("rule failed to populate");
       return std::unique_ptr<DeclarativeManifestData>();
     }
 
-    if (!ConvertManifestRule(*rule, &error_builder))
+    if (!ConvertManifestRule(rule, &error_builder))
       return std::unique_ptr<DeclarativeManifestData>();
 
-    result->event_rules_map_[event].push_back(rule);
+    result->event_rules_map_[event].push_back(std::move(rule));
   }
   return result;
 }
 
-std::vector<linked_ptr<DeclarativeManifestData::Rule>>&
+std::vector<DeclarativeManifestData::Rule>
 DeclarativeManifestData::RulesForEvent(const std::string& event) {
-  return event_rules_map_[event];
+  const auto& rules = event_rules_map_[event];
+  std::vector<DeclarativeManifestData::Rule> result;
+  result.reserve(rules.size());
+  for (const auto& rule : rules) {
+    // TODO(rdevlin.cronin): It would be nice if we could have the RulesRegistry
+    // reference the rules owned here, but the ownership issues are a bit
+    // tricky. Revisit this.
+    std::unique_ptr<base::DictionaryValue> rule_value = rule.ToValue();
+    std::unique_ptr<DeclarativeManifestData::Rule> rule_copy =
+        DeclarativeManifestData::Rule::FromValue(*rule_value);
+    result.push_back(std::move(*rule_copy));
+  }
+  return result;
 }
 
 }  // namespace extensions

extensions/common/api/declarative/declarative_manifest_data.h

@@ -8,7 +8,6 @@
 #include <vector>
 
 #include "base/macros.h"
-#include "base/memory/linked_ptr.h"
 #include "base/strings/string16.h"
 #include "extensions/common/api/events.h"
 #include "extensions/common/extension.h"
@@ -19,7 +18,7 @@ namespace extensions {
 // The parsed form of the "event_rules" manifest entry.
 class DeclarativeManifestData : public Extension::ManifestData {
  public:
-  typedef extensions::api::events::Rule Rule;
+  using Rule = extensions::api::events::Rule;
 
   DeclarativeManifestData();
   ~DeclarativeManifestData() override;
@@ -34,11 +33,10 @@ class DeclarativeManifestData : public Extension::ManifestData {
       const base::Value& value,
       base::string16* error);
 
-  std::vector<linked_ptr<DeclarativeManifestData::Rule>>& RulesForEvent(
-      const std::string& event);
+  std::vector<Rule> RulesForEvent(const std::string& event);
 
  private:
-  std::map<std::string, std::vector<linked_ptr<Rule>>> event_rules_map_;
+  std::map<std::string, std::vector<Rule>> event_rules_map_;
   DISALLOW_COPY_AND_ASSIGN(DeclarativeManifestData);
 };
 

extensions/common/api/declarative/declarative_manifest_unittest.cc

@@ -18,7 +18,7 @@ TEST_F(DeclarativeManifestTest, Valid) {
   DeclarativeManifestData* manifest_data =
       DeclarativeManifestData::Get(extension.get());
   ASSERT_TRUE(manifest_data);
-  std::vector<linked_ptr<DeclarativeManifestData::Rule>>& rules =
+  std::vector<DeclarativeManifestData::Rule> rules =
       manifest_data->RulesForEvent("foo");
   EXPECT_EQ(1u, rules.size());
   std::unique_ptr<base::DictionaryValue> expected_rule = ParseDictionary(
@@ -30,7 +30,7 @@ TEST_F(DeclarativeManifestTest, Valid) {
       "    \"instanceType\" : \"condition_type\""
       "  }]"
       "}");
-  EXPECT_TRUE(expected_rule->Equals(rules[0]->ToValue().get()));
+  EXPECT_TRUE(expected_rule->Equals(rules[0].ToValue().get()));
 }
 
 TEST_F(DeclarativeManifestTest, ConditionMissingType) {