HostContentSettingsMap refactoring.

chrome/browser/ui/webui/options/content_settings_handler.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/ui/webui/options/content_settings_handler.h"
 
 #include <map>
 #include <string>
 #include <vector>
 
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/content_settings/content_settings_details.h"
 #include "chrome/browser/content_settings/content_settings_pattern.h"
 #include "chrome/browser/content_settings/content_settings_utils.h"
+#include "chrome/browser/content_settings/cookie_settings.h"
 #include "chrome/browser/content_settings/host_content_settings_map.h"
 #include "chrome/browser/custom_handlers/protocol_handler_registry.h"
 #include "chrome/browser/notifications/desktop_notification_service.h"
 #include "chrome/browser/notifications/desktop_notification_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/content_settings_helper.h"
 #include "chrome/common/pref_names.h"
@@ skipping 25 matching lines @@
 
 const ContentSettingsTypeNameEntry kContentSettingsTypeGroupNames[] = {
   {CONTENT_SETTINGS_TYPE_COOKIES, "cookies"},
   {CONTENT_SETTINGS_TYPE_IMAGES, "images"},
   {CONTENT_SETTINGS_TYPE_JAVASCRIPT, "javascript"},
   {CONTENT_SETTINGS_TYPE_PLUGINS, "plugins"},
   {CONTENT_SETTINGS_TYPE_POPUPS, "popups"},
   {CONTENT_SETTINGS_TYPE_GEOLOCATION, "location"},
   {CONTENT_SETTINGS_TYPE_NOTIFICATIONS, "notifications"},
   {CONTENT_SETTINGS_TYPE_INTENTS, "intents"},
+  {CONTENT_SETTINGS_TYPE_COOKIES_SESSION_ONLY, "cookies-session-only"},

[Bernhard Bauer, 2011/08/28 19:18:49]

This seems weird, as this content type isn't exposed in the UI. Also, this map
is only used for looking up content types from strings, so it will never be
called with "cookies-session-only".

[marja, 2011/08/30 10:10:38]

What breaks if that is simply removed: ContentSettingsHandler is reading the
"list of exceptions" to dispay in the UI, for all content types. It loops from
CONTENT_SETTINGS_TYPE_DEFAULT + 1 to CONTENT_SETTING_NUM_TYPES, and for other
types than geolocation or notifications, it calls
UpdateExeceptionsViewFromHostContentSettingsMap, and that calls
ContentSettingsTypeToGroupName. That just asserts, if there is no entry for the
content setting in kContentSettingsTypeGroupNames.

Should I e.g., make it loop through a separate list of "ui content types"
instead of all content types, as a part of this cl? Or some other fix (like,
loop through all types as before, but ignore it if it doesn't have an entry in
kContentSettingsTypeGroupNames)?

[Bernhard Bauer, 2011/08/30 11:03:12]

Ah, I missed |ContentSettingsTypeToGroupName| below (and there is UI for
intents, it's just hidden).

I still think what we should do here is return NULL in
|ContentSettingsTypeToGroupName| for an unknown content type, and not update the
UI in that case. That should allow us to add more content types without having
to change the Web UI code.

[markusheintz_, 2011/08/30 11:49:34]

How about providing a method that converts a ContentSettingsType to a string and
vice versa? Then we don't need this array at all. This method should ideally be
provided by content_settings_type.h

 };
 
 ContentSettingsType ContentSettingsTypeFromGroupName(const std::string& name) {
   for (size_t i = 0; i < arraysize(kContentSettingsTypeGroupNames); ++i) {
     if (name == kContentSettingsTypeGroupNames[i].name)
       return kContentSettingsTypeGroupNames[i].type;
   }
 
   NOTREACHED() << name << " is not a recognized content settings type.";
   return CONTENT_SETTINGS_TYPE_DEFAULT;
@@ skipping 526 matching lines @@
     NOTREACHED();
     return;
   }
 
   ContentSetting default_setting = ContentSettingFromString(setting);
   ContentSettingsType content_type = ContentSettingsTypeFromGroupName(group);
   if (content_type == CONTENT_SETTINGS_TYPE_NOTIFICATIONS) {
     Profile* profile = Profile::FromWebUI(web_ui_);
     DesktopNotificationServiceFactory::GetForProfile(profile)->
         SetDefaultContentSetting(default_setting);
+  } else if (content_type == CONTENT_SETTINGS_TYPE_COOKIES) {
+    GetContentSettingsMap()->GetCookieSettings()->SetDefaultSetting(
+        default_setting);
+
   } else {
     GetContentSettingsMap()->
         SetDefaultContentSetting(content_type, default_setting);
   }
 }
 
 void ContentSettingsHandler::RemoveException(const ListValue* args) {
   size_t arg_i = 0;
   std::string type_string;
   CHECK(args->GetString(arg_i++, &type_string));
@@ skipping 35 matching lines @@
     bool rv = args->GetString(arg_i++, &mode);
     DCHECK(rv);
 
     std::string pattern;
     rv = args->GetString(arg_i++, &pattern);
     DCHECK(rv);
 
     HostContentSettingsMap* settings_map =
         mode == "normal" ? GetContentSettingsMap() :
                            GetOTRContentSettingsMap();
+
     // The settings map could be null if the mode was OTR but the OTR profile
     // got destroyed before we received this message.
     if (settings_map) {
-      settings_map->SetContentSetting(
-          ContentSettingsPattern::FromString(pattern),
-          ContentSettingsPattern::Wildcard(),
-          ContentSettingsTypeFromGroupName(type_string),
-          "",
-          CONTENT_SETTING_DEFAULT);
+      if (type == CONTENT_SETTINGS_TYPE_COOKIES) {
+        settings_map->GetCookieSettings()->ResetCookieAllowed(
+            ContentSettingsPattern::FromString(pattern),
+            ContentSettingsPattern::Wildcard());
+        settings_map->GetCookieSettings()->ResetCookieSessionOnly(
+            ContentSettingsPattern::FromString(pattern));
+      } else {
+        settings_map->SetContentSetting(
+            ContentSettingsPattern::FromString(pattern),
+            ContentSettingsPattern::Wildcard(),
+            ContentSettingsTypeFromGroupName(type_string),
+            "",
+            CONTENT_SETTING_DEFAULT);
+      }
     }
   }
 }
 
 void ContentSettingsHandler::SetException(const ListValue* args) {
   size_t arg_i = 0;
   std::string type_string;
   CHECK(args->GetString(arg_i++, &type_string));
   std::string mode;
   CHECK(args->GetString(arg_i++, &mode));
   std::string pattern;
   CHECK(args->GetString(arg_i++, &pattern));
   std::string setting;
   CHECK(args->GetString(arg_i++, &setting));
 
   ContentSettingsType type = ContentSettingsTypeFromGroupName(type_string);
   if (type == CONTENT_SETTINGS_TYPE_GEOLOCATION ||
       type == CONTENT_SETTINGS_TYPE_NOTIFICATIONS) {
     NOTREACHED();
     return;
   }
 
   HostContentSettingsMap* settings_map =
       mode == "normal" ? GetContentSettingsMap() :
                          GetOTRContentSettingsMap();
-
   // The settings map could be null if the mode was OTR but the OTR profile
   // got destroyed before we received this message.
   if (!settings_map)
     return;
-  settings_map->SetContentSetting(ContentSettingsPattern::FromString(pattern),
-                                  ContentSettingsPattern::Wildcard(),
-                                  type,
-                                  "",
-                                  ContentSettingFromString(setting));
+
+  if (type == CONTENT_SETTINGS_TYPE_COOKIES) {
+    ContentSetting content_setting = ContentSettingFromString(setting);
+    CookieSettings* cookie_settings = settings_map->GetCookieSettings();
+    bool allow = (content_setting == CONTENT_SETTING_ALLOW ||
+                  content_setting == CONTENT_SETTING_SESSION_ONLY);
+    bool session_only = (content_setting == CONTENT_SETTING_SESSION_ONLY);
+    cookie_settings->SetCookieAllowed(
+          ContentSettingsPattern::FromString(pattern),
+          ContentSettingsPattern::Wildcard(), allow);
+    cookie_settings->SetCookieSessionOnly(
+        ContentSettingsPattern::FromString(pattern), session_only);

[Bernhard Bauer, 2011/08/28 19:18:49]

Maybe we could add a convenience method for this to CookieSettings?

[marja, 2011/09/01 11:03:19]

Now there is CookieSettings::SetCookieSetting, which makes this a lot simpler.

[Bernhard Bauer, 2011/09/01 11:41:54]

Nice, thank you!

+  } else {
+    settings_map->SetContentSetting(ContentSettingsPattern::FromString(pattern),
+                                    ContentSettingsPattern::Wildcard(),
+                                    type,
+                                    "",
+                                    ContentSettingFromString(setting));
+  }
 }
 
 void ContentSettingsHandler::CheckExceptionPatternValidity(
     const ListValue* args) {
   size_t arg_i = 0;
   Value* type;
   CHECK(args->Get(arg_i++, &type));
   std::string mode_string;
   CHECK(args->GetString(arg_i++, &mode_string));
   std::string pattern_string;
@@ skipping 34 matching lines @@
   return Profile::FromWebUI(web_ui_)->GetProtocolHandlerRegistry();
 }
 
 HostContentSettingsMap*
     ContentSettingsHandler::GetOTRContentSettingsMap() {
   Profile* profile = Profile::FromWebUI(web_ui_);
   if (profile->HasOffTheRecordProfile())
     return profile->GetOffTheRecordProfile()->GetHostContentSettingsMap();
   return NULL;
 }