Remove the old synchronous CookieMonster API.

net/base/cookie_monster.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.
 
 // Brought to you by the letter D and the number 2.
 
 #ifndef NET_BASE_COOKIE_MONSTER_H_
 #define NET_BASE_COOKIE_MONSTER_H_
 #pragma once
 
 #include <map>
+#include <queue>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/synchronization/lock.h"
 #include "base/task.h"
 #include "base/time.h"
 #include "net/base/cookie_store.h"
 #include "net/base/net_api.h"
 
 class GURL;
 
 namespace base {
 class Histogram;
+class MessageLoopProxy;

[Randy Smith (Not in Mondays), 2011/08/19 21:40:05]

Not used in this file--maybe it should be in cookie_monster.cc?

[erikwright (departed), 2011/09/06 17:34:45]

Done.

 }
 
 namespace net {
 
 class CookieList;
+class CookieMonsterTask;
 
 // The cookie monster is the system for storing and retrieving cookies. It has
 // an in-memory list of all cookies, and synchronizes non-session cookies to an
 // optional permanent storage that implements the PersistentCookieStore
 // interface.
 //
 // This class IS thread-safe. Normally, it is only used on the I/O thread, but
 // is also accessed directly through Automation for UI testing.
 //
 // Several methods exist in asynchronous forms. Calls may be deferred if all
@@ skipping 86 matching lines @@
                 Delegate* delegate,
                 int last_access_threshold_milliseconds);
 
   // Parses the string with the cookie time (very forgivingly).
   static base::Time ParseCookieTime(const std::string& time_string);
 
   // Returns true if a domain string represents a host-only cookie,
   // i.e. it doesn't begin with a leading '.' character.
   static bool DomainIsHostOnly(const std::string& domain_string);
 
+  // Helper function that adds all cookies from |cookie_monster| into this
+  // instance.
+  bool InitializeFrom(const CookieList& list);
+
+  typedef base::Callback<void(const CookieList& cookies)> GetCookieListCallback;
+  typedef base::Callback<void(int num_deleted)> DeleteCallback;
+  typedef SetCookiesCallback DeleteCookieCallback;
+
   // Sets a cookie given explicit user-provided cookie attributes. The cookie
   // name, value, domain, etc. are each provided as separate strings. This
   // function expects each attribute to be well-formed. It will check for
   // disallowed characters (e.g. the ';' character is disallowed within the
   // cookie value attribute) and will return false without setting the cookie
   // if such characters are found.
-  bool SetCookieWithDetails(const GURL& url,
-                            const std::string& name,
-                            const std::string& value,
-                            const std::string& domain,
-                            const std::string& path,
-                            const base::Time& expiration_time,
-                            bool secure, bool http_only);
   void SetCookieWithDetailsAsync(const GURL& url,
                                  const std::string& name,
                                  const std::string& value,
                                  const std::string& domain,
                                  const std::string& path,
                                  const base::Time& expiration_time,
                                  bool secure, bool http_only,
                                  const SetCookiesCallback& callback);
 
-
-  // Helper function that adds all cookies from |cookie_monster| into this
-  // instance.
-  bool InitializeFrom(CookieMonster* cookie_monster);
-
   // Returns all the cookies, for use in management UI, etc. This does not mark
   // the cookies as having been accessed.
   // The returned cookies are ordered by longest path, then by earliest
   // creation date.
-  CookieList GetAllCookies();
-  typedef base::Callback<void(const CookieList& cookies)> GetCookieListCallback;
   void GetAllCookiesAsync(const GetCookieListCallback& callback);
 
   // Returns all the cookies, for use in management UI, etc. Filters results
   // using given url scheme, host / domain and path and options. This does not
   // mark the cookies as having been accessed.
   // The returned cookies are ordered by longest path, then earliest
   // creation date.
-  CookieList GetAllCookiesForURLWithOptions(const GURL& url,
-                                            const CookieOptions& options);
   void GetAllCookiesForURLWithOptionsAsync(
       const GURL& url,
       const CookieOptions& options,
       const GetCookieListCallback& callback);
 
   // Invokes GetAllCookiesForURLWithOptions with options set to include HTTP
   // only cookies.
