Move pref names and default value into WebsiteSettingsInfo

components/content_settings/core/browser/website_settings_info.cc

Index: components/content_settings/core/browser/website_settings_info.cc
diff --git a/components/content_settings/core/browser/website_settings_info.cc b/components/content_settings/core/browser/website_settings_info.cc
index fa19d9cf561f24f28edbc6581a843d7ce4e92556..ec0594d6e7884ed5daec683e8328cd162760fedb 100644
--- a/components/content_settings/core/browser/website_settings_info.cc
+++ b/components/content_settings/core/browser/website_settings_info.cc
@@ -4,11 +4,41 @@
 
 #include "components/content_settings/core/browser/website_settings_info.h"
 
+#include "base/logging.h"
+#include "base/strings/string_util.h"
+#include "base/values.h"
+
+namespace {
+
+const char* kPrefPrefix = "profile.content_settings.exceptions.";
+const char* kDefaultPrefPrefix = "profile.default_content_setting_values.";
+
+std::string GetPrefName(const std::string& name, const char* prefix) {
+  std::string pref_name = name;
+  base::ReplaceChars(pref_name, "-", "_", &pref_name);
+  return std::string(prefix).append(pref_name);
+}
+
+}  // namespace
+
 namespace content_settings {
 
 WebsiteSettingsInfo::WebsiteSettingsInfo(ContentSettingsType type,
-                                         const std::string& name)
-    : type_(type), name_(name) {}
+                                         const std::string& name,
+                                         base::Value* initial_default_value)

[msramek, 2015/07/29 09:20:07]

It seems a bit unnecessary that we convert ContentSetting to base::Value in
RegisterPermission(), then back to int in this constructor. Then as a consumer,
I would convert that int back to ContentSetting which is more meaningful.

I understand that this is because you want to support arbitrary default values.
Wouldn't it then be better to store the default value as base::Value, and make
getters

base::Value* default_value()
ContentSetting default_permission()
base::DictionaryValue* default_dictionary()

where for each content type one of the latter two would fail (or return
CONTENT_SETTING_DEFAULT/nullptr) depending on if it is permissiony?

[raymes, 2015/07/30 05:25:03]

Done. Since it's only used in one place I haven't added the getter. Also I'd
prefer to try to keep content settings specific stuff (i.e. stuff that's related
to permission-like things) separate from this class if possible and instead
build an abstraction on top.

[msramek, 2015/07/31 15:27:49]

Abstraction on top for permission-like things also sounds good.

+    : type_(type),
+      name_(name),
+      pref_name_(GetPrefName(name, kPrefPrefix)),
+      default_value_pref_name_(GetPrefName(name, kDefaultPrefPrefix)),
+      initial_default_value_(0) {
+  // For legacy reasons the default value is currently restricted to be an int.
+  // TODO(raymes): We should migrate the underlying pref to be a dictionary
+  // rather than an int.
+  if (initial_default_value) {
+    bool success = initial_default_value->GetAsInteger(&initial_default_value_);
+    DCHECK(success);
+  }
+}
 
 WebsiteSettingsInfo::~WebsiteSettingsInfo() {}