Add FlushStore to CookieStore interface.

This results in consumers no longer having to downcast to
CookieMonster to call the method, and is part of the process
of removing CookieStore::GetCookieMonster entirely.

Also move a lot of the CookieMonster documentation to
the corresponding CookieStore methods, and moved methods
around to have consistent ordering.

BUG=579653
TBR=newt@chromium.org

Review URL: https://codereview.chromium.org/1616443003

Cr-Commit-Position: refs/heads/master@{#370988}

4 files changed, 35 insertions, 41 deletions

diff --git a/net/cookies/cookie_monster.h b/net/cookies/cookie_monster.h
index 320ee66..fc27714 100644
--- a/net/cookies/cookie_monster.h
+++ b/net/cookies/cookie_monster.h
@@ -51,10 +51,8 @@ class ParsedCookie;
 // 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.
 //
-// All cookie tasks are handled asynchronously. Tasks may be deferred if
-// all affected cookies are not yet loaded from the backing store. Otherwise,
-// the callback may be invoked immediately (prior to return of the asynchronous
-// function).
+// Tasks may be deferred if all affected cookies are not yet loaded from the
+// backing store. Otherwise, callbacks may be invoked immediately.
 //
 // A cookie task is either pending loading of the entire cookie store, or
 // loading of cookies for a specfic domain key(eTLD+1). In the former case, the
@@ -204,64 +202,33 @@ class NET_EXPORT CookieMonster : public CookieStore {
   void DeleteCanonicalCookieAsync(const CanonicalCookie& cookie,
                                   const DeleteCookieCallback& callback);
 
-  // Flush the backing store (if any) to disk and post the given callback when
-  // done.
-  // 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(const base::Closure& callback);
-
   // Replaces all the cookies by |list|. This method does not flush the backend.
   void SetAllCookiesAsync(const CookieList& list,
                           const SetCookiesCallback& callback);
 
   // CookieStore implementation.
-
-  // Sets the cookies specified by |cookie_list| returned from |url|
-  // with options |options| in effect.
   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.
   void GetCookiesWithOptionsAsync(const GURL& url,
                                   const CookieOptions& options,
                                   const GetCookiesCallback& callback) override;
-
-  // Invokes GetAllCookiesForURLWithOptions with options set to include HTTP
-  // only cookies.
   void GetAllCookiesForURLAsync(const GURL& url,
                                 const GetCookieListCallback& callback) override;
-
-  // Deletes all cookies with that might apply to |url| that has |cookie_name|.
   void DeleteCookieAsync(const GURL& url,
                          const std::string& cookie_name,
                          const base::Closure& callback) override;
-
-  // 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.
   void DeleteAllCreatedBetweenAsync(const base::Time& delete_begin,
                                     const base::Time& delete_end,
                                     const DeleteCallback& callback) override;
-
-  // Deletes all of the cookies that match the host of the given URL
-  // regardless of path and that have a creation_date greater than or
-  // equal to |delete_begin| and less then |delete_end|. 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.
   void DeleteAllCreatedBetweenForHostAsync(
       const base::Time delete_begin,
       const base::Time delete_end,
       const GURL& url,
       const DeleteCallback& callback) override;
-
   void DeleteSessionCookiesAsync(const DeleteCallback&) override;
+  void FlushStore(const base::Closure& callback) override;
 
   CookieMonster* GetCookieMonster() override;
 
diff --git a/net/cookies/cookie_store.h b/net/cookies/cookie_store.h
index e2e2440..9d757f3 100644
--- a/net/cookies/cookie_store.h
+++ b/net/cookies/cookie_store.h
@@ -26,6 +26,10 @@ class CookieMonster;
 
 // An interface for storing and retrieving cookies. Implementations need to
 // be thread safe as its methods can be accessed from IO as well as UI threads.
+//
+// All async functions may either invoke the callback asynchronously on the same
+// thread, or they may be invoked immediately (prior to return of the
+// asynchronous function).
 class NET_EXPORT CookieStore : public base::RefCountedThreadSafe<CookieStore> {
  public:
   // Callback definitions.
@@ -39,7 +43,9 @@ class NET_EXPORT CookieStore : public base::RefCountedThreadSafe<CookieStore> {
       CookieChangedCallbackList;
   typedef CookieChangedCallbackList::Subscription CookieChangedSubscription;
 
-  // Sets a single cookie.  Expects a cookie line, like "a=1; domain=b.com".
+  // Sets the cookies specified by |cookie_list| returned from |url|
+  // with options |options| in effect.  Expects a cookie line, like
+  // "a=1; domain=b.com".
   //
   // Fails either if the cookie is invalid or if this is a non-HTTPONLY cookie
   // and it would overwrite an existing HTTPONLY cookie.
@@ -55,19 +61,23 @@ class NET_EXPORT CookieStore : public base::RefCountedThreadSafe<CookieStore> {
   // Note: Some sites, such as Facebook, occasionally use Cookie headers >4k.
   //
   // Simple interface, gets a cookie string "a=b; c=d" for the given URL.
-  // Use options to access httponly cookies.
+  // Gets all cookies that apply to |url| given |options|. Use options to
+  // access httponly cookies.
+  //
+  // The returned cookies are ordered by longest path, then earliest
+  // creation date.
   virtual void GetCookiesWithOptionsAsync(
       const GURL& url,
       const CookieOptions& options,
       const GetCookiesCallback& callback) = 0;
 
-  // Returns all matching cookies without marking them as accessed,
-  // including HTTP only cookies.
+  // Invokes GetAllCookiesForURLWithOptions with options set to include HTTP
+  // only cookies.
   virtual void GetAllCookiesForURLAsync(
       const GURL& url,
       const GetCookieListCallback& callback) = 0;
 
-  // Deletes the passed in cookie for the specified URL.
+  // Deletes all cookies that might apply to |url| that have |cookie_name|.
   virtual void DeleteCookieAsync(const GURL& url,
                                  const std::string& cookie_name,
                                  const base::Closure& callback) = 0;
@@ -93,6 +103,17 @@ class NET_EXPORT CookieStore : public base::RefCountedThreadSafe<CookieStore> {
 
   virtual void DeleteSessionCookiesAsync(const DeleteCallback&) = 0;
 
+  // Flush the backing store (if any) to disk and post the given callback when
+  // done.
+  // 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.
+  // TODO(mmenke):  Once this class is no longer thread-safe, this will always
+  // be invoked on the CookieStore's thread, and this comment can be removed.
+  // https://crbug.com/46185
+  virtual void FlushStore(const base::Closure& callback) = 0;
+
   // Returns the underlying CookieMonster.
   virtual CookieMonster* GetCookieMonster() = 0;
 
diff --git a/net/cookies/cookie_store_test_helpers.cc b/net/cookies/cookie_store_test_helpers.cc
index e75039c..b173332 100644
--- a/net/cookies/cookie_store_test_helpers.cc
+++ b/net/cookies/cookie_store_test_helpers.cc
@@ -150,6 +150,10 @@ void DelayedCookieMonster::DeleteSessionCookiesAsync(const DeleteCallback&) {
   ADD_FAILURE();
 }
 
+void DelayedCookieMonster::FlushStore(const base::Closure& callback) {
+  ADD_FAILURE();
+}
+
 CookieMonster* DelayedCookieMonster::GetCookieMonster() {
   return cookie_monster_.get();
 }
diff --git a/net/cookies/cookie_store_test_helpers.h b/net/cookies/cookie_store_test_helpers.h
index 1750c7f..64ae73d 100644
--- a/net/cookies/cookie_store_test_helpers.h
+++ b/net/cookies/cookie_store_test_helpers.h
@@ -63,6 +63,8 @@ class DelayedCookieMonster : public CookieStore {
 
   void DeleteSessionCookiesAsync(const DeleteCallback&) override;
 
+  void FlushStore(const base::Closure& callback) override;
+
   CookieMonster* GetCookieMonster() override;
 
   scoped_ptr<CookieStore::CookieChangedSubscription>