-  CookieList GetAllCookiesForURL(const GURL& url);
   void GetAllCookiesForURLAsync(const GURL& url,
                                 const GetCookieListCallback& callback);
 
   // Deletes all of the cookies.
-  int DeleteAll(bool sync_to_store);
+  void DeleteAllAsync(const DeleteCallback& callback);
+
   // Deletes all of the cookies that have a creation_date greater than or equal
   // to |delete_begin| and less than |delete_end|
   // Returns the number of cookies that have been deleted.
-  int DeleteAllCreatedBetween(const base::Time& delete_begin,
-                              const base::Time& delete_end,
-                              bool sync_to_store);
-  typedef base::Callback<void(int num_deleted)> DeleteCallback;
   void DeleteAllCreatedBetweenAsync(const base::Time& delete_begin,
                                     const base::Time& delete_end,
-                                    bool sync_to_store,
                                     const DeleteCallback& callback);
 
   // Deletes all cookies that match the host of the given URL
   // regardless of path.  This includes all http_only and secure cookies,
   // but does not include any domain cookies that may apply to this host.
   // Returns the number of cookies deleted.
-  int DeleteAllForHost(const GURL& url);
-
   void DeleteAllForHostAsync(const GURL& url,
                              const DeleteCallback& callback);
 
   // Deletes one specific cookie.
-  bool DeleteCanonicalCookie(const CanonicalCookie& cookie);
-
-  typedef SetCookiesCallback DeleteCookieCallback;
-
   void DeleteCanonicalCookieAsync(const CanonicalCookie& cookie,
                                   const DeleteCookieCallback& callback);
 
   // Override the default list of schemes that are allowed to be set in
   // this cookie store.  Calling his overrides the value of
   // "enable_file_scheme_".
   // If this this method is called, it must be called before first use of
   // the instance (i.e. as part of the instance initialization process).
   void SetCookieableSchemes(const char* schemes[], size_t num_schemes);
 
@@ skipping 21 matching lines @@
   // WARNING: THE CALLBACK WILL RUN ON A RANDOM THREAD. IT MUST BE THREAD SAFE.
   // It may be posted to the current thread, or it may run on the thread that
   // actually does the flushing. Your Task should generally post a notification
   // to the thread you actually want to be notified on.
   void FlushStore(Task* completion_task);
 
   // CookieStore implementation.
 
   // Sets the cookies specified by |cookie_list| returned from |url|
   // with options |options| in effect.
-  virtual bool SetCookieWithOptions(const GURL& url,
-                                    const std::string& cookie_line,
-                                    const CookieOptions& options);
-
-  virtual void SetCookieWithOptionsAsync(const GURL& url,
-                                         const std::string& cookie_line,
-                                         const CookieOptions& options,
-                                         const SetCookiesCallback& callback);
+  virtual void SetCookieWithOptionsAsync(
+      const GURL& url,
+      const std::string& cookie_line,
+      const CookieOptions& options,
+      const SetCookiesCallback& callback) OVERRIDE;
 
   // Gets all cookies that apply to |url| given |options|.
   // The returned cookies are ordered by longest path, then earliest
   // creation date.
-  virtual std::string GetCookiesWithOptions(const GURL& url,
-                                            const CookieOptions& options);
-
   virtual void GetCookiesWithOptionsAsync(
       const GURL& url,
       const CookieOptions& options,
-      const GetCookiesCallback& callback);
+      const GetCookiesCallback& callback) OVERRIDE;
 
-  virtual void GetCookiesWithInfo(const GURL& url,
-                                  const CookieOptions& options,
-                                  std::string* cookie_line,
-                                  std::vector<CookieInfo>* cookie_infos);
-
-  virtual void GetCookiesWithInfoAsync(const GURL& url,
-                                       const CookieOptions& options,
-                                       const GetCookieInfoCallback& callback);
+  virtual void GetCookiesWithInfoAsync(
+      const GURL& url,
+      const CookieOptions& options,
+      const GetCookieInfoCallback& callback) OVERRIDE;
 
   // Deletes all cookies with that might apply to |url| that has |cookie_name|.
-  virtual void DeleteCookie(const GURL& url, const std::string& cookie_name);
   virtual void DeleteCookieAsync(
       const GURL& url, const std::string& cookie_name,
-      const base::Closure& callback);
+      const base::Closure& callback) OVERRIDE;
 
-  virtual CookieMonster* GetCookieMonster();
+  virtual CookieMonster* GetCookieMonster() OVERRIDE;
 
   // Debugging method to perform various validation checks on the map.
   // Currently just checking that there are no null CanonicalCookie pointers
   // in the map.
   // Argument |arg| is to allow retaining of arbitrary data if the CHECKs
   // in the function trip.  TODO(rdsmith):Remove hack.
   void ValidateMap(int arg);
 
   // The default list of schemes the cookie monster can handle.
   static const char* kDefaultCookieableSchemes[];
   static const int kDefaultCookieableSchemesCount;
 
  private:
+  // For queueing the cookie monster calls.
+  friend class DeleteAllCreatedBetweenTask;
+  friend class DeleteAllForHostTask;
+  friend class DeleteAllTask;
+  friend class DeleteCookieTask;
+  friend class DeleteCanonicalCookieTask;
+  friend class GetAllCookiesForURLWithOptionsTask;
+  friend class GetAllCookiesTask;
+  friend class GetCookiesWithOptionsTask;
+  friend class GetCookiesWithInfoTask;
+  friend class SetCookieWithDetailsTask;
+  friend class SetCookieWithOptionsTask;

[Randy Smith (Not in Mondays), 2011/08/19 21:40:05]

I'm feeling torn about this long list of friend classes, and the creation of all
of the different tasks.  It's both uglier and heresy, but I'm playing around
with the idea of a case statement instead (i.e. have a single object that can be
used for all the different operations that has the set union (literally; I'm not
suggesting a C union) of all the information of all of these objects plus an
enum to indicate which operation a particular object refers to, do a single
"friend class CookieMonster" inside that class, and then pass objects of that
class into a CookieMonster method (which is where the case statement will be)
for execution.  I think that results in a lot less code, many fewer friends of
the CookieMonster (the CookieMonster should have no friends--more cookies for
it!!  :-}); it's not good object oriented polymorphism, but I think it would
still be pretty clear.  I won't insist on the idea, but I wanted to float it by
you in case it appealed to you over this approach.

[erikwright (departed), 2011/09/06 17:34:45]

The case statement, IME is more error prone in addition to being uglier and
heresy :)

I would prefer to reduce the number of methods that actually need to be
asynchronous by extracting different smaller classes out of this one. This will
have the same effect (in fact, probably even a better outcome).

[Randy Smith (Not in Mondays), 2011/09/07 15:46:27]

Ok.

+
   // Testing support.
   // For SetCookieWithCreationTime.
   FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest,
                            TestCookieDeleteAllCreatedBetweenTimestamps);
 
   // For gargage collection constants.
   FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest, TestHostGarbageCollection);
   FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest, TestTotalGarbageCollection);
   FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest, GarbageCollectionTriggers);
   FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest, TestGCTimes);
@@ skipping 68 matching lines @@
 
   // Default value for key and expiry scheme scheme.
   static const ExpiryAndKeyScheme expiry_and_key_default_ =
       EKS_KEEP_RECENT_AND_PURGE_ETLDP1;
 
   // Record statistics every kRecordStatisticsIntervalSeconds of uptime.
   static const int kRecordStatisticsIntervalSeconds = 10 * 60;
 
   virtual ~CookieMonster();
 
+  bool SetCookieWithDetails(const GURL& url,

[Randy Smith (Not in Mondays), 2011/08/19 21:40:05]

Maybe a comment before these indicating how they're used and what the
constraints are (I'm presuming by the async callbacks, only valid to call once
the db is loaded).

[erikwright (departed), 2011/09/06 17:34:45]

Done.

+                            const std::string& name,
+                            const std::string& value,
+                            const std::string& domain,
+                            const std::string& path,
+                            const base::Time& expiration_time,
+                            bool secure, bool http_only);
+
+  CookieList GetAllCookies();
+
+  CookieList GetAllCookiesForURLWithOptions(const GURL& url,
+                                            const CookieOptions& options);
+
+  CookieList GetAllCookiesForURL(const GURL& url);

[Randy Smith (Not in Mondays), 2011/08/19 21:40:05]

nit: If this is now only a helper call called from a single place, you can
remove it and force callers to use the more detailed version.

[erikwright (departed), 2011/09/06 17:34:45]

It has four callers besides tests. For now I'll leave it I guess.

On 2011/08/19 21:40:05, rdsmith wrote:

+
+  int DeleteAll(bool sync_to_store);
+
+  int DeleteAllCreatedBetween(const base::Time& delete_begin,
+                              const base::Time& delete_end);
+
+  int DeleteAllForHost(const GURL& url);
+
+  bool DeleteCanonicalCookie(const CanonicalCookie& cookie);
+
+  bool SetCookieWithOptions(const GURL& url,
+                            const std::string& cookie_line,
+                            const CookieOptions& options);
+
+  std::string GetCookiesWithOptions(const GURL& url,
+                                    const CookieOptions& options);
+
+  void GetCookiesWithInfo(const GURL& url,
+                          const CookieOptions& options,
+                          std::string* cookie_line,
+                          std::vector<CookieInfo>* cookie_infos);
+
+  void DeleteCookie(const GURL& url, const std::string& cookie_name);
+
   bool SetCookieWithCreationTime(const GURL& url,
                                  const std::string& cookie_line,
                                  const base::Time& creation_time);
 
   // Called by all non-static functions to ensure that the cookies store has
   // been initialized. This is not done during creating so it doesn't block
   // the window showing.
   // Note: this method should always be called with lock_ held.
   void InitIfNecessary() {
     if (!initialized_) {
-      if (store_)
+      if (store_) {
         InitStore();
+      } else {
+        loaded_ = true;
+      }
       initialized_ = true;
     }
   }
 
   // Initializes the backing store and reads existing cookies from it.
   // Should only be called by InitIfNecessary().
   void InitStore();
 
+  // Reads the existing cookies loaded form the backing store.

[Randy Smith (Not in Mondays), 2011/08/19 21:40:05]

I'd suggest instead "Called when loading of the existing cookies on the backing
store is complete."

[erikwright (departed), 2011/09/06 17:34:45]

Done.

+  void OnLoaded(const std::vector<CanonicalCookie*>& cookies);
+
+  // Ininitalize the cookies.
+  void StoreLoadedCookies(const std::vector<CanonicalCookie*>& cookies);
+
+  // Invoke the queueing calls of CookieMonster when loading cookies from

[Randy Smith (Not in Mondays), 2011/08/19 21:40:05]

nit: "queueing" -> "queued".

[erikwright (departed), 2011/09/06 17:34:45]

Done.

+  // the back store is complete.
+  void InvokeQueue();
+
   // Checks that |cookies_| matches our invariants, and tries to repair any
   // inconsistencies. (In other words, it does not have duplicate cookies).
   void EnsureCookiesMapIsValid();
 
   // Checks for any duplicate cookies for CookieMap key |key| which lie between
   // |begin| and |end|. If any are found, all but the most recent are deleted.
   // Returns the number of duplicate cookies that were deleted.
   int TrimDuplicateCookiesForKey(const std::string& key,
                                  CookieMap::iterator begin,
                                  CookieMap::iterator end);
@@ skipping 89 matching lines @@
   void RecordPeriodicStats(const base::Time& current_time);
 
   // Initialize the above variables; should only be called from
   // the constructor.
   void InitializeHistograms();
 
   // The resolution of our time isn't enough, so we do something
   // ugly and increment when we've seen the same time twice.
   base::Time CurrentTime();
 
+  // Run the cookie request task if cookie loaded, otherwise added the task
+  // to task queue.
+  void DoCookieTask(scoped_refptr<CookieMonsterTask> task_item);
+
   // Histogram variables; see CookieMonster::InitializeHistograms() in
   // cookie_monster.cc for details.
   base::Histogram* histogram_expiration_duration_minutes_;
   base::Histogram* histogram_between_access_interval_minutes_;
   base::Histogram* histogram_evicted_last_access_minutes_;
   base::Histogram* histogram_count_;
   base::Histogram* histogram_domain_count_;
   base::Histogram* histogram_etldp1_count_;
   base::Histogram* histogram_domain_per_etldp1_count_;
   base::Histogram* histogram_number_duplicate_db_cookies_;
   base::Histogram* histogram_cookie_deletion_cause_;
   base::Histogram* histogram_time_get_;
   base::Histogram* histogram_time_mac_;
   base::Histogram* histogram_time_load_;
 
   CookieMap cookies_;
 
   // Indicates whether the cookie store has been initialized. This happens
   // lazily in InitStoreIfNecessary().
   bool initialized_;
 
+  // Indicates whether loading from the backend store is completed and
+  // calls may be immediately processed.
+  bool loaded_;
+
+  // Queues calls to CookieMonster until loading from the backend store is
+  // completed.
+  std::queue<base::Closure> queue_;
+
   // Indicates whether this cookie monster uses the new effective domain
   // key scheme or not.
   ExpiryAndKeyScheme expiry_and_key_scheme_;
 
   scoped_refptr<PersistentCookieStore> store_;
 
   base::Time last_time_seen_;
 
   // Minimum delay after updating a cookie's LastAccessDate before we will
   // update it again.
@@ skipping 50 matching lines @@
 
   // This constructor does canonicalization but not validation.
   // The result of this constructor should not be relied on in contexts
   // in which pre-validation of the ParsedCookie has not been done.
   CanonicalCookie(const GURL& url, const ParsedCookie& pc);
 
   ~CanonicalCookie();
 
   // Supports the default copy constructor.
 
+  // Creates a canonical cookie from parsed cookie.
+  // Canonicalizes and validates inputs.  May return NULL if an attribute
+  // value is invalid.
+  static CanonicalCookie* Create(const GURL& url,
+                                 const ParsedCookie& pc);
+
   // Creates a canonical cookie from unparsed attribute values.
   // Canonicalizes and validates inputs.  May return NULL if an attribute
   // value is invalid.
   static CanonicalCookie* Create(const GURL& url,
                                  const std::string& name,
                                  const std::string& value,
                                  const std::string& domain,
                                  const std::string& path,
                                  const std::string& mac_key,
                                  const std::string& mac_algorithm,
@@ skipping 218 matching lines @@
 };
 
 typedef base::RefCountedThreadSafe<CookieMonster::PersistentCookieStore>
     RefcountedPersistentCookieStore;
 
 class CookieMonster::PersistentCookieStore
     : public RefcountedPersistentCookieStore {
  public:
   virtual ~PersistentCookieStore() {}
 
+  typedef base::Callback<void(const std::vector<
+      CookieMonster::CanonicalCookie*>&)> LoadedCallback;
+
   // Initializes the store and retrieves the existing cookies. This will be
   // called only once at startup.
-  virtual bool Load(std::vector<CookieMonster::CanonicalCookie*>* cookies) = 0;
+  virtual bool Load(const LoadedCallback& loaded_callback) = 0;
 
   virtual void AddCookie(const CanonicalCookie& cc) = 0;
   virtual void UpdateCookieAccessTime(const CanonicalCookie& cc) = 0;
   virtual void DeleteCookie(const CanonicalCookie& cc) = 0;
 
   // Sets the value of the user preference whether the persistent storage
   // must be deleted upon destruction.
   virtual void SetClearLocalStateOnExit(bool clear_local_state) = 0;
 
   // Flush the store and post the given Task when complete.
   virtual void Flush(Task* completion_task) = 0;
 
  protected:
   PersistentCookieStore() {}
 
  private:
   DISALLOW_COPY_AND_ASSIGN(PersistentCookieStore);
 };
 
 class CookieList : public std::vector<CookieMonster::CanonicalCookie> {
 };
 
 }  // namespace net
 
 #endif  // NET_BASE_COOKIE_MONSTER_H_