Add a method to override all the cookies in CookieMonster

net/cookies/cookie_monster.h

 // Copyright (c) 2012 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_COOKIES_COOKIE_MONSTER_H_
 #define NET_COOKIES_COOKIE_MONSTER_H_
 
 #include <deque>
@@ skipping 216 matching lines @@
   void SetForceKeepSessionState();
 
   // 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, and does not update the creation
+  // time nor the last access time: if a cookie in |list| already exists in the
+  // cookie monster with a different time, the cookie is left untouched in the
+  // cookie monster.

[droger, 2015/03/12 15:50:29]

I don't know if this behavior w.r.t. creation time and last access time makes
sense.
If you think it does not, I can check if it can be removed, I think it is here
mainly for performance reasons (to minimize the updates on the backend).

[erikwright (departed), 2015/03/13 13:59:01]

Presumably the rationale for not needing to update the last_access_time is that
expiry is being handled by the iOS cookie store anyways? Or because we are
handling the network requests and iOS is only handling the JS cookie APIs, and
thus we assume that anything JS is accessing has already been touched recently
by the network stack?

I'm not sure, off the top of my head, what creation_time is used for.

I can see an edge case where this matters. If JS in a page is setting many
cookies, we might not evict the right ones, as the last access ordering will not
be meaningful.

At a minimum I'd like more information about why this was considered "safe" and
whether there is any justification for why it is "necessary".

[droger, 2015/03/13 15:10:47]

Some context: on iOS when a WebView is attached to a request context, we switch
to the system cookie store as a source of truth (both for the net stack and for
javascript). When this happen, we periodically copy all the cookies to cookie
monster (so that cookies can be persisted on disk), using that new
SetAllCookiesAsync() function.

When web views are detached from the request context, the source of truth is
moved back to cookie monster again.

Now, to answer your question: the system store does not have a notion of "last
access", and so we just don't bother with them at all. When we convert NSHTTP
cookies to canonical cookies, we use an invalid time base::Time().

So far it seems that it's been working, although now that I read the code, I see
that it is used to evict cookies when a domain has too many. This means that the
cookie eviction is probably broken (depending on how the system store handles
eviction by itself). It is not clear if/how it can be fixed though:

it would be possible to keep track of the last access time from the network
stack, because we see the calls in CookieStoreIOS, but we can't do anything for
javascript access.
I don't exactly know either. It is at least used to uniquely identify cookies.
For example DeleteCanonicalCookie is based on that, and also the SQL database
completely crashes if two cookies share the same creation time.
It seemed that updating the creation time as part of this new
SetAllCookiesAsync() was not very useful, and it is unclear if we can support
that or not.

+  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|.
@@ skipping 68 matching lines @@
   class DeleteTask;
   class DeleteAllCreatedBetweenTask;
   class DeleteAllCreatedBetweenForHostTask;
   class DeleteAllForHostTask;
   class DeleteAllTask;
   class DeleteCookieTask;
   class DeleteCanonicalCookieTask;
   class GetAllCookiesForURLWithOptionsTask;
   class GetAllCookiesTask;
   class GetCookiesWithOptionsTask;
+  class SetAllCookiesTask;
   class SetCookieWithDetailsTask;
   class SetCookieWithOptionsTask;
   class DeleteSessionCookiesTask;
   class HasCookiesForETLDP1Task;
 
   // Testing support.
   // For SetCookieWithCreationTime.
   FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest,
                            TestCookieDeleteAllCreatedBetweenTimestamps);
   // For SetCookieWithCreationTime.
   FRIEND_TEST_ALL_PREFIXES(MultiThreadedCookieMonsterTest,
                            ThreadCheckDeleteAllCreatedBetweenForHost);
 
   // 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);
 
   // For validation of key values.
   FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest, TestDomainTree);
   FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest, TestImport);
   FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest, GetKey);
   FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest, TestGetKey);
 
   // For FindCookiesForKey.
   FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest, ShortLivedSessionCookies);
 
+  // For ComputeCookieDiff.
+  FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest, ComputeCookieDiff);
+
   // Internal reasons for deletion, used to populate informative histograms
   // and to provide a public cause for onCookieChange notifications.
   //
   // If you add or remove causes from this list, please be sure to also update
   // the CookieMonsterDelegate::ChangeCause mapping inside ChangeCauseMapping.
   // Moreover, these are used as array indexes, so avoid reordering to keep the
   // histogram buckets consistent. New items (if necessary) should be added
   // at the end of the list, just before DELETE_COOKIE_LAST_ENTRY.
   enum DeletionCause {
     DELETE_COOKIE_EXPLICIT = 0,
@@ skipping 174 matching lines @@
                                            const std::string& cookie_line,
                                            const base::Time& creation_time,
                                            const CookieOptions& options);
 
   // Helper function that sets a canonical cookie, deleting equivalents and
   // performing garbage collection.
   bool SetCanonicalCookie(scoped_ptr<CanonicalCookie>* cc,
                           const base::Time& creation_time,
                           const CookieOptions& options);
 
+  // Helper function calling SetCanonicalCookie() for all cookies in |list|.
+  bool SetCanonicalCookies(const CookieList& list);
+
   void InternalUpdateCookieAccessTime(CanonicalCookie* cc,
                                       const base::Time& current_time);
 
   // |deletion_cause| argument is used for collecting statistics and choosing
   // the correct CookieMonsterDelegate::ChangeCause for OnCookieChanged
   // notifications.  Guarantee: All iterators to cookies_ except to the
   // deleted entry remain vaild.
   void InternalDeleteCookie(CookieMap::iterator it,
                             bool sync_to_store,
                             DeletionCause deletion_cause);
@@ skipping 44 matching lines @@
 
   // Runs the task if, or defers the task until, the full cookie database is
   // loaded.
   void DoCookieTask(const scoped_refptr<CookieMonsterTask>& task_item);
 
   // Runs the task if, or defers the task until, the cookies for the given URL
   // are loaded.
   void DoCookieTaskForURL(const scoped_refptr<CookieMonsterTask>& task_item,
                           const GURL& url);
 
+  // Computes the difference between |old_cookies| and |new_cookies|, and writes
+  // the result in |cookies_to_add| and |cookies_to_delete|.
+  // This function has the side effect of changing the order of |old_cookies|
+  // and |new_cookies|. |cookies_to_add| and |cookies_to_delete| must be empty,
+  // and none of the arguments can be null.
+  // Creation time and last access time are ignored (cookies differing only by
+  // their time are considered identical).
+  void ComputeCookieDiff(CookieList* old_cookies,
+                         CookieList* new_cookies,
+                         CookieList* cookies_to_add,
+                         CookieList* cookies_to_delete);
+
   // Run all cookie changed callbacks that are monitoring |cookie|.
   // |removed| is true if the cookie was deleted.
   void RunCallbacks(const CanonicalCookie& cookie, bool removed);
 
   // Histogram variables; see CookieMonster::InitializeHistograms() in
   // cookie_monster.cc for details.
   base::HistogramBase* histogram_expiration_duration_minutes_;
   base::HistogramBase* histogram_between_access_interval_minutes_;
   base::HistogramBase* histogram_evicted_last_access_minutes_;
   base::HistogramBase* histogram_count_;
@@ skipping 151 matching lines @@
   virtual ~PersistentCookieStore() {}
 
  private:
   friend class base::RefCountedThreadSafe<PersistentCookieStore>;
   DISALLOW_COPY_AND_ASSIGN(PersistentCookieStore);
 };
 
 }  // namespace net
 
 #endif  // NET_COOKIES_COOKIE_MONSTER_H_