RulesRegistry for declarative APIs.

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

+// Copyright (c) 2012 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/extensions/api/declarative/test_rules_registry.h"
+
+#include <algorithm>
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/stl_util.h"
+#include "chrome/browser/extensions/api/declarative/declarative_api_constants.h"
+
+namespace keys = extension_declarative_api_constants;
+
+namespace {
+
+std::string GetRuleId(DictionaryValue* rule) {
+  std::string rule_id;
+  CHECK(rule->GetString(keys::kId, &rule_id));
+  return rule_id;
+}
+
+void PrintErrorsToLog(const std::string& error) {
+  LOG(ERROR) << error;
+}
+
+}  // namespace
+
+namespace extensions {
+
+TestRulesRegistry::TestRulesRegistry() {
+}
+
+TestRulesRegistry::~TestRulesRegistry() {
+  STLDeleteValues(&rules_);
+}
+
+bool TestRulesRegistry::AddRules(
+    const std::string& extension_id,
+    const std::vector<DictionaryValue*>& rules,
+    SetErrorCallback set_error_callback) {
+  std::vector<DictionaryValue*>::const_iterator i;
+  for (i = rules.begin(); i != rules.end(); ++i) {

[not at google - send to devlin, 2012/02/02 11:59:16]

I prefer to always define iterators inline, and wrap however necessary to make
that work.

[battre, 2012/02/02 19:12:36]

Do you feel strongly about this? I dislike the for statements that stretch over
3 lines.

[not at google - send to devlin, 2012/02/03 13:06:20]

I generally don't feel strongly enough about style things to worry about talking
about it.

However, I dislike situations where you have multiple iterators in the one
function, e.g.

foo() {
  iterator i;
  for (i = bar.begin(); i < bar.end(); i++)
    ...
  iterator j;
  for (j = bar.begin(); j < bar.end(); j++)
    ...
}

I've had those situations before.  Basically, I'm just in the habit now of
always limiting iterator scope to the for loops regardless of... anything.

By the way, I don't think it would stretch over 3 lines:

  for (std::vector<DictionaryValue*>::const_iterator i =
      rules.begin(); i < rules.end(); ++i) {
    ...
  }
}

[battre, 2012/02/06 16:21:04]

Done.

+    std::string rule_id = GetRuleId(*i);
+    // TODO: relax this (check first and abort with returning false).
+    CHECK(rules_.find(rule_id) == rules_.end());

[not at google - send to devlin, 2012/02/02 11:59:16]

Perhaps CHECK(rules_.count(rule_id)) is more concise.

[battre, 2012/02/02 19:12:36]

But it would be wrong ;-)
CHECK(!rules_.count(rule_id))
or
CHECK(rules_.count(rule_id) == 0)
would be correct.

I like the .find() == .end() because it is pretty literal and hard to get wrong.

+    rules_[rule_id] = (*i)->DeepCopy();
+  }
+  return true;
+}
+
+bool TestRulesRegistry::RemoveRules(
+    const std::string& extension_id,
+    const std::vector<std::string>& rule_identifiers,
+    SetErrorCallback set_error_callback) {
+  if (rule_identifiers.empty()) {
+    STLDeleteValues(&rules_);
+  } else {
+    std::vector<std::string>::const_iterator i;
+    for (i = rule_identifiers.begin(); i != rule_identifiers.end(); ++i) {
+      std::map<std::string, DictionaryValue*>::iterator entry = rules_.find(*i);
+      // TODO: relax this (check first and abort with returning false).
+      CHECK(entry != rules_.end());
+      delete entry->second;
+      rules_.erase(entry);
+    }
+  }
+  return true;
+}
+
+void TestRulesRegistry::GetRules(
+    const std::string& extension_id,
+    const std::vector<std::string>& rule_identifiers,
+    std::vector<DictionaryValue*>* out) {
+  if (rule_identifiers.empty()) {
+    std::map<std::string, DictionaryValue*>::const_iterator i;
+    for (i = rules_.begin(); i != rules_.end(); ++i)
+      out->push_back(i->second->DeepCopy());
+  } else {
+    std::vector<std::string>::const_iterator i;
+    for (i = rule_identifiers.begin(); i != rule_identifiers.end(); ++i) {
+      std::map<std::string, DictionaryValue*>::iterator entry = rules_.find(*i);
+      // TODO: relax this (check first and abort with returning false).
+      CHECK(entry != rules_.end());
+      out->push_back(entry->second->DeepCopy());
+    }
+  }
+}
+
+void TestRulesRegistry::OnExtensionUnloaded(const std::string& extension_id) {
+  std::vector<std::string> no_rule_identifiers;
+  RemoveRules(extension_id, no_rule_identifiers, base::Bind(&PrintErrorsToLog));
+}
+
+}  // namespace extensions