Bug 608545 - Set cookie: unexpected "deleted" observer notification when cookie already exists. r=sdwilsh, a=b7+
authorDan Witte <dwitte@mozilla.com>
Tue, 02 Nov 2010 00:06:54 -0700
changeset 56824 45d6a73ef138b3416ffcd22b0270bd27e855e1f1
parent 56823 c7c2a896d3237f1efb0d3ad93e31aca052bf0aa1
child 56825 255eeac8a15e5fc3839820c0edb16d76b10ff318
push id16691
push userdwitte@mozilla.com
push dateTue, 02 Nov 2010 07:07:05 +0000
treeherdermozilla-central@45d6a73ef138 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssdwilsh, b7
bugs608545
milestone2.0b8pre
first release with
nightly linux32
45d6a73ef138 / 4.0b8pre / 20101102025911 / files
nightly linux64
45d6a73ef138 / 4.0b8pre / 20101102030119 / files
nightly mac
45d6a73ef138 / 4.0b8pre / 20101102030737 / files
nightly win32
45d6a73ef138 / 4.0b8pre / 20101102042148 / files
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
Bug 608545 - Set cookie: unexpected "deleted" observer notification when cookie already exists. r=sdwilsh, a=b7+
netwerk/cookie/nsCookieService.cpp
netwerk/cookie/nsICookieService.idl
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -1309,17 +1309,17 @@ nsCookieService::NotifyRejected(nsIURI *
 }
 
 // notify observers that the cookie list changed. there are five possible
 // values for aData:
 // "deleted" means a cookie was deleted. aSubject is the deleted cookie.
 // "added"   means a cookie was added. aSubject is the added cookie.
 // "changed" means a cookie was altered. aSubject is the new cookie.
 // "cleared" means the entire cookie list was cleared. aSubject is null.
-// "batch-deleted" means multiple cookies were deleted. aSubject is the list of
+// "batch-deleted" means a set of cookies was purged. aSubject is the list of
 // cookies.
 void
 nsCookieService::NotifyChanged(nsISupports     *aSubject,
                                const PRUnichar *aData)
 {
   if (mObserverService)
     mObserverService->NotifyObservers(aSubject, "cookie-changed", aData);
 }
@@ -2329,44 +2329,46 @@ nsCookieService::AddInternal(const nsCSt
         // The new cookie has expired and the old one is stale. Nothing to do.
         COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, aCookieHeader,
           "cookie has already expired");
         return;
       }
 
       // Remove the stale cookie and notify.
       RemoveCookieFromList(matchIter);
-
       COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, aCookieHeader,
-        "stale cookie was deleted");
-      NotifyChanged(oldCookie, NS_LITERAL_STRING("deleted").get());
+        "stale cookie was purged");
+
+      nsCOMPtr<nsIMutableArray> removedList =
+        do_CreateInstance(NS_ARRAY_CONTRACTID);
+      removedList->AppendElement(oldCookie, PR_FALSE);
+      NotifyChanged(removedList, NS_LITERAL_STRING("batch-deleted").get());
 
       // We've done all we need to wrt removing and notifying the stale cookie.
       // From here on out, we pretend pretend it didn't exist, so that we
       // preserve expected notification semantics when adding the new cookie.
       foundCookie = PR_FALSE;
 
     } else {
       // If the old cookie is httponly, make sure we're not coming from script.
       if (!aFromHttp && oldCookie->IsHttpOnly()) {
         COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, aCookieHeader,
           "previously stored cookie is httponly; coming from script");
         return;
       }
 
-      // Remove the old cookie and notify.
+      // Remove the old cookie.
       RemoveCookieFromList(matchIter);
 
-      COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, aCookieHeader,
-        "previously stored cookie was deleted");
-      NotifyChanged(oldCookie, NS_LITERAL_STRING("deleted").get());
-
       // If the new cookie has expired -- i.e. the intent was simply to delete
       // the old cookie -- then we're done.
       if (aCookie->Expiry() <= currentTime) {
+        COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, aCookieHeader,
+          "previously stored cookie was deleted");
+        NotifyChanged(oldCookie, NS_LITERAL_STRING("deleted").get());
         return;
       }
 
       // Preserve creation time of cookie for ordering purposes.
       aCookie->SetCreationTime(oldCookie->CreationTime());
     }
 
   } else {
@@ -2377,22 +2379,26 @@ nsCookieService::AddInternal(const nsCSt
       return;
     }
 
     // check if we have to delete an old cookie.
     nsCookieEntry *entry = mDBState->hostTable.GetEntry(aBaseDomain);
     if (entry && entry->GetCookies().Length() >= mMaxCookiesPerHost) {
       nsListIter iter;
       FindStaleCookie(entry, currentTime, iter);
+      oldCookie = iter.Cookie();
 
       // remove the oldest cookie from the domain
-      COOKIE_LOGEVICTED(iter.Cookie(), "Too many cookies for this domain");
       RemoveCookieFromList(iter);
-
-      NotifyChanged(iter.Cookie(), NS_LITERAL_STRING("deleted").get());
+      COOKIE_LOGEVICTED(oldCookie, "Too many cookies for this domain");
+
+      nsCOMPtr<nsIMutableArray> removedList =
+        do_CreateInstance(NS_ARRAY_CONTRACTID);
+      removedList->AppendElement(oldCookie, PR_FALSE);
+      NotifyChanged(removedList, NS_LITERAL_STRING("batch-deleted").get());
 
     } else if (mDBState->cookieCount >= ADD_TEN_PERCENT(mMaxNumberOfCookies)) {
       PRInt64 maxAge = aCurrentTimeInUsec - mDBState->cookieOldestTime;
       PRInt64 purgeAge = ADD_TEN_PERCENT(mCookiePurgeAge);
       if (maxAge >= purgeAge) {
         // we're over both size and age limits by 10%; time to purge the table!
         // do this by:
         // 1) removing expired cookies;
--- a/netwerk/cookie/nsICookieService.idl
+++ b/netwerk/cookie/nsICookieService.idl
@@ -61,19 +61,20 @@ interface nsIChannel;
  *          "added"
  *          a cookie was added. the subject is an nsICookie2 representing
  *          the added cookie.
  *          "changed"
  *          a cookie was changed. the subject is an nsICookie2 representing
  *          the new cookie. (note that host, path, and name are invariant
  *          for a given cookie; other parameters may change.)
  *          "batch-deleted"
- *          a batch of cookies was deleted (for instance, as part of a purging
- *          operation). the subject is an nsIArray of nsICookie2's representing
- *          the deleted cookies.
+ *          a set of cookies was purged (typically, because they have either
+ *          expired or because the cookie list has grown too large). The subject
+ *          is an nsIArray of nsICookie2's representing the deleted cookies.
+ *          Note that the array could contain a single cookie.
  *          "cleared"
  *          the entire cookie list was cleared. the subject is null.
  *          "reload"
  *          the entire cookie list should be reloaded.  the subject is null.
  *
  * topic  : "cookie-rejected"
  *          broadcast whenever a cookie was rejected from being set as a
  *          result of user prefs.