[CookieStore] Upgrading host-based deleting to predicate-based deleting.

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 573 matching lines @@
   Time delete_end_;
 
   DISALLOW_COPY_AND_ASSIGN(DeleteAllCreatedBetweenTask);
 };
 
 int CookieMonster::DeleteAllCreatedBetweenTask::RunDeleteTask() {
   return this->cookie_monster()->DeleteAllCreatedBetween(delete_begin_,
                                                          delete_end_);
 }
 
-// Task class for DeleteAllCreatedBetweenForHost call.
-class CookieMonster::DeleteAllCreatedBetweenForHostTask
+// Task class for DeleteAllCreatedBetweenWithPredicate call.
+class CookieMonster::DeleteAllCreatedBetweenWithPredicateTask
     : public DeleteTask<int> {
  public:
-  DeleteAllCreatedBetweenForHostTask(CookieMonster* cookie_monster,
-                                     Time delete_begin,
-                                     Time delete_end,
-                                     const GURL& url,
-                                     const DeleteCallback& callback)
+  DeleteAllCreatedBetweenWithPredicateTask(
+      CookieMonster* cookie_monster,
+      Time delete_begin,
+      Time delete_end,
+      base::Callback<bool(const CanonicalCookie&)> predicate,
+      const DeleteCallback& callback)
       : DeleteTask<int>(cookie_monster, callback),
         delete_begin_(delete_begin),
         delete_end_(delete_end),
-        url_(url) {}
+        predicate_(predicate) {}
 
   // DeleteTask:
   int RunDeleteTask() override;
 
  protected:
-  ~DeleteAllCreatedBetweenForHostTask() override {}
+  ~DeleteAllCreatedBetweenWithPredicateTask() override {}
 
  private:
   Time delete_begin_;
   Time delete_end_;
-  GURL url_;
+  base::Callback<bool(const CanonicalCookie&)> predicate_;
 
-  DISALLOW_COPY_AND_ASSIGN(DeleteAllCreatedBetweenForHostTask);
+  DISALLOW_COPY_AND_ASSIGN(DeleteAllCreatedBetweenWithPredicateTask);
 };
 
-int CookieMonster::DeleteAllCreatedBetweenForHostTask::RunDeleteTask() {
-  return this->cookie_monster()->DeleteAllCreatedBetweenForHost(
-      delete_begin_, delete_end_, url_);
+int CookieMonster::DeleteAllCreatedBetweenWithPredicateTask::RunDeleteTask() {
+  return this->cookie_monster()->DeleteAllCreatedBetweenWithPredicate(
+      delete_begin_, delete_end_, predicate_);
 }
 
 // Task class for DeleteCanonicalCookie call.
 class CookieMonster::DeleteCanonicalCookieTask : public DeleteTask<int> {
  public:
   DeleteCanonicalCookieTask(CookieMonster* cookie_monster,
                             const CanonicalCookie& cookie,
                             const DeleteCallback& callback)
       : DeleteTask<int>(cookie_monster, callback), cookie_(cookie) {}
 
@@ skipping 280 matching lines @@
 void CookieMonster::DeleteAllCreatedBetweenAsync(
     const Time& delete_begin,
     const Time& delete_end,
     const DeleteCallback& callback) {
   scoped_refptr<DeleteAllCreatedBetweenTask> task =
       new DeleteAllCreatedBetweenTask(this, delete_begin, delete_end, callback);
 
   DoCookieTask(task);
 }
 
-void CookieMonster::DeleteAllCreatedBetweenForHostAsync(
-    const Time delete_begin,
-    const Time delete_end,
-    const GURL& url,
+void CookieMonster::DeleteAllCreatedBetweenWithPredicateAsync(
+    const Time& delete_begin,
+    const Time& delete_end,
+    const base::Callback<bool(const CanonicalCookie&)>& predicate,
     const DeleteCallback& callback) {
-  scoped_refptr<DeleteAllCreatedBetweenForHostTask> task =
-      new DeleteAllCreatedBetweenForHostTask(this, delete_begin, delete_end,
-                                             url, callback);
-
-  DoCookieTaskForURL(task, url);
+  if (predicate.is_null()) {

[Mike West, 2016/04/01 07:40:52]

This seems strange. If no predicate is provided, it seems like nothing would
match. Why would you match everything instead?

Either way, it would be nice if you added a test for this behavior.

[dmurph, 2016/04/01 23:16:45]

Ah, true. I'll do it that way.

+    DeleteAllCreatedBetweenAsync(delete_begin, delete_end, callback);
+    return;
+  }
+  scoped_refptr<DeleteAllCreatedBetweenWithPredicateTask> task =
+      new DeleteAllCreatedBetweenWithPredicateTask(
+          this, delete_begin, delete_end, predicate, callback);
+  DoCookieTask(task);
 }
 
 void CookieMonster::DeleteSessionCookiesAsync(
     const CookieStore::DeleteCallback& callback) {
   scoped_refptr<DeleteSessionCookiesTask> task =
       new DeleteSessionCookiesTask(this, callback);
 
   DoCookieTask(task);
 }
 
@@ skipping 172 matching lines @@
         (delete_end.is_null() || cc->CreationDate() < delete_end)) {
       InternalDeleteCookie(curit, true, /*sync_to_store*/
                            DELETE_COOKIE_EXPLICIT);
       ++num_deleted;
     }
   }
 
   return num_deleted;
 }
 
