Origin Identifier Value Map.

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 {
+typedef content_settings::OriginIdentifierValueMap::EntryList::const_iterator

[Bernhard Bauer, 2011/05/19 15:05:17]

The typedef doesn't need to be in an anoymous namespace, as it doesn't create a
symbol that could cause linking errors.

[markusheintz_, 2011/05/23 19:55:11]

moved it back to the content_settings namespace.

+    ConstEntryIterator;
+
+typedef content_settings::OriginIdentifierValueMap::EntryList::iterator
+    EntryIterator;
+}
+
+namespace content_settings {
+
+ConstEntryIterator OriginIdentifierValueMap::HighestPrecedence(
+    ConstEntryIterator first,
+    ConstEntryIterator second) const {
+  // Compare requesting patterns
+  if (first->a.Compare(second->a) == ContentSettingsPattern::PREDECESSOR)

[jochen (gone - plz use gerrit), 2011/05/19 14:37:11]

comments should always end in dot

[Bernhard Bauer, 2011/05/19 15:05:17]

Pull out the comparison result into a variable and then check whether it's
PREDECESSOR or SUCCESSOR?

[markusheintz_, 2011/05/23 19:55:11]

Done.

[markusheintz_, 2011/05/23 19:55:11]

Actually I'd like to add "<" and ">" operator to the content settings pattern
class. Then it would look like:

first->a > second->a

would that be fine with you?

[Bernhard Bauer, 2011/05/24 14:21:29]

Yes, if you inline it below (so it just becomes:)

if (best_match == entries.end() || best_match < entry)
  best_match = entry;

[markusheintz_, 2011/05/26 13:22:13]

best_match < entry requires to add a < operator for the
OriginIdentifierValueMap::Entry(tuple). So I could do the following:


namespace {
bool operator<(OriginIdentifierValueMap::const_iterator first,
               OriginIdentifierValueMap::const_iterator second) {
...
}
}

That would replace the GetEntryWithHighest....

[Bernhard Bauer, 2011/05/26 23:14:42]

Yes. Ideally you could use ConstEntryIterator to make the signature less ugly.

[markusheintz_, 2011/05/31 11:46:41]

I used Entry instead of the ConstEntryIterator (does not exist anymore I renamed
it to const_iterator).

+    return first;
+  if (second->a.Compare(first->a) == ContentSettingsPattern::PREDECESSOR)
+    return second;
+  DCHECK(first->a == second->a);
+
+  // Compare embedding patterns
+  if (first->a.Compare(second->a) == ContentSettingsPattern::PREDECESSOR)
+    return first;
+  DCHECK(first->b.Compare(second->b) == ContentSettingsPattern::PREDECESSOR);
+  return second;
+}
+
+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.
+  ConstEntryIterator best_match = entries_.end();
+  for (ConstEntryIterator entry = entries_.begin();
+       entry != entries_.end();
+       ++entry) {
+    if (entry->a.Matches(item_url) &&
+        entry->b.Matches(top_level_frame_url) &&
+        entry->c == content_type &&
+        entry->d == resource_identifier) {
+      if (best_match == entries_.end()) {
+        best_match = entry;
+      } else {
+        best_match = HighestPrecedence(best_match, entry);

[Bernhard Bauer, 2011/05/19 15:05:17]

Do you have plans to use that method somewhere else? Otherwise you can just
inline it (getting rid of the naming problem as well :-D )

[markusheintz_, 2011/05/23 19:55:11]

No I don't have plans to use it anywhere else. I just put it into it's own
method to improve readability. Found it more readable this way. But I don't
object in-lining. So I can inline if you like.

+      }
+    }
+  }
+  if (best_match != entries_.end())
+    return best_match->e->DeepCopy();

[Bernhard Bauer, 2011/05/19 15:05:17]

As discussed offline, it's probably preferable to just return the value.

[markusheintz_, 2011/05/23 19:55:11]

Done.

+  return Value::CreateNullValue();

[Bernhard Bauer, 2011/05/19 15:05:17]

return an actual NULL?

[markusheintz_, 2011/05/23 19:55:11]

Cool I returned a NULL in first place anyway. Done

+}
+
+void OriginIdentifierValueMap::SetValue(
+    const ContentSettingsPattern& item_pattern,
+    const ContentSettingsPattern& top_level_frame_pattern,
+    ContentSettingsType content_type,
+    const ResourceIdentifier& resource_identifier,
+    Value* value) {
+  EntryIterator 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.
+    list_entry = entries_.insert(list_entry, MakeTuple(item_pattern,
+                                                       top_level_frame_pattern,
+                                                       content_type,
+                                                       resource_identifier,
+                                                       value));
+  }
+
+  // Update or entry

[Bernhard Bauer, 2011/05/19 15:05:17]

Nit: "our" entry? Also, you can move this into the else clause.

[markusheintz_, 2011/05/23 19:55:11]

Done. Done.

+  list_entry->e = value;
+}
+
+void OriginIdentifierValueMap::DeleteValue(
+      const ContentSettingsPattern& item_pattern,
+      const ContentSettingsPattern& top_level_frame_pattern,
+      ContentSettingsType content_type,
+      const ResourceIdentifier& resource_identifier) {
+  EntryIterator entry_to_delete = FindEntry(item_pattern,
+                                            top_level_frame_pattern,
+                                            content_type,
+                                            resource_identifier);
+  if (entry_to_delete != entries_.end()) {
+    delete entry_to_delete->e;
+    entries_.erase(entry_to_delete);
+  }
+}
+
+EntryIterator OriginIdentifierValueMap::FindEntry(
+      const ContentSettingsPattern& item_pattern,
+      const ContentSettingsPattern& top_level_frame_pattern,
+      ContentSettingsType content_type,
+      const ResourceIdentifier& resource_identifier) {
+  EntryIterator list_entry(entries_.end());
+  for (EntryList::iterator entry = entries_.begin();
+       entry != entries_.end();
+       ++entry) {
+    if (item_pattern == entry->a &&
+        top_level_frame_pattern == entry->b &&
+        content_type == entry->c &&
+        resource_identifier == entry->d) {
+      list_entry = entry;

[Bernhard Bauer, 2011/05/19 15:05:17]

Directly |return entry|?

[markusheintz_, 2011/05/23 19:55:11]

Done.

+      break;
+    }
+  }
+  return list_entry;
+}
+
+

[jochen (gone - plz use gerrit), 2011/05/19 14:37:11]

only one empty line

[markusheintz_, 2011/05/23 19:55:11]

Done.

+}  // namespace content_settings