In memory map to store |Value| objects for

(origin pattern, origin pattern, content type, resource) identifier) tuples.

This is supposed to replace the content_settings::BaseProvider.

BUG=63656
TEST=origin_identifier_value_map_unittest.cc,
     content_settings_pref_provider_unittest.cc,
     host_content_settings_map_unittest.cc

Review URL: http://codereview.chromium.org/7049007

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

chrome/browser/content_settings/content_settings_origin_identifier_value_map.cc

+// Copyright (c) 2011 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.
+
+#include "chrome/browser/content_settings/content_settings_origin_identifier_value_map.h"
+
+#include "base/logging.h"
+#include "base/values.h"
+#include "googleurl/src/gurl.h"
+
+namespace content_settings {
+
+OriginIdentifierValueMap::OriginIdentifierValueMap() {}
+
+OriginIdentifierValueMap::~OriginIdentifierValueMap() {
+  Clear();
+}
+
+bool operator>(const OriginIdentifierValueMap::Entry& first,
+               const OriginIdentifierValueMap::Entry& second) {
+  // Compare item patterns.
+  if (first.item_pattern > second.item_pattern)
+    return true;
+  if (first.item_pattern < second.item_pattern)
+    return false;
+
+  // Compare top_level_frame patterns.
+  if (first.top_level_frame_pattern > second.top_level_frame_pattern)
+    return true;
+  return false;
+}
+
+Value* OriginIdentifierValueMap::GetValue(
+    const GURL& item_url,
+    const GURL& top_level_frame_url,
+    ContentSettingsType content_type,
+    const ResourceIdentifier& resource_identifier) const {
+  // Find best matching list entry.
+  OriginIdentifierValueMap::const_iterator best_match = entries_.end();
+  for (OriginIdentifierValueMap::const_iterator entry = entries_.begin();
+       entry != entries_.end();
+       ++entry) {
+    if (entry->item_pattern.Matches(item_url) &&
+        entry->top_level_frame_pattern.Matches(top_level_frame_url) &&
+        entry->content_type == content_type &&
+        entry->identifier == resource_identifier) {
+      if (best_match == entries_.end() || *entry > *best_match) {
+        best_match = entry;
+      }
+    }
+  }
+  if (best_match != entries_.end())
+    return best_match->value.get();
+  return NULL;
+}
+
+void OriginIdentifierValueMap::SetValue(
+    const ContentSettingsPattern& item_pattern,
+    const ContentSettingsPattern& top_level_frame_pattern,
+    ContentSettingsType content_type,
+    const ResourceIdentifier& resource_identifier,
+    Value* value) {
+  OriginIdentifierValueMap::iterator list_entry =
+      FindEntry(item_pattern,
+                top_level_frame_pattern,
+                content_type,
+                resource_identifier);
+  if (list_entry == entries_.end()) {
+    // No matching list entry found. Add a new entry to the list.
+    entries_.insert(list_entry, Entry(item_pattern,
+                                      top_level_frame_pattern,
+                                      content_type,
+                                      resource_identifier,
+                                      value));
+  } else {
+    // Update the list entry.
+    list_entry->value.reset(value);
+  }
+}
+
+void OriginIdentifierValueMap::DeleteValue(
+      const ContentSettingsPattern& item_pattern,
+      const ContentSettingsPattern& top_level_frame_pattern,
+      ContentSettingsType content_type,
+      const ResourceIdentifier& resource_identifier) {
+  OriginIdentifierValueMap::iterator entry_to_delete =
+      FindEntry(item_pattern,
+                top_level_frame_pattern,
+                content_type,
+                resource_identifier);
+  if (entry_to_delete != entries_.end()) {
+    entries_.erase(entry_to_delete);
+  }
+}
+
+void OriginIdentifierValueMap::Clear() {
+  // Delete all owned value objects.
+  /*
+  for (OriginIdentifierValueMap::iterator entry = entries_.begin();
+       entry != entries_.end();
+       ++entry) {
+    delete entry->value;
+  }
+  */
+  entries_.clear();
+}
+
+OriginIdentifierValueMap::iterator OriginIdentifierValueMap::DeleteValue(
+    OriginIdentifierValueMap::iterator entry) {
+  return entries_.erase(entry);
+}
+
+OriginIdentifierValueMap::iterator OriginIdentifierValueMap::FindEntry(
+      const ContentSettingsPattern& item_pattern,
+      const ContentSettingsPattern& top_level_frame_pattern,
+      ContentSettingsType content_type,
+      const ResourceIdentifier& resource_identifier) {
+  for (OriginIdentifierValueMap::iterator entry = entries_.begin();
+       entry != entries_.end();
+       ++entry) {
+    if (item_pattern == entry->item_pattern &&
+        top_level_frame_pattern == entry->top_level_frame_pattern &&
+        content_type == entry->content_type &&
+        resource_identifier == entry->identifier) {
+      return entry;
+    }
+  }
+  return entries_.end();
+}
+
+}  // namespace content_settings

