Adding 'explicit_expiration' cause to 'OnCookieChanged'

BUG=77298
TEST=browser_tests

Review URL: http://codereview.chromium.org/6730049

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@79802 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 22 insertions, 8 deletions

diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc
index 09bee76..b850bbe 100644
--- a/net/base/cookie_monster.cc
+++ b/net/base/cookie_monster.cc
@@ -376,6 +376,8 @@ ChangeCausePair ChangeCauseMapping[] = {
   { CookieMonster::Delegate::CHANGE_COOKIE_EVICTED, true },
   // DELETE_COOKIE_EVICTED_DOMAIN_POST_SAFE
   { CookieMonster::Delegate::CHANGE_COOKIE_EVICTED, true },
+  // DELETE_COOKIE_EXPIRED_OVERWRITE
+  { CookieMonster::Delegate::CHANGE_COOKIE_EXPIRED_OVERWRITE, true },
   // DELETE_COOKIE_LAST_ENTRY
   { CookieMonster::Delegate::CHANGE_COOKIE_EXPLICIT, false }
 };
@@ -1239,7 +1241,8 @@ void CookieMonster::FindCookiesForKey(
 
 bool CookieMonster::DeleteAnyEquivalentCookie(const std::string& key,
                                               const CanonicalCookie& ecc,
-                                              bool skip_httponly) {
+                                              bool skip_httponly,
+                                              bool already_expired) {
   lock_.AssertAcquired();
   ValidateMapWhileLockHeld(0);
 
@@ -1259,7 +1262,8 @@ bool CookieMonster::DeleteAnyEquivalentCookie(const std::string& key,
       if (skip_httponly && cc->IsHttpOnly()) {
         skipped_httponly = true;
       } else {
-        InternalDeleteCookie(curit, true, DELETE_COOKIE_OVERWRITE);
+        InternalDeleteCookie(curit, true, already_expired ?
+            DELETE_COOKIE_EXPIRED_OVERWRITE : DELETE_COOKIE_OVERWRITE);
       }
       found_equivalent_cookie = true;
     }
@@ -1338,7 +1342,9 @@ bool CookieMonster::SetCanonicalCookie(scoped_ptr<CanonicalCookie>* cc,
                                        const Time& creation_time,
                                        const CookieOptions& options) {
   const std::string key(GetKey((*cc)->Domain()));
-  if (DeleteAnyEquivalentCookie(key, **cc, options.exclude_httponly())) {
+  bool already_expired = (*cc)->IsExpired(creation_time);
+  if (DeleteAnyEquivalentCookie(key, **cc, options.exclude_httponly(),
+                                already_expired)) {
     VLOG(kVlogSetCookies) << "SetCookie() not clobbering httponly cookie";
     return false;
   }
@@ -1348,7 +1354,7 @@ bool CookieMonster::SetCanonicalCookie(scoped_ptr<CanonicalCookie>* cc,
 
   // Realize that we might be setting an expired cookie, and the only point
   // was to delete the cookie which we've already done.
-  if (!(*cc)->IsExpired(creation_time) || keep_expired_cookies_) {
+  if (!already_expired || keep_expired_cookies_) {
     // See InitializeHistograms() for details.
     if ((*cc)->DoesExpire()) {
       histogram_expiration_duration_minutes_->Add(
diff --git a/net/base/cookie_monster.h b/net/base/cookie_monster.h
index 440b38a..869b0f5 100644
--- a/net/base/cookie_monster.h
+++ b/net/base/cookie_monster.h
@@ -270,9 +270,10 @@ class CookieMonster : public CookieStore {
   // 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 Delegate::ChangeCause mapping inside InternalDeleteCookie. Moreover,
-  // these are used as array indexes, so please avoid reordering without good
-  // reason.
+  // the Delegate::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,
     DELETE_COOKIE_OVERWRITE,
@@ -292,6 +293,10 @@ class CookieMonster : public CookieStore {
     // (and thus would have been preserved by global garbage collection).
     DELETE_COOKIE_EVICTED_DOMAIN_POST_SAFE,
 
+    // A common idiom is to remove a cookie by overwriting it with an
+    // already-expired expiration date. This captures that case.
+    DELETE_COOKIE_EXPIRED_OVERWRITE,
+
     DELETE_COOKIE_LAST_ENTRY
   };
 
@@ -393,7 +398,8 @@ class CookieMonster : public CookieStore {
   // NOTE: There should never be more than a single matching equivalent cookie.
   bool DeleteAnyEquivalentCookie(const std::string& key,
                                  const CanonicalCookie& ecc,
-                                 bool skip_httponly);
+                                 bool skip_httponly,
+                                 bool already_expired);
 
   // Takes ownership of *cc.
   void InternalInsertCookie(const std::string& key,
@@ -653,6 +659,8 @@ class CookieMonster::Delegate
     CHANGE_COOKIE_EXPIRED,
     // The cookie was automatically evicted during garbage collection.
     CHANGE_COOKIE_EVICTED,
+    // The cookie was overwritten with an already-expired expiration date.
+    CHANGE_COOKIE_EXPIRED_OVERWRITE
   };
 
   // Will be called when a cookie is added or removed. The function is passed