HostContentSettingsMap refactoring.

chrome/browser/content_settings/host_content_settings_map.h

 // 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.
 
 // Maps hostnames to custom content settings.  Written on the UI thread and read
 // on any thread.  One instance per profile.
 
 #ifndef CHROME_BROWSER_CONTENT_SETTINGS_HOST_CONTENT_SETTINGS_MAP_H_
 #define CHROME_BROWSER_CONTENT_SETTINGS_HOST_CONTENT_SETTINGS_MAP_H_
 #pragma once
@@ skipping 21 matching lines @@
 }  // namespace content_settings
 
 class ContentSettingsDetails;
 class ExtensionService;
 class GURL;
 class PrefService;
 class Profile;
 
 class HostContentSettingsMap
     : public content_settings::Observer,
-      public NotificationObserver,
       public base::RefCountedThreadSafe<HostContentSettingsMap> {
  public:
   typedef Tuple4<ContentSettingsPattern,
                  ContentSettingsPattern,
                  ContentSetting,
                  std::string> PatternSettingSourceTuple;
   typedef std::vector<PatternSettingSourceTuple> SettingsForOneType;
 
   HostContentSettingsMap(PrefService* prefs,
                          ExtensionService* extension_service,
@@ skipping 17 matching lines @@
   // require an resource identifier to be specified, the |resource_identifier|
   // must be non-empty.
   //
   // This may be called on any thread.
   ContentSetting GetContentSetting(
       const GURL& primary_url,
       const GURL& secondary_url,
       ContentSettingsType content_type,
       const std::string& resource_identifier) const;
 
-  // Gets the content setting for cookies. This takes the third party cookie
-  // flag into account, and therefore needs to know whether we read or write a
-  // cookie.
-  //
-  // This may be called on any thread.
-  ContentSetting GetCookieContentSetting(
-      const GURL& url,
-      const GURL& first_party_url,
-      bool setting_cookie) const;
-
   // Returns a single ContentSetting which applies to the given URLs or
   // CONTENT_SETTING_DEFAULT, if no exception applies. Note that certain
   // internal schemes are whitelisted. For ContentSettingsTypes that require an
   // resource identifier to be specified, the |resource_identifier| must be
   // non-empty.
   //
   // This may be called on any thread.
   ContentSetting GetNonDefaultContentSetting(
       const GURL& primary_url,
       const GURL& secondary_url,
@@ skipping 65 matching lines @@
                           ContentSetting setting);
 
   // Clears all host-specific settings for one content type.
   //
   // This should only be called on the UI thread.
   void ClearSettingsForOneType(ContentSettingsType content_type);
 
   static bool IsSettingAllowedForType(ContentSetting setting,
                                       ContentSettingsType content_type);
 
-  // This setting trumps any host-specific settings.
-  bool BlockThirdPartyCookies() const { return block_third_party_cookies_; }
-  bool IsBlockThirdPartyCookiesManaged() const {
-    return is_block_third_party_cookies_managed_;
-  }
-
-  // Sets whether we block all third-party cookies. This method must not be
-  // invoked on an incognito map.
-  //
-  // This should only be called on the UI thread.
-  void SetBlockThirdPartyCookies(bool block);
-
   // Returns true if the default setting for the |content_type| is managed.
   bool IsDefaultContentSettingManaged(ContentSettingsType content_type) const;
 
   // Detaches the HostContentSettingsMap from all Profile-related objects like
   // PrefService. This methods needs to be called before destroying the Profile.
   // Afterwards, none of the methods above that should only be called on the UI
   // thread should be called anymore.
   void ShutdownOnUIThread();
 
   // content_settings::Observer implementation.
   virtual void OnContentSettingChanged(
       ContentSettingsPattern primary_pattern,
       ContentSettingsPattern secondary_pattern,
       ContentSettingsType content_type,
       std::string resource_identifier);
 
-  // NotificationObserver implementation.
-  virtual void Observe(int type,
-                       const NotificationSource& source,
-                       const NotificationDetails& details);
-
  private:
   friend class base::RefCountedThreadSafe<HostContentSettingsMap>;
 
   virtual ~HostContentSettingsMap();
 
   ContentSetting GetContentSettingInternal(
       const GURL& primary_url,
       const GURL& secondary_url,
       ContentSettingsType content_type,
       const std::string& resource_identifier) const;
 
-  // Various migration methods (old cookie, popup and per-host data gets
-  // migrated to the new format).
-  void MigrateObsoleteCookiePref();
-
   // Weak; owned by the Profile.
   PrefService* prefs_;
 
-  PrefChangeRegistrar pref_change_registrar_;
-
   // Whether this settings map is for an OTR session.
   bool is_off_the_record_;
 
   // Whether we are currently updating preferences, this is used to ignore
   // notifications from the preferences service that we triggered ourself.
   bool updating_preferences_;
 
   // Default content setting providers.
   std::vector<linked_ptr<content_settings::DefaultProviderInterface> >
       default_content_settings_providers_;
 
   // Content setting providers.
   std::vector<linked_ptr<content_settings::ProviderInterface> >
       content_settings_providers_;
 
+  DISALLOW_COPY_AND_ASSIGN(HostContentSettingsMap);
+};
+
+// FIXME(marja): This should be moved into a separate file.

[markusheintz_, 2011/08/24 12:29:40]

Please move to a new file: cookie_settings.h

[marja, 2011/08/25 13:56:11]

Done.

+class CookieContentSettings

[markusheintz_, 2011/08/24 12:29:40]

Maybe we could just call it 'CookieSettings' since this class allows to query
settings for cookies.

I'd like to get rid of the 'Content' part of the names as content settings will
be called website settings in the future.

[marja, 2011/08/25 13:56:11]

Done.

+    : public NotificationObserver,
+      public base::RefCountedThreadSafe<CookieContentSettings>{
+ public:
+  CookieContentSettings(
+      HostContentSettingsMap* host_content_settings_map,
+      PrefService* prefs,
+      bool incognito);
+
+  bool Allow(const GURL& url,

[markusheintz_, 2011/08/24 12:29:40]

I guess you should add a comment here that describes what this method does.

[markusheintz_, 2011/08/24 12:29:40]

I just wonder whether this method should be called 'IsCookieAllowed'.

The reason why I don't like the name 'Allow' here is that is suggests that the
method takes some action to allow something. But it only returns a state.

[marja, 2011/08/25 13:56:11]

Done.

[marja, 2011/08/25 13:56:11]

Done.

+             const GURL& first_party_url,
+             bool setting_cookie) const;
+
+  bool EnforceSessionOnly(const GURL& url) const;

[markusheintz_, 2011/08/24 12:29:40]

Add a comment please.

[markusheintz_, 2011/08/24 12:29:40]

Analog to the proposed name of the method above this method could be alled
'IsCookieSessionOnly'.

The reason why I don't like the name 'EnforceSessionOnly' is that is suggest
that the method enforces something. But all the method does is getting a state.

[marja, 2011/08/25 13:56:11]

Done.

[marja, 2011/08/25 13:56:11]

Done.

+
+  // This setting trumps any host-specific settings.
+  bool BlockThirdPartyCookies() const { return block_third_party_cookies_; }
+  bool IsBlockThirdPartyCookiesManaged() const {
+    return is_block_third_party_cookies_managed_;
+  }
+
+  // Sets whether we block all third-party cookies. This method must not be
+  // invoked on an incognito map.
+  //
+  // This should only be called on the UI thread.
+  void SetBlockThirdPartyCookies(bool block);
+
+  // NotificationObserver implementation.
+  virtual void Observe(int type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+
+  // Detaches the HostContentSettingsMap from all Profile-related objects like

[markusheintz_, 2011/08/24 12:29:40]

Please update the comment. s/HostContentSettingsMap/CookieContentSettings/g

[marja, 2011/08/25 13:56:11]

Done.

+  // PrefService. This methods needs to be called before destroying the Profile.
+  // Afterwards, none of the methods above that should only be called on the UI
+  // thread should be called anymore.
+  void ShutdownOnUIThread();
+
+ private:
+  // Various migration methods (old cookie, popup and per-host data gets
+  // migrated to the new format).
+  void MigrateObsoleteCookiePref();

[markusheintz_, 2011/08/24 12:29:40]

This method is around for quite a while. I guess we could get rid of it now.

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

Should I also remove const char kCookieBehavior[] from pref_names.cc, and
prefs->RegisterIntegerPref(prefs::kCookieBehavior, ...) from
host_content_settings_map.cc, or will it break if I remove those, too?

+
+  ContentSetting GetCookieContentSetting(const GURL& url,
+                                         const GURL& first_party_url,
+                                         bool setting_cookie) const;
+
+  scoped_refptr<HostContentSettingsMap> host_content_settings_map_;
+
+  // Weak; owned by the profile.
+  PrefService* prefs_;
+  PrefChangeRegistrar pref_change_registrar_;
+
+  bool is_off_the_record_;
+
   // Used around accesses to the following objects to guarantee thread safety.
   mutable base::Lock lock_;
 
   // Misc global settings.
   bool block_third_party_cookies_;
   bool is_block_third_party_cookies_managed_;
-
-  DISALLOW_COPY_AND_ASSIGN(HostContentSettingsMap);
 };
 
 #endif  // CHROME_BROWSER_CONTENT_SETTINGS_HOST_CONTENT_SETTINGS_MAP_H_