chrome/browser/content_settings/content_settings_origin_identifier_value_map.h

+// Copyright (c) 2011 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.
+
+#ifndef CHROME_BROWSER_CONTENT_SETTINGS_CONTENT_SETTINGS_ORIGIN_IDENTIFIER_VALUE_MAP_H_
+#define CHROME_BROWSER_CONTENT_SETTINGS_CONTENT_SETTINGS_ORIGIN_IDENTIFIER_VALUE_MAP_H_
+
+#include <list>
+#include <map>
+#include <string>
+
+#include "base/tuple.h"
+#include "base/memory/linked_ptr.h"
+#include "chrome/browser/content_settings/content_settings_pattern.h"
+#include "chrome/common/content_settings_types.h"
+
+class GURL;
+class Value;
+
+namespace content_settings {
+
+class OriginIdentifierValueMap {
+ public:
+  typedef std::string ResourceIdentifier;
+
+  struct Entry {
+    Entry(ContentSettingsPattern item_pattern,
+          ContentSettingsPattern top_level_frame_pattern,
+          ContentSettingsType content_type,
+          ResourceIdentifier identifier,
+          Value* value)
+          : item_pattern(item_pattern),
+            top_level_frame_pattern(top_level_frame_pattern),
+            content_type(content_type),
+            identifier(identifier),
+            value(value) {
+    }
+
+    ContentSettingsPattern item_pattern;
+    ContentSettingsPattern top_level_frame_pattern;
+    ContentSettingsType content_type;
+    ResourceIdentifier identifier;
+    linked_ptr<Value> value;
+  };
+
+  typedef std::list<Entry> EntryList;
+
+  typedef EntryList::const_iterator const_iterator;
+
+  typedef EntryList::iterator iterator;
+
+  EntryList::iterator begin() {
+    return entries_.begin();
+  }
+
+  EntryList::iterator end() {
+    return entries_.end();
+  }
+
+  EntryList::const_iterator begin() const {
+    return entries_.begin();
+  }
+
+  EntryList::const_iterator end() const {
+    return entries_.end();
+  }
+
+  size_t size() const {
+    return entries_.size();
+  }
+
+  OriginIdentifierValueMap();
+  ~OriginIdentifierValueMap();
+
+  // Returns a weak pointer to the value for the given |item_pattern|,
+  // |top_level_frame_pattern|, |content_type|, |resource_identifier| tuple. If
+  // no value is stored for the passed parameter |NULL| is returned.
+  Value* GetValue(
+      const GURL& item_url,
+      const GURL& top_level_frame_url,
+      ContentSettingsType content_type,
+      const ResourceIdentifier& resource_identifier) const;
+
+  // Sets the |value| for the given |item_pattern|, |top_level_frame_pattern|,
+  // |content_type|, |resource_identifier| tuple. The method takes the ownership
+  // of the passed |value|.
+  void SetValue(
+      const ContentSettingsPattern& item_pattern,
+      const ContentSettingsPattern& top_level_frame_pattern,
+      ContentSettingsType content_type,
+      const ResourceIdentifier& resource_identifier,
+      Value* value);
+
+  // Deletes the map entry for the given |item_pattern|,
+  // |top_level_frame_pattern|, |content_type|, |resource_identifier| tuple.
+  void DeleteValue(
+      const ContentSettingsPattern& item_pattern,
+      const ContentSettingsPattern& top_level_frame_pattern,
+      ContentSettingsType content_type,
+      const ResourceIdentifier& resource_identifier);
+
+  // Deletes the map entry at the passed position. The method returns the
+  // position of the next entry in the map.
+  EntryList::iterator DeleteValue(EntryList::iterator entry);
+
+  // Clears all map entries.
+  void Clear();
+
+ private:
+  // Finds the list entry for the given |item_pattern|,
+  // |top_level_frame_pattern|, |content_type|, |resource_identifier| tuple and
+  // returns the iterator of the list entry. If no entry is found for the passed
+  // parameters then the end of list iterator is returned.
+  EntryList::iterator FindEntry(
+      const ContentSettingsPattern& item_pattern,
+      const ContentSettingsPattern& top_level_frame_pattern,
+      ContentSettingsType content_type,
+      const ResourceIdentifier& resource_identifier);
+
+  EntryList entries_;
+
+  DISALLOW_COPY_AND_ASSIGN(OriginIdentifierValueMap);
+};
+
+// Compares two origin value map entries and tests if the item, top-level-frame
+// pattern pair of the first entry has a higher precedences then the pattern
+// pair of the second entry.
+bool operator>(const OriginIdentifierValueMap::Entry& first,
+               const OriginIdentifierValueMap::Entry& second);
+
+}  // namespace content_settings
+
+#endif  // CHROME_BROWSER_CONTENT_SETTINGS_CONTENT_SETTINGS_ORIGIN_IDENTIFIER_VALUE_MAP_H_

chrome/browser/content_settings/content_settings_origin_identifier_value_map_unittest.cc

+// Copyright (c) 2011 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.
+
+#include "chrome/browser/content_settings/content_settings_origin_identifier_value_map.h"
+
+#include "base/scoped_ptr.h"
+#include "base/values.h"
+#include "googleurl/src/gurl.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(OriginIdentifierValueMapTest, SetGetValue) {
+  content_settings::OriginIdentifierValueMap map;
+
+  EXPECT_EQ(NULL, map.GetValue(GURL("http://www.google.com"),
+                               GURL("http://www.google.com"),
+                               CONTENT_SETTINGS_TYPE_COOKIES,
+                               ""));
+  map.SetValue(
+      ContentSettingsPattern::FromString("[*.]google.com"),
+      ContentSettingsPattern::FromString("[*.]google.com"),
+      CONTENT_SETTINGS_TYPE_COOKIES,
+      "",
+      Value::CreateIntegerValue(1));
+
+  scoped_ptr<Value> expected_value(Value::CreateIntegerValue(1));
+  EXPECT_TRUE(expected_value->Equals(
+      map.GetValue(GURL("http://www.google.com"),
+                   GURL("http://www.google.com"),
+                   CONTENT_SETTINGS_TYPE_COOKIES,
+                   "")));
+
+  EXPECT_EQ(NULL, map.GetValue(GURL("http://www.google.com"),
+                               GURL("http://www.youtube.com"),
+                               CONTENT_SETTINGS_TYPE_COOKIES,
+                               ""));
+
+  EXPECT_EQ(NULL, map.GetValue(GURL("http://www.youtube.com"),
+                               GURL("http://www.google.com"),
+                               CONTENT_SETTINGS_TYPE_COOKIES,
+                               ""));
+
+  EXPECT_EQ(NULL, map.GetValue(GURL("http://www.google.com"),
+                               GURL("http://www.google.com"),
+                               CONTENT_SETTINGS_TYPE_POPUPS,
+                               ""));
+
+  EXPECT_EQ(NULL, map.GetValue(GURL("http://www.google.com"),
+                               GURL("http://www.google.com"),
+                               CONTENT_SETTINGS_TYPE_COOKIES,
+                               "resource_id"));
+}
+
+TEST(OriginIdentifierValueMapTest, SetDeleteValue) {
+  content_settings::OriginIdentifierValueMap map;
+
+  EXPECT_EQ(NULL, map.GetValue(GURL("http://www.google.com"),
+                               GURL("http://www.google.com"),
+                               CONTENT_SETTINGS_TYPE_PLUGINS,
+                               "java-plugin"));
+
+  // Set sample values.
+  map.SetValue(
+      ContentSettingsPattern::FromString("[*.]google.com"),
+      ContentSettingsPattern::FromString("[*.]google.com"),
+      CONTENT_SETTINGS_TYPE_PLUGINS,
+      "java-plugin",
+      Value::CreateIntegerValue(1));
+
+  int actual_value;
+  EXPECT_TRUE(map.GetValue(GURL("http://www.google.com"),
+                           GURL("http://www.google.com"),
+                           CONTENT_SETTINGS_TYPE_PLUGINS,
+                           "java-plugin")->GetAsInteger(&actual_value));
+  EXPECT_EQ(1, actual_value);
+  EXPECT_EQ(NULL, map.GetValue(GURL("http://www.google.com"),
+                               GURL("http://www.google.com"),
+                               CONTENT_SETTINGS_TYPE_PLUGINS,
+                               "flash-plugin"));
+  // Delete non-existing value.
+  map.DeleteValue(
+      ContentSettingsPattern::FromString("[*.]google.com"),
+      ContentSettingsPattern::FromString("[*.]google.com"),
+      CONTENT_SETTINGS_TYPE_PLUGINS,
+      "flash-plugin");
+  EXPECT_EQ(NULL, map.GetValue(GURL("http://www.google.com"),
+                               GURL("http://www.google.com"),
+                               CONTENT_SETTINGS_TYPE_PLUGINS,
+                               "flash-plugin"));
+  EXPECT_TRUE(map.GetValue(GURL("http://www.google.com"),
+                           GURL("http://www.google.com"),
+                           CONTENT_SETTINGS_TYPE_PLUGINS,
+                           "java-plugin")->GetAsInteger(&actual_value));
+  EXPECT_EQ(1, actual_value);
+
+  // Delete existing value.
+  map.DeleteValue(
+      ContentSettingsPattern::FromString("[*.]google.com"),
+      ContentSettingsPattern::FromString("[*.]google.com"),
+      CONTENT_SETTINGS_TYPE_PLUGINS,
+      "java-plugin");
+
+  EXPECT_EQ(NULL, map.GetValue(GURL("http://www.google.com"),
+                               GURL("http://www.google.com"),
+                               CONTENT_SETTINGS_TYPE_PLUGINS,
+                               "java-plugin"));
+}
+
+TEST(OriginIdentifierValueMapTest, Clear) {
+  content_settings::OriginIdentifierValueMap map;
+  EXPECT_EQ(map.begin(), map.end());
+
+  // Set two values.
+  map.SetValue(
+      ContentSettingsPattern::FromString("[*.]google.com"),
+      ContentSettingsPattern::FromString("[*.]google.com"),
+      CONTENT_SETTINGS_TYPE_PLUGINS,
+      "java-plugin",
+      Value::CreateIntegerValue(1));
+  map.SetValue(
+      ContentSettingsPattern::FromString("[*.]google.com"),
+      ContentSettingsPattern::FromString("[*.]google.com"),
+      CONTENT_SETTINGS_TYPE_COOKIES,
+      "",
+      Value::CreateIntegerValue(1));
+  EXPECT_NE(map.begin(), map.end());
+  int actual_value;
+  EXPECT_TRUE(map.GetValue(GURL("http://www.google.com"),
+                           GURL("http://www.google.com"),
+                           CONTENT_SETTINGS_TYPE_PLUGINS,
+                           "java-plugin")->GetAsInteger(&actual_value));
+  EXPECT_EQ(1, actual_value);
+
+  // Clear the map.
+  map.Clear();
+  EXPECT_EQ(map.begin(), map.end());
+  EXPECT_EQ(NULL, map.GetValue(GURL("http://www.google.com"),
+                               GURL("http://www.google.com"),
+                               CONTENT_SETTINGS_TYPE_PLUGINS,
+                               "java-plugin"));
+}
+
+TEST(OriginIdentifierValueMapTest, ListEntryPrecedences) {
+  content_settings::OriginIdentifierValueMap map;
+
+  map.SetValue(
+      ContentSettingsPattern::FromString("[*.]google.com"),
+      ContentSettingsPattern::FromString("[*.]google.com"),
+      CONTENT_SETTINGS_TYPE_COOKIES,
+      "",
+      Value::CreateIntegerValue(1));
+
+  map.SetValue(
+      ContentSettingsPattern::FromString("www.google.com"),
+      ContentSettingsPattern::FromString("[*.]google.com"),
+      CONTENT_SETTINGS_TYPE_COOKIES,
+      "",
+      Value::CreateIntegerValue(2));
+
+  int actual_value;
+  EXPECT_TRUE(map.GetValue(GURL("http://mail.google.com"),
+                           GURL("http://www.google.com"),
+                           CONTENT_SETTINGS_TYPE_COOKIES,
+                           "")->GetAsInteger(&actual_value));
+  EXPECT_EQ(1, actual_value);
+
+  EXPECT_TRUE(map.GetValue(GURL("http://www.google.com"),
+                           GURL("http://www.google.com"),
+                           CONTENT_SETTINGS_TYPE_COOKIES,
+                           "")->GetAsInteger(&actual_value));
+  EXPECT_EQ(2, actual_value);
+}

chrome/browser/content_settings/content_settings_pattern.cc

@@ -309,35 +309,6 @@ ContentSettingsPattern ContentSettingsPattern::FromURL(
   return builder->Build();
 }
 
-// static
-ContentSettingsPattern ContentSettingsPattern::LegacyFromURL(
-    const GURL& url) {
-  scoped_ptr<ContentSettingsPattern::BuilderInterface> builder(
-      ContentSettingsPattern::CreateBuilder(true));
-
-  if (url.SchemeIsFile()) {
-    builder->WithScheme(url.scheme())->WithPath(url.path());
-  } else {
-    // Please keep the order of the ifs below as URLs with an IP as host can
-    // also have a "http" scheme.
-    // TODO(markusheintz): For HTTPS we will create scheme specific patterns as
-    // soon as the pattern matching code in the HostContentSettingsMap is
-    // replaced.
-    if (url.HostIsIPAddress()) {
-      builder->WithSchemeWildcard()->WithHost(url.host());
-    } else if (url.SchemeIs(chrome::kHttpScheme)) {
-      builder->WithSchemeWildcard()->WithDomainWildcard()->WithHost(url.host());
-    } else if (url.SchemeIs(chrome::kHttpsScheme)) {
-      builder->WithSchemeWildcard()->WithDomainWildcard()->WithHost(
-          url.host());
-    } else {
-      // Unsupported scheme
-    }
-    builder->WithPortWildcard();
-  }
-  return builder->Build();
-}
-
 // static
 ContentSettingsPattern ContentSettingsPattern::FromURLNoWildcard(
     const GURL& url) {
@@ -357,21 +328,6 @@ ContentSettingsPattern ContentSettingsPattern::FromURLNoWildcard(
   return builder->Build();
 }
 
-// static
-ContentSettingsPattern ContentSettingsPattern::LegacyFromURLNoWildcard(
-    const GURL& url) {
-  scoped_ptr<ContentSettingsPattern::BuilderInterface> builder(
-      ContentSettingsPattern::CreateBuilder(true));
-
-  if (url.SchemeIsFile()) {
-    builder->WithScheme(url.scheme())->WithPath(url.path());
-  } else {
-    builder->WithSchemeWildcard()->WithHost(url.host());
-  }
-  builder->WithPortWildcard();
-  return builder->Build();
-}
-
 // static
 ContentSettingsPattern ContentSettingsPattern::FromString(
     const std::string& pattern_spec) {

chrome/browser/content_settings/content_settings_pattern.h

@@ -99,13 +99,9 @@ class ContentSettingsPattern {
   // all subdomains and ports.
   static ContentSettingsPattern FromURL(const GURL& url);
 
-  static ContentSettingsPattern LegacyFromURL(const GURL& url);
-
   // Returns a pattern that matches exactly this URL.
   static ContentSettingsPattern FromURLNoWildcard(const GURL& url);
 
-  static ContentSettingsPattern LegacyFromURLNoWildcard(const GURL& url);
-
   // Returns a pattern that matches the given pattern specification.
   // Valid patterns specifications are:
   //   - [*.]domain.tld (matches domain.tld and all sub-domains)

chrome/browser/content_settings/content_settings_pref_provider.h

@@ -14,8 +14,9 @@
 
 #include "base/basictypes.h"
 #include "base/synchronization/lock.h"
-#include "chrome/browser/content_settings/content_settings_base_provider.h"
+#include "chrome/browser/content_settings/content_settings_origin_identifier_value_map.h"
 #include "chrome/browser/content_settings/content_settings_provider.h"
+#include "chrome/browser/content_settings/content_settings_utils.h"
 #include "chrome/browser/prefs/pref_change_registrar.h"
 #include "content/common/notification_observer.h"
 #include "content/common/notification_registrar.h"
@@ -99,7 +100,7 @@ class PrefDefaultProvider : public DefaultProviderInterface,
 
 // Content settings provider that provides content settings from the user
 // preference.
-class PrefProvider : public BaseProvider,
+class PrefProvider : public ProviderInterface,
                      public NotificationObserver {
  public:
   static void RegisterUserPrefs(PrefService* prefs);
@@ -107,6 +108,7 @@ class PrefProvider : public BaseProvider,
   explicit PrefProvider(Profile* profile);
   virtual ~PrefProvider();
 
+  // ProviderInterface implementations.
   virtual void SetContentSetting(
       const ContentSettingsPattern& requesting_pattern,
       const ContentSettingsPattern& embedding_pattern,
@@ -114,20 +116,30 @@ class PrefProvider : public BaseProvider,
       const ResourceIdentifier& resource_identifier,
       ContentSetting content_setting);
 
+  virtual ContentSetting GetContentSetting(
+      const GURL& requesting_url,
+      const GURL& embedding_url,
+      ContentSettingsType content_type,
+      const ResourceIdentifier& resource_identifier) const;
+
+  virtual void GetAllContentSettingsRules(
+      ContentSettingsType content_type,
+      const ResourceIdentifier& resource_identifier,
+      Rules* content_setting_rules) const;
+
   virtual void ClearAllContentSettingsRules(
       ContentSettingsType content_type);
 
   virtual void ResetToDefaults();
 
-  // BaseProvider implementations.
-  virtual void Init();
-
   // NotificationObserver implementation.
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
  private:
+  void Init();
+
   void ReadExceptions(bool overwrite);
 
   // Various migration methods (old cookie, popup and per-host data gets
@@ -138,20 +150,14 @@ class PrefProvider : public BaseProvider,
   void CanonicalizeContentSettingsExceptions(
       DictionaryValue* all_settings_dictionary);
 
-  void GetSettingsFromDictionary(
-      const DictionaryValue* dictionary,
-      ContentSettings* settings);
-
-  void GetResourceSettingsFromDictionary(
-      const DictionaryValue* dictionary,
-      ResourceContentSettings* settings);
-
   void NotifyObservers(const ContentSettingsDetails& details);
 
   void UnregisterObservers();
 
   Profile* profile_;
 
+  bool is_incognito_;
+
   PrefChangeRegistrar pref_change_registrar_;
   NotificationRegistrar notification_registrar_;
 
@@ -162,6 +168,14 @@ class PrefProvider : public BaseProvider,
   // Do not fire any Notifications as long as we are in the constructor.
   bool initializing_;
 
+  OriginIdentifierValueMap value_map_;
+
+  OriginIdentifierValueMap incognito_value_map_;
+
+  // Used around accesses to the value map objects to guarantee
+  // thread safety.
+  mutable base::Lock lock_;
+
   DISALLOW_COPY_AND_ASSIGN(PrefProvider);
 };
 

chrome/browser/content_settings/host_content_settings_map.cc

@@ -260,34 +260,33 @@ void HostContentSettingsMap::GetSettingsForOneType(
     const std::string& resource_identifier,
     SettingsForOneType* settings) const {
   DCHECK(settings);
-  settings->clear();
-
   // Collect content_settings::Rules for the given content_type and
   // resource_identifier from the content settings providers.
-  Rules content_settings_rules;
+  std::map<std::string, PatternSettingPair>
+      pattern_str_pattern_setting_pair_map;
   for (ConstProviderIterator provider = content_settings_providers_.begin();
-       provider != content_settings_providers_.end();
-       ++provider) {
-    // TODO(markusheintz): Only the rules that are applied should be collected.
-    // Merge rules.
-    // TODO(markusheintz): GetAllContentSettingsRules should maybe not clear the
-    // passed vector in case rule sets are just unified.
+     provider != content_settings_providers_.end();
+     ++provider) {
     Rules rules;
     (*provider)->GetAllContentSettingsRules(
         content_type, resource_identifier, &rules);
-    content_settings_rules.insert(content_settings_rules.end(),
-                                  rules.begin(),
-                                  rules.end());
+    // TODO(markusheintz): Only the rules that are applied should be collected.
+    for (Rules::iterator rule = rules.begin();
+         rule != rules.end();
+         ++rule) {
+      const ContentSettingsPattern& pattern(rule->requesting_url_pattern);
+      pattern_str_pattern_setting_pair_map[pattern.ToString()] =
+          PatternSettingPair(pattern, rule->content_setting);
+    }
   }
 
-  // convert Rules to SettingsForOneType
-  for (const_rules_iterator rule_iterator =
-           content_settings_rules.begin();
-       rule_iterator != content_settings_rules.end();
-       ++rule_iterator) {
-    settings->push_back(std::make_pair(ContentSettingsPattern(
-        rule_iterator->requesting_url_pattern),
-        rule_iterator->content_setting));
+  settings->clear();
+  // Rely on the maps iterator to sort the rules.
+  for (std::map<std::string, PatternSettingPair>::iterator i(
+           pattern_str_pattern_setting_pair_map.begin());
+       i != pattern_str_pattern_setting_pair_map.end();
+       ++i) {
+    settings->push_back(i->second);
   }
 }
 
@@ -321,11 +320,11 @@ void HostContentSettingsMap::AddExceptionForURL(
     ContentSetting setting) {
   // Make sure there is no entry that would override the pattern we are about
   // to insert for exactly this URL.
-  SetContentSetting(ContentSettingsPattern::LegacyFromURLNoWildcard(url),
+  SetContentSetting(ContentSettingsPattern::FromURLNoWildcard(url),
                     content_type,
                     resource_identifier,
                     CONTENT_SETTING_DEFAULT);
-  SetContentSetting(ContentSettingsPattern::LegacyFromURL(url),
+  SetContentSetting(ContentSettingsPattern::FromURL(url),
                     content_type,
                     resource_identifier,
                     setting);

chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc

@@ -111,6 +111,10 @@ TEST_F(ContentSettingBubbleModelTest, MultiplePlugins) {
   HostContentSettingsMap* map = profile_->GetHostContentSettingsMap();
   std::string fooPlugin = "foo";
   std::string barPlugin = "bar";
+
+  // Navigating to some sample url prevents the GetURL method from returning an
+  // invalid empty URL.
+  contents()->NavigateAndCommit(GURL("http://www.example.com"));
   GURL url = contents()->GetURL();
   map->AddExceptionForURL(url,
                           CONTENT_SETTINGS_TYPE_PLUGINS,

chrome/browser/ui/webui/options/content_settings_handler.cc

@@ -662,7 +662,7 @@ void ContentSettingsHandler::RemoveException(const ListValue* args) {
     // got destroyed before we received this message.
     if (settings_map) {
       settings_map->SetContentSetting(
-          ContentSettingsPattern::LegacyFromString(pattern),
+          ContentSettingsPattern::FromString(pattern),
           ContentSettingsTypeFromGroupName(type_string),
           "",
           CONTENT_SETTING_DEFAULT);
@@ -697,7 +697,7 @@ void ContentSettingsHandler::SetException(const ListValue* args) {
   if (!settings_map)
     return;
 
-  settings_map->SetContentSetting(ContentSettingsPattern::LegacyFromString(
+  settings_map->SetContentSetting(ContentSettingsPattern::FromString(
                                       pattern),
                                   type,
                                   "",
@@ -715,7 +715,7 @@ void ContentSettingsHandler::CheckExceptionPatternValidity(
   CHECK(args->GetString(arg_i++, &pattern_string));
 
   ContentSettingsPattern pattern =
-      ContentSettingsPattern::LegacyFromString(pattern_string);
+      ContentSettingsPattern::FromString(pattern_string);
 
   scoped_ptr<Value> mode_value(Value::CreateStringValue(mode_string));
   scoped_ptr<Value> pattern_value(Value::CreateStringValue(pattern_string));

chrome/chrome_browser.gypi

@@ -761,6 +761,8 @@
         'browser/content_settings/content_settings_extension_provider.h',
         'browser/content_settings/content_settings_notification_provider.cc',
         'browser/content_settings/content_settings_notification_provider.h',
+        'browser/content_settings/content_settings_origin_identifier_value_map.cc',
+        'browser/content_settings/content_settings_origin_identifier_value.h',
         'browser/content_settings/content_settings_pattern.cc',
         'browser/content_settings/content_settings_pattern.h',
         'browser/content_settings/content_settings_pattern_parser.cc',

chrome/chrome_tests.gypi

@@ -1351,6 +1351,7 @@
         'browser/command_updater_unittest.cc',
         'browser/content_settings/content_settings_mock_provider.cc',
         'browser/content_settings/content_settings_mock_provider.h',
+        'browser/content_settings/content_settings_origin_identifier_value_map_unittest.cc',
         'browser/content_settings/content_settings_pattern_unittest.cc',
         'browser/content_settings/content_settings_pattern_parser_unittest.cc',
         'browser/content_settings/content_settings_policy_provider_unittest.cc',