-int CookieMonster::DeleteAllCreatedBetweenForHost(const Time delete_begin,
-                                                  const Time delete_end,
-                                                  const GURL& url) {
-  DCHECK(thread_checker_.CalledOnValidThread());
+int CookieMonster::DeleteAllCreatedBetweenWithPredicate(
+    const base::Time& delete_begin,
+    const base::Time& delete_end,
+    const base::Callback<bool(const CanonicalCookie&)>& predicate) {
+  int num_deleted = 0;
+  for (CookieMap::iterator it = cookies_.begin(); it != cookies_.end();) {
+    CookieMap::iterator curit = it;
+    CanonicalCookie* cc = curit->second;
+    ++it;
 
-  if (!HasCookieableScheme(url))
-    return 0;
-
-  const std::string host(url.host());
-
-  // We store host cookies in the store by their canonical host name;
-  // domain cookies are stored with a leading ".".  So this is a pretty
-  // simple lookup and per-cookie delete.
-  int num_deleted = 0;
-  for (CookieMapItPair its = cookies_.equal_range(GetKey(host));
-       its.first != its.second;) {
-    CookieMap::iterator curit = its.first;
-    ++its.first;
-
-    const CanonicalCookie* const cc = curit->second;
-
-    // Delete only on a match as a host cookie.
-    if (cc->IsHostCookie() && cc->IsDomainMatch(host) &&
-        cc->CreationDate() >= delete_begin &&
+    if (cc->CreationDate() >= delete_begin &&
         // The assumption that null |delete_end| is equivalent to
         // Time::Max() is confusing.
-        (delete_end.is_null() || cc->CreationDate() < delete_end)) {
-      num_deleted++;
-
-      InternalDeleteCookie(curit, true, DELETE_COOKIE_EXPLICIT);
+        (delete_end.is_null() || cc->CreationDate() < delete_end) &&

[Mike West, 2016/04/01 07:40:52]

Please add a test for the null behavior.

[dmurph, 2016/04/01 23:16:45]

Done.

+        predicate.Run(*cc)) {
+      InternalDeleteCookie(curit, true, /*sync_to_store*/
+                           DELETE_COOKIE_EXPLICIT);
+      ++num_deleted;
     }
   }
+
   return num_deleted;
 }
 
-
 bool CookieMonster::SetCookieWithOptions(const GURL& url,
                                          const std::string& cookie_line,
                                          const CookieOptions& options) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!HasCookieableScheme(url)) {
     return false;
   }
 
   return SetCookieWithCreationTimeAndOptions(url, cookie_line, Time(), options);
@@ skipping 1166 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, removed);
     }
   }
 }
 
 }  // namespace net