Add a method to the HostContentSettings map to return the |Value| of a content setting

chrome/browser/content_settings/content_settings_policy_provider.cc

Index: chrome/browser/content_settings/content_settings_policy_provider.cc
diff --git a/chrome/browser/content_settings/content_settings_policy_provider.cc b/chrome/browser/content_settings/content_settings_policy_provider.cc
index 8015b56589c258beaf2258e365b55635c9bf1220..aea2b7ff945d7b8623024abd867f60caa5ff392c 100644
--- a/chrome/browser/content_settings/content_settings_policy_provider.cc
+++ b/chrome/browser/content_settings/content_settings_policy_provider.cc
@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "base/command_line.h"
+#include "base/json/json_reader.h"
 #include "chrome/browser/content_settings/content_settings_pattern.h"
 #include "chrome/browser/content_settings/content_settings_utils.h"
 #include "chrome/browser/prefs/pref_service.h"
@@ -45,10 +46,6 @@ struct PrefsForManagedContentSettingsMapEntry {
 const PrefsForManagedContentSettingsMapEntry
     kPrefsForManagedContentSettingsMap[] = {
   {
-    prefs::kManagedAutoSelectCertificateForUrls,
-    CONTENT_SETTINGS_TYPE_AUTO_SELECT_CERTIFICATE,
-    CONTENT_SETTING_ALLOW
-  }, {
     prefs::kManagedCookiesAllowedForUrls,
     CONTENT_SETTINGS_TYPE_COOKIES,
     CONTENT_SETTING_ALLOW
@@ -341,6 +338,7 @@ void PolicyProvider::GetContentSettingsFromPreferences(
 
       ContentSettingsType content_type =
           kPrefsForManagedContentSettingsMap[i].content_type;
+      DCHECK(content_type != CONTENT_SETTINGS_TYPE_AUTO_SELECT_CERTIFICATE);

[wtc, 2011/09/01 22:42:34]

Nit: use DCHECK_NE.

[markusheintz_, 2011/09/02 14:55:59]

Done.

       // If only one pattern was defined auto expand it to a pattern pair.
       ContentSettingsPattern secondary_pattern =
           !pattern_pair.second.IsValid() ? ContentSettingsPattern::Wildcard()
@@ -355,6 +353,64 @@ void PolicyProvider::GetContentSettingsFromPreferences(
   }
 }
 
+void PolicyProvider::GetAutoSelectCertificateSettingsFromPreferences(
+    OriginIdentifierValueMap* value_map) {
+  const char* pref_name = prefs::kManagedAutoSelectCertificateForUrls;
+
+  if (!prefs_->HasPrefPath(pref_name)) {
+    VLOG(2) << "Skipping unset preference: " << pref_name;
+    return;
+  }
+
+  const PrefService::Preference* pref = prefs_->FindPreference(pref_name);
+  DCHECK(pref);
+  DCHECK(pref->IsManaged());
+
+  const ListValue* pattern_filter_str_list = NULL;
+  if (!pref->GetValue()->GetAsList(&pattern_filter_str_list)) {
+    NOTREACHED();
+    return;
+  }
+
+  for (size_t j = 0; j < pattern_filter_str_list->GetSize(); ++j) {
+    std::string pattern_filter_str;
+    pattern_filter_str_list->GetString(j, &pattern_filter_str);
+
+    size_t separator_pos = pattern_filter_str.find("|");

[wtc, 2011/09/01 22:42:34]

Nit: is there a version of find() that takes a character '|'
(instead of a string "|") as input?  That may be slightly
more efficient.

It would be nice to document the format of the pattern_filter_str.
By reading the code, I can tell it's:
  pattern|filter
where filter is a JSON dictionary.

[markusheintz_, 2011/09/02 14:55:59]

Done.

+    if (separator_pos == std::string::npos) {
+      VLOG(1) << "No filter string found. Ignoring list entry: "
+              << pattern_filter_str;
+      continue;
+    }
+    std::string pattern_str = pattern_filter_str.substr(0, separator_pos);
+    std::string filter_str = pattern_filter_str.substr(separator_pos + 1);
+
+    ContentSettingsPattern pattern =
+        ContentSettingsPattern::FromString(pattern_str);
+    // Ignore invalid patterns.
+    if (!pattern.IsValid()) {
+      VLOG(1) << "Invalid content settings pattern: " << pattern_str
+              << ". Ignoring list entry: " << pattern_filter_str;
+      continue;
+    }
+
+    // Allow trailing commas to be more fault tolerant.
+    scoped_ptr<Value> filter_value(base::JSONReader::Read(filter_str, true));
+    DCHECK(filter_value->IsType(Value::TYPE_DICTIONARY));

[wtc, 2011/09/01 22:42:34]

The DCHECK should be done after the null pointer test
on line 400.

But, if the input comes from an external source, such as a file,
we should not use DCHECK here, because the file may be
corrupted.  We should handle the error.

I guess JSON by definition is a dictionary?  If so, then
it is appropriate to use a DCHECK here.

[markusheintz_, 2011/09/02 14:55:59]

Done.
If the string is corrupted then |base::JSONReader::Read| should return NULL. I
guess I better ignore such strings then. Better asking the user than auto
selecting certs.
Shouldn't we rm the DCHECK then?

+    if (!filter_value.get()) {
+      VLOG(1) << "Invalid JSON string: " << filter_str
+              << ". Ignoring list entry: " << pattern_filter_str;
+      continue;
+    }
+
+    value_map->SetValue(pattern,
+                        ContentSettingsPattern::Wildcard(),
+                        CONTENT_SETTINGS_TYPE_AUTO_SELECT_CERTIFICATE,
+                        std::string(),
+                        filter_value.release());
+  }
+}
+
 void PolicyProvider::ReadManagedContentSettings(bool overwrite) {
   ContentSettingsRules rules;
   GetContentSettingsFromPreferences(&rules);
@@ -371,6 +427,7 @@ void PolicyProvider::ReadManagedContentSettings(bool overwrite) {
                           rule->d,
                           Value::CreateIntegerValue(rule->e));
     }
+    GetAutoSelectCertificateSettingsFromPreferences(&value_map_);
   }
 }
 
@@ -389,6 +446,7 @@ ContentSetting PolicyProvider::GetContentSetting(
     const GURL& secondary_url,
     ContentSettingsType content_type,
     const ResourceIdentifier& resource_identifier) const {
+  DCHECK_NE(content_type, CONTENT_SETTINGS_TYPE_AUTO_SELECT_CERTIFICATE);
   // Resource identifier are not supported by policies as long as the feature is
   // behind a flag. So resource identifiers are simply ignored.
   scoped_ptr<Value> value(GetContentSettingValue(primary_url,