Simplify CookieMonster::SetAllCookies{,Async}() implementation.

net/cookies/cookie_monster.cc

 // 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.
 
 // Portions of this code based on Mozilla:
 //   (netwerk/cookie/src/nsCookieService.cpp)
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
  * The contents of this file are subject to the Mozilla Public License Version
@@ skipping 1470 matching lines @@
   // if a cookie was set, in the common case it will be used soon after,
   // and we will purge the expired cookies in GetCookies().
   GarbageCollect(creation_date, key);
 
   ConditionalCookieCallback(std::move(callback), true);
 }
 
 void CookieMonster::SetAllCookies(CookieList list,
                                   SetCookiesCallback callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  CookieList positive_diff;
-  CookieList negative_diff;
 
-  CookieList old_cookies;
-  old_cookies.reserve(cookies_.size());
-  for (const auto& cookie : cookies_)
-    old_cookies.push_back(*cookie.second.get());
-
-  ComputeCookieDiff(&old_cookies, &list, &positive_diff, &negative_diff);

[mmenke, 2017/07/12 18:56:35]

Is ComputeCookieDiff still used?  I'd check it myself, but cs and cs-staging
both have borked indices.  We clearly need cs-as-it-was-2-days-ago.

[Randy Smith (Not in Mondays), 2017/07/12 20:07:08]

Used by test code in order to test it, which I think can be nuked.  Removed.

-
-  for (const auto& cookie_to_delete : negative_diff) {
-    for (CookieMapItPair its =
-             cookies_.equal_range(GetKey(cookie_to_delete.Domain()));
-         its.first != its.second; ++its.first) {
-      // The creation date acts as the unique index...
-      if (its.first->second->CreationDate() ==
-          cookie_to_delete.CreationDate()) {
-        // TODO(rdsmith): DELETE_COOKIE_CANONICAL is incorrect and should
-        // be changed.
-        InternalDeleteCookie(its.first, true, DELETE_COOKIE_CANONICAL);
-        break;
-      }
-    }
+  // Nuke the existing store.
+  for (auto prev_it = cookies_.begin(), next_it = prev_it;
+       next_it != cookies_.end(); prev_it = next_it) {

[mmenke, 2017/07/12 18:56:35]

I think this can just be:

while (!cookies_.empty()) {
  InternalDeleteCookie(cookies_.begin(),....);
}

[Randy Smith (Not in Mondays), 2017/07/12 20:07:08]

You're right, that's quite a bit nicer.  Done.

+    ++next_it;
+    // TODO(rdsmith): The CANONICAL is a lie.
+    InternalDeleteCookie(prev_it, true, DELETE_COOKIE_CANONICAL);
   }
 
-  if (positive_diff.size() == 0) {
-    ConditionalCookieCallback(std::move(callback), true);
-    return;
-  }
-
+  // Set all passed in cookies.
   for (const auto& cookie : list) {
     const std::string key(GetKey(cookie.Domain()));
     Time creation_time = cookie.CreationDate();
-    bool already_expired = cookie.IsExpired(creation_time);
-
-    bool result =
-        DeleteAnyEquivalentCookie(key, cookie, true, false, already_expired);
-    DCHECK(!result);
-
-    if (already_expired)
+    if (cookie.IsExpired(creation_time))
       continue;
 
     if (cookie.IsPersistent()) {
       histogram_expiration_duration_minutes_->Add(
           (cookie.ExpiryDate() - creation_time).InMinutes());
     }
 
     InternalInsertCookie(key, base::MakeUnique<CanonicalCookie>(cookie), true);
     GarbageCollect(creation_time, key);
   }
@@ skipping 571 matching lines @@
        it != hook_map_.end(); ++it) {
     std::pair<GURL, std::string> key = it->first;
     if (cookie.IncludeForRequestURL(key.first, opts) &&
         cookie.Name() == key.second) {
       it->second->Notify(cookie, cause);
     }
   }
 }
 
 }  // namespace net