Make URLMatcherConditionSet refcounted

This CL makes the URLMatcherConditionSet refcounted so that we don't need to copy-by-value it anymore in stl containers. This is a preparation for extending it by additional criteria for filtering schemas and ports.

BUG=112155
TEST=no

Review URL: https://chromiumcodereview.appspot.com/9986001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@130887 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 71 insertions, 88 deletions

diff --git a/chrome/browser/extensions/api/declarative/url_matcher.cc b/chrome/browser/extensions/api/declarative/url_matcher.cc
index 30993cd..b44662c 100644
--- a/chrome/browser/extensions/api/declarative/url_matcher.cc
+++ b/chrome/browser/extensions/api/declarative/url_matcher.cc
@@ -386,8 +386,6 @@ bool URLMatcherConditionFactory::SubstringPatternPointerCompare::operator()(
 // URLMatcherConditionSet
 //
 
-URLMatcherConditionSet::URLMatcherConditionSet() : id_(-1) {}
-
 URLMatcherConditionSet::~URLMatcherConditionSet() {}
 
 URLMatcherConditionSet::URLMatcherConditionSet(
@@ -396,22 +394,11 @@ URLMatcherConditionSet::URLMatcherConditionSet(
     : id_(id),
       conditions_(conditions) {}
 
-URLMatcherConditionSet::URLMatcherConditionSet(
-    const URLMatcherConditionSet& rhs)
-    : id_(rhs.id_), conditions_(rhs.conditions_) {}
-
-URLMatcherConditionSet& URLMatcherConditionSet::operator=(
-    const URLMatcherConditionSet& rhs) {
-  id_ = rhs.id_;
-  conditions_ = rhs.conditions_;
-  return *this;
-}
-
 bool URLMatcherConditionSet::IsMatch(
     const std::set<SubstringPattern::ID>& matching_substring_patterns,
     const GURL& url) const {
   for (Conditions::const_iterator i = conditions_.begin();
-      i != conditions_.end(); ++i) {
+       i != conditions_.end(); ++i) {
     if (!i->IsMatch(matching_substring_patterns, url))
       return false;
   }
@@ -428,12 +415,12 @@ URLMatcher::URLMatcher() {}
 URLMatcher::~URLMatcher() {}
 
 void URLMatcher::AddConditionSets(
-    const std::vector<URLMatcherConditionSet>& condition_sets) {
-  for (std::vector<URLMatcherConditionSet>::const_iterator i =
-      condition_sets.begin(); i != condition_sets.end(); ++i) {
-    DCHECK(url_matcher_condition_sets_.find(i->id()) ==
-          url_matcher_condition_sets_.end());
-    url_matcher_condition_sets_[i->id()] = *i;
+    const URLMatcherConditionSet::Vector& condition_sets) {
+  for (URLMatcherConditionSet::Vector::const_iterator i =
+       condition_sets.begin(); i != condition_sets.end(); ++i) {
+    DCHECK(url_matcher_condition_sets_.find((*i)->id()) ==
+        url_matcher_condition_sets_.end());
+    url_matcher_condition_sets_[(*i)->id()] = *i;
   }
   UpdateInternalDatastructures();
 }
@@ -441,7 +428,7 @@ void URLMatcher::AddConditionSets(
 void URLMatcher::RemoveConditionSets(
     const std::vector<URLMatcherConditionSet::ID>& condition_set_ids) {
   for (std::vector<URLMatcherConditionSet::ID>::const_iterator i =
-      condition_set_ids.begin(); i != condition_set_ids.end(); ++i) {
+       condition_set_ids.begin(); i != condition_set_ids.end(); ++i) {
     DCHECK(url_matcher_condition_sets_.find(*i) !=
         url_matcher_condition_sets_.end());
     url_matcher_condition_sets_.erase(*i);
@@ -467,7 +454,7 @@ std::set<URLMatcherConditionSet::ID> URLMatcher::MatchURL(const GURL& url) {
   // were fulfilled.
   std::set<URLMatcherConditionSet::ID> result;
   for (std::set<SubstringPattern::ID>::const_iterator i = matches.begin();
-      i != matches.end(); ++i) {
+       i != matches.end(); ++i) {
     // For each URLMatcherConditionSet there is exactly one condition
     // registered in substring_match_triggers_. This means that the following
     // logic tests each URLMatcherConditionSet exactly once if it can be
@@ -475,8 +462,8 @@ std::set<URLMatcherConditionSet::ID> URLMatcher::MatchURL(const GURL& url) {
     std::set<URLMatcherConditionSet::ID>& condition_sets =
         substring_match_triggers_[*i];
     for (std::set<URLMatcherConditionSet::ID>::const_iterator j =
-        condition_sets.begin(); j != condition_sets.end(); ++j) {
-      if (url_matcher_condition_sets_[*j].IsMatch(matches, url))
+         condition_sets.begin(); j != condition_sets.end(); ++j) {
+      if (url_matcher_condition_sets_[*j]->IsMatch(matches, url))
         result.insert(*j);
     }
   }
@@ -507,10 +494,10 @@ void URLMatcher::UpdateSubstringSetMatcher(bool full_url_conditions) {
       condition_set_iter != url_matcher_condition_sets_.end();
       ++condition_set_iter) {
     const URLMatcherConditionSet::Conditions& conditions =
-        condition_set_iter->second.conditions();
+        condition_set_iter->second->conditions();
     for (URLMatcherConditionSet::Conditions::const_iterator condition_iter =
-        conditions.begin(); condition_iter != conditions.end();
-        ++condition_iter) {
+         conditions.begin(); condition_iter != conditions.end();
+         ++condition_iter) {
       // If we are called to process Full URL searches, ignore all others,
       // and vice versa.
       if (full_url_conditions == condition_iter->IsFullURLCondition())
@@ -558,10 +545,10 @@ void URLMatcher::UpdateTriggers() {
       condition_set_iter != url_matcher_condition_sets_.end();
       ++condition_set_iter) {
     const URLMatcherConditionSet::Conditions& conditions =
-        condition_set_iter->second.conditions();
+        condition_set_iter->second->conditions();
     for (URLMatcherConditionSet::Conditions::const_iterator condition_iter =
-        conditions.begin(); condition_iter != conditions.end();
-        ++condition_iter) {
+         conditions.begin(); condition_iter != conditions.end();
+         ++condition_iter) {
       const SubstringPattern* pattern = condition_iter->substring_pattern();
       substring_pattern_frequencies[pattern->id()]++;
     }
@@ -580,7 +567,7 @@ void URLMatcher::UpdateTriggers() {
       condition_set_iter != url_matcher_condition_sets_.end();
       ++condition_set_iter) {
     const URLMatcherConditionSet::Conditions& conditions =
-        condition_set_iter->second.conditions();
+        condition_set_iter->second->conditions();
     if (conditions.empty())
       continue;
     URLMatcherConditionSet::Conditions::const_iterator condition_iter =
@@ -596,7 +583,7 @@ void URLMatcher::UpdateTriggers() {
         trigger = current_id;
       }
     }
-    substring_match_triggers_[trigger].insert(condition_set_iter->second.id());
+    substring_match_triggers_[trigger].insert(condition_set_iter->second->id());
   }
 }
 
@@ -607,10 +594,10 @@ void URLMatcher::UpdateConditionFactory() {
       condition_set_iter != url_matcher_condition_sets_.end();
       ++condition_set_iter) {
     const URLMatcherConditionSet::Conditions& conditions =
-        condition_set_iter->second.conditions();
+        condition_set_iter->second->conditions();
     for (URLMatcherConditionSet::Conditions::const_iterator condition_iter =
-        conditions.begin(); condition_iter != conditions.end();
-        ++condition_iter) {
+         conditions.begin(); condition_iter != conditions.end();
+         ++condition_iter) {
       used_patterns.insert(condition_iter->substring_pattern()->id());
     }
   }
diff --git a/chrome/browser/extensions/api/declarative/url_matcher.h b/chrome/browser/extensions/api/declarative/url_matcher.h
index 0e163eb..51b244a 100644
--- a/chrome/browser/extensions/api/declarative/url_matcher.h
+++ b/chrome/browser/extensions/api/declarative/url_matcher.h
@@ -9,6 +9,7 @@
 #include <set>
 #include <vector>
 
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "chrome/browser/extensions/api/declarative/substring_set_matcher.h"
@@ -180,16 +181,13 @@ class URLMatcherConditionFactory {
 
 // This class represents a set of conditions that all need to match on a
 // given URL in order to be considered a match.
-class URLMatcherConditionSet {
+class URLMatcherConditionSet : public base::RefCounted<URLMatcherConditionSet> {
  public:
   typedef int ID;
   typedef std::set<URLMatcherCondition> Conditions;
+  typedef std::vector<scoped_refptr<URLMatcherConditionSet> > Vector;
 
-  URLMatcherConditionSet();
-  ~URLMatcherConditionSet();
   URLMatcherConditionSet(ID id, const Conditions& conditions);
-  URLMatcherConditionSet(const URLMatcherConditionSet& rhs);
-  URLMatcherConditionSet& operator=(const URLMatcherConditionSet& rhs);
 
   ID id() const { return id_; }
   const Conditions& conditions() const { return conditions_; }
@@ -199,8 +197,12 @@ class URLMatcherConditionSet {
       const GURL& url) const;
 
  private:
+  friend class base::RefCounted<URLMatcherConditionSet>;
+  ~URLMatcherConditionSet();
   ID id_;
   Conditions conditions_;
+
+  DISALLOW_COPY_AND_ASSIGN(URLMatcherConditionSet);
 };
 
 // This class allows matching one URL against a large set of
@@ -215,8 +217,7 @@ class URLMatcher {
   // This is an expensive operation as it triggers pre-calculations on the
   // currently registered condition sets. Do not call this operation many
   // times with a single condition set in each call.
-  void AddConditionSets(
-      const std::vector<URLMatcherConditionSet>& condition_sets);
+  void AddConditionSets(const URLMatcherConditionSet::Vector& condition_sets);
 
   // Removes the listed condition sets. All |condition_set_ids| must be
   // currently registered. This function should be called with large batches
@@ -249,7 +250,8 @@ class URLMatcher {
 
   // Maps the ID of a URLMatcherConditionSet to the respective
   // URLMatcherConditionSet.
-  typedef std::map<URLMatcherConditionSet::ID, URLMatcherConditionSet>
+  typedef std::map<URLMatcherConditionSet::ID,
+                   scoped_refptr<URLMatcherConditionSet> >
       URLMatcherConditionSets;
   URLMatcherConditionSets url_matcher_condition_sets_;
 
diff --git a/chrome/browser/extensions/api/declarative/url_matcher_unittest.cc b/chrome/browser/extensions/api/declarative/url_matcher_unittest.cc
index 5d0839a..dae2ecc 100644
--- a/chrome/browser/extensions/api/declarative/url_matcher_unittest.cc
+++ b/chrome/browser/extensions/api/declarative/url_matcher_unittest.cc
@@ -284,7 +284,7 @@ TEST(URLMatcherConditionFactoryTest, TestFullSearches) {
 // URLMatcherConditionSet
 //
 
-TEST(URLMatcherConditionSetTest, Constructors) {
+TEST(URLMatcherConditionSetTest, Constructor) {
   URLMatcherConditionFactory factory;
   URLMatcherCondition m1 = factory.CreateHostSuffixCondition("example.com");
   URLMatcherCondition m2 = factory.CreatePathContainsCondition("foo");
@@ -293,20 +293,10 @@ TEST(URLMatcherConditionSetTest, Constructors) {
   conditions.insert(m1);
   conditions.insert(m2);
 
-  URLMatcherConditionSet condition_set(1, conditions);
-  EXPECT_EQ(1, condition_set.id());
-  EXPECT_EQ(2u, condition_set.conditions().size());
-
-  std::set<URLMatcherCondition> other_conditions;
-  other_conditions.insert(m1);
-  URLMatcherConditionSet condition_set2(2, other_conditions);
-  condition_set2 = condition_set;
-  EXPECT_EQ(1, condition_set2.id());
-  EXPECT_EQ(2u, condition_set2.conditions().size());
-
-  URLMatcherConditionSet condition_set3(condition_set);
-  EXPECT_EQ(1, condition_set2.id());
-  EXPECT_EQ(2u, condition_set2.conditions().size());
+  scoped_refptr<URLMatcherConditionSet> condition_set(
+      new URLMatcherConditionSet(1, conditions));
+  EXPECT_EQ(1, condition_set->id());
+  EXPECT_EQ(2u, condition_set->conditions().size());
 }
 
 TEST(URLMatcherConditionSetTest, Matching) {
@@ -321,17 +311,18 @@ TEST(URLMatcherConditionSetTest, Matching) {
   conditions.insert(m1);
   conditions.insert(m2);
 
-  URLMatcherConditionSet condition_set(1, conditions);
-  EXPECT_EQ(1, condition_set.id());
-  EXPECT_EQ(2u, condition_set.conditions().size());
+  scoped_refptr<URLMatcherConditionSet> condition_set(
+      new URLMatcherConditionSet(1, conditions));
+  EXPECT_EQ(1, condition_set->id());
+  EXPECT_EQ(2u, condition_set->conditions().size());
 
   std::set<SubstringPattern::ID> matching_substring_patterns;
   matching_substring_patterns.insert(m1.substring_pattern()->id());
-  EXPECT_FALSE(condition_set.IsMatch(matching_substring_patterns, url1));
+  EXPECT_FALSE(condition_set->IsMatch(matching_substring_patterns, url1));
 
   matching_substring_patterns.insert(m2.substring_pattern()->id());
-  EXPECT_TRUE(condition_set.IsMatch(matching_substring_patterns, url1));
-  EXPECT_FALSE(condition_set.IsMatch(matching_substring_patterns, url2));
+  EXPECT_TRUE(condition_set->IsMatch(matching_substring_patterns, url1));
+  EXPECT_FALSE(condition_set->IsMatch(matching_substring_patterns, url2));
 }
 
 
@@ -352,8 +343,9 @@ TEST(URLMatcherTest, FullTest) {
   conditions1.insert(factory->CreatePathContainsCondition("foo"));
 
   const int kConditionSetId1 = 1;
-  std::vector<URLMatcherConditionSet> insert1;
-  insert1.push_back(URLMatcherConditionSet(kConditionSetId1, conditions1));
+  URLMatcherConditionSet::Vector insert1;
+  insert1.push_back(make_scoped_refptr(
+      new URLMatcherConditionSet(kConditionSetId1, conditions1)));
   matcher.AddConditionSets(insert1);
   EXPECT_EQ(1u, matcher.MatchURL(url1).size());
   EXPECT_EQ(0u, matcher.MatchURL(url2).size());
@@ -363,8 +355,9 @@ TEST(URLMatcherTest, FullTest) {
   conditions2.insert(factory->CreateHostSuffixCondition("example.com"));
 
   const int kConditionSetId2 = 2;
-  std::vector<URLMatcherConditionSet> insert2;
-  insert2.push_back(URLMatcherConditionSet(kConditionSetId2, conditions2));
+  URLMatcherConditionSet::Vector insert2;
+  insert2.push_back(make_scoped_refptr(
+      new URLMatcherConditionSet(kConditionSetId2, conditions2)));
   matcher.AddConditionSets(insert2);
   EXPECT_EQ(2u, matcher.MatchURL(url1).size());
   EXPECT_EQ(1u, matcher.MatchURL(url2).size());
diff --git a/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.cc b/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.cc
index 609a109..04708c7 100644
--- a/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.cc
+++ b/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.cc
@@ -118,11 +118,12 @@ namespace keys = declarative_webrequest_constants;
 //
 
 WebRequestCondition::WebRequestCondition(
-    const URLMatcherConditionSet& url_matcher_conditions,
+    scoped_refptr<URLMatcherConditionSet> url_matcher_conditions,
     const WebRequestConditionAttributes& condition_attributes)
     : url_matcher_conditions_(url_matcher_conditions),
       condition_attributes_(condition_attributes),
       applicable_request_stages_(~0) {
+  CHECK(url_matcher_conditions.get());
   for (WebRequestConditionAttributes::const_iterator i =
        condition_attributes_.begin(); i != condition_attributes_.end(); ++i) {
     applicable_request_stages_ &= (*i)->GetStages();
@@ -214,8 +215,8 @@ scoped_ptr<WebRequestCondition> WebRequestCondition::Create(
         url_matcher_condition_factory->CreateHostPrefixCondition(""));
   }
 
-  URLMatcherConditionSet url_matcher_condition_set(++g_next_id,
-                                                   url_matcher_conditions);
+  scoped_refptr<URLMatcherConditionSet> url_matcher_condition_set(
+      new URLMatcherConditionSet(++g_next_id, url_matcher_conditions));
   return scoped_ptr<WebRequestCondition>(
       new WebRequestCondition(url_matcher_condition_set, attributes));
 }
@@ -272,7 +273,7 @@ bool WebRequestConditionSet::IsFulfilled(
 }
 
 void WebRequestConditionSet::GetURLMatcherConditionSets(
-    std::vector<URLMatcherConditionSet>* condition_sets) const {
+    URLMatcherConditionSet::Vector* condition_sets) const {
   for (Conditions::const_iterator i = conditions_.begin();
        i != conditions_.end(); ++i) {
     condition_sets->push_back((*i)->url_matcher_condition_set());
diff --git a/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.h b/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.h
index e5e8b58..ed9268b 100644
--- a/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.h
+++ b/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.h
@@ -36,7 +36,7 @@ namespace extensions {
 class WebRequestCondition {
  public:
   WebRequestCondition(
-      const URLMatcherConditionSet& url_matcher_conditions,
+      scoped_refptr<URLMatcherConditionSet> url_matcher_conditions,
       const WebRequestConditionAttributes& condition_attributes);
   ~WebRequestCondition();
 
@@ -56,14 +56,14 @@ class WebRequestCondition {
   // This ID is registered in a URLMatcher that can inform us in case of a
   // match.
   URLMatcherConditionSet::ID url_matcher_condition_set_id() const {
-    return url_matcher_conditions_.id();
+    return url_matcher_conditions_->id();
   }
 
   // Returns the set of conditions that are checked on the URL. This is the
   // primary trigger for WebRequestCondition and therefore never empty.
   // (If it was empty, the URLMatcher would never notify us about network
   // requests which might fulfill the entire WebRequestCondition).
-  const URLMatcherConditionSet& url_matcher_condition_set() const {
+  scoped_refptr<URLMatcherConditionSet> url_matcher_condition_set() const {
     return url_matcher_conditions_;
   }
 
@@ -80,7 +80,7 @@ class WebRequestCondition {
       const base::Value* value,
       std::string* error);
 
-  URLMatcherConditionSet url_matcher_conditions_;
+  scoped_refptr<URLMatcherConditionSet> url_matcher_conditions_;
   WebRequestConditionAttributes condition_attributes_;
 
   // Bit vector indicating all RequestStages during which all
@@ -124,7 +124,7 @@ class WebRequestConditionSet {
 
   // Appends the URLMatcherConditionSet from all conditions to |condition_sets|.
   void GetURLMatcherConditionSets(
-      std::vector<URLMatcherConditionSet>* condition_sets) const;
+      URLMatcherConditionSet::Vector* condition_sets) const;
 
  private:
   typedef std::vector<linked_ptr<WebRequestCondition> > Conditions;
diff --git a/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition_unittest.cc b/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition_unittest.cc
index ca49c39..8499e34 100644
--- a/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition_unittest.cc
+++ b/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition_unittest.cc
@@ -107,7 +107,7 @@ TEST(WebRequestConditionTest, CreateConditionSet) {
   EXPECT_EQ(2u, result->conditions().size());
 
   // Tell the URLMatcher about our shiny new patterns.
-  std::vector<URLMatcherConditionSet> url_matcher_condition_set;
+  URLMatcherConditionSet::Vector url_matcher_condition_set;
   result->GetURLMatcherConditionSets(&url_matcher_condition_set);
   matcher.AddConditionSets(url_matcher_condition_set);
 
diff --git a/chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry.cc b/chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry.cc
index 8f4c69c..256e0a0 100644
--- a/chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry.cc
+++ b/chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry.cc
@@ -97,17 +97,17 @@ std::string WebRequestRulesRegistry::AddRulesImpl(
   // Create the triggers.
   for (RulesMap::iterator i = new_webrequest_rules.begin();
        i != new_webrequest_rules.end(); ++i) {
-    std::vector<URLMatcherConditionSet> url_condition_sets;
+    URLMatcherConditionSet::Vector url_condition_sets;
     const WebRequestConditionSet& conditions = i->second->conditions();
     conditions.GetURLMatcherConditionSets(&url_condition_sets);
-    for (std::vector<URLMatcherConditionSet>::iterator j =
-        url_condition_sets.begin(); j != url_condition_sets.end(); ++j) {
-      rule_triggers_[j->id()] = i->second.get();
+    for (URLMatcherConditionSet::Vector::iterator j =
+         url_condition_sets.begin(); j != url_condition_sets.end(); ++j) {
+      rule_triggers_[(*j)->id()] = i->second.get();
     }
   }
 
   // Register url patterns in url_matcher_.
-  std::vector<URLMatcherConditionSet> all_new_condition_sets;
+  URLMatcherConditionSet::Vector all_new_condition_sets;
   for (RulesMap::iterator i = new_webrequest_rules.begin();
        i != new_webrequest_rules.end(); ++i) {
     i->second->conditions().GetURLMatcherConditionSets(&all_new_condition_sets);
@@ -133,13 +133,13 @@ std::string WebRequestRulesRegistry::RemoveRulesImpl(
       continue;
 
     // Remove all triggers but collect their IDs.
-    std::vector<URLMatcherConditionSet> condition_sets;
+    URLMatcherConditionSet::Vector condition_sets;
     WebRequestRule* rule = webrequest_rules_entry->second.get();
     rule->conditions().GetURLMatcherConditionSets(&condition_sets);
-    for (std::vector<URLMatcherConditionSet>::iterator j =
-         condition_sets.begin(); j != condition_sets.end(); ++j) {
-      remove_from_url_matcher.push_back(j->id());
-      rule_triggers_.erase(j->id());
+    for (URLMatcherConditionSet::Vector::iterator j = condition_sets.begin();
+         j != condition_sets.end(); ++j) {
+      remove_from_url_matcher.push_back((*j)->id());
+      rule_triggers_.erase((*j)->id());
     }
 
     // Remove reference to actual rule.