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

chrome/browser/content_settings/host_content_settings_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/host_content_settings_map.h"
 
 #include <list>
 
 #include "base/command_line.h"
 #include "base/string_util.h"
@@ skipping 213 matching lines @@
     const GURL& secondary_url,
     ContentSettingsType content_type,
     const std::string& resource_identifier) const {
   DCHECK_NE(CONTENT_SETTINGS_TYPE_COOKIES, content_type);
   DCHECK_NE(content_settings::RequiresResourceIdentifier(content_type),
             resource_identifier.empty());
   return GetContentSettingInternal(
       primary_url, secondary_url, content_type, resource_identifier);
 }
 
+Value* HostContentSettingsMap::GetContentSettingValue(
+    const GURL& primary_url,
+    const GURL& secondary_url,
+    ContentSettingsType content_type,
+    const std::string& resource_identifier) const {
+  if (ShouldAllowAllContent(secondary_url, content_type))
+    return Value::CreateIntegerValue(CONTENT_SETTING_ALLOW);
+
+  // Iterate through the list of providers and break when the first non NULL
+  // value is returned.
+  Value* value = NULL;

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

Nit: move the declaration of |value| inside the for loop and
merge it with line 248.

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

Done.

+  for (ConstProviderIterator provider = content_settings_providers_.begin();
+       provider != content_settings_providers_.end();
+       ++provider) {
+    value= (*provider)->GetContentSettingValue(

[Pam (message me for reviews), 2011/09/02 11:05:29]

Nit: space before =

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

Done.

+        primary_url, secondary_url, content_type, resource_identifier);
+    if (value)
+      return value;
+  }
+
+  // TODO(markusheintz): Add a comment.

[Pam (message me for reviews), 2011/09/02 11:05:29]

Please do todo.

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

Done.

+  if (content_type == CONTENT_SETTINGS_TYPE_AUTO_SELECT_CERTIFICATE)
+    return NULL;
+  return Value::CreateIntegerValue(GetDefaultContentSetting(content_type));

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

It seems like you have four cases in this function.
1. ShouldAllowAllContent(secondary_url, content_type) is true.
2. The for loop.
3. content_type == CONTENT_SETTINGS_TYPE_AUTO_SELECT_CERTIFICATE
4. The rest.

It's not clear to me why you handle the four cases in
this particular order.  If this order is obvious to the
people familiar with content settings, or if the order
is not important, then please ignore this question.
Otherwise, it would be nice to document why you handle
the cases in that order.

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

I added some comments. I hope they answer you questions.

+}
+
 ContentSetting HostContentSettingsMap::GetContentSettingInternal(
       const GURL& primary_url,
       const GURL& secondary_url,
       ContentSettingsType content_type,
       const std::string& resource_identifier) const {
   ContentSetting setting = GetNonDefaultContentSetting(
       primary_url, secondary_url, content_type, resource_identifier);
   if (setting == CONTENT_SETTING_DEFAULT)
     return GetDefaultContentSetting(content_type);
   return setting;
@@ skipping 299 matching lines @@
         SetDefaultContentSetting(CONTENT_SETTINGS_TYPE_COOKIES,
             (cookie_behavior == net::StaticCookiePolicy::BLOCK_ALL_COOKIES) ?
                 CONTENT_SETTING_BLOCK : CONTENT_SETTING_ALLOW);
     }
     if (!prefs_->HasPrefPath(prefs::kBlockThirdPartyCookies)) {
       SetBlockThirdPartyCookies(cookie_behavior ==
           net::StaticCookiePolicy::BLOCK_SETTING_THIRD_PARTY_COOKIES);
     }
   }
 }