Bug 1182961 (part 5) - Use nsTHashtable::Iterator in nsCookieService. r=michal.
authorNicholas Nethercote <nnethercote@mozilla.com>
Sun, 26 Jul 2015 23:40:51 -0700
changeset 287929 bcf6fd3ab9bb82279a07416fb0142573e7c6c4ba
parent 287928 6da154b43265058c8020291161fff10c5cc2ff78
child 287930 125b87d6530313f6fe176045dc4dd77ea94e20dd
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmichal
bugs1182961
milestone42.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1182961 (part 5) - Use nsTHashtable::Iterator in nsCookieService. r=michal.
netwerk/cookie/nsCookieService.cpp
netwerk/cookie/nsCookieService.h
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -1437,35 +1437,16 @@ nsCookieService::HandleCorruptDB(DBState
       mDefaultDBState->dbConn->AsyncClose(mDefaultDBState->closeListener);
     }
     CleanupDefaultDBConnection();
     break;
   }
   }
 }
 
-static PLDHashOperator
-RebuildDBCallback(nsCookieEntry *aEntry,
-                  void          *aArg)
-{
-  mozIStorageBindingParamsArray* paramsArray =
-    static_cast<mozIStorageBindingParamsArray*>(aArg);
-
-  const nsCookieEntry::ArrayType &cookies = aEntry->GetCookies();
-  for (nsCookieEntry::IndexType i = 0; i < cookies.Length(); ++i) {
-    nsCookie* cookie = cookies[i];
-
-    if (!cookie->IsSession()) {
-      bindCookieParameters(paramsArray, nsCookieKey(aEntry), cookie);
-    }
-  }
-
-  return PL_DHASH_NEXT;
-}
-
 void
 nsCookieService::RebuildCorruptDB(DBState* aDBState)
 {
   NS_ASSERTION(!aDBState->dbConn, "shouldn't have an open db connection");
   NS_ASSERTION(aDBState->corruptFlag == DBState::CLOSING_FOR_REBUILD,
     "should be in CLOSING_FOR_REBUILD state");
 
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
@@ -1509,17 +1490,28 @@ nsCookieService::RebuildCorruptDB(DBStat
   if (os) {
     os->NotifyObservers(nullptr, "cookie-db-rebuilding", nullptr);
   }
 
   // Enumerate the hash, and add cookies to the params array.
   mozIStorageAsyncStatement* stmt = aDBState->stmtInsert;
   nsCOMPtr<mozIStorageBindingParamsArray> paramsArray;
   stmt->NewBindingParamsArray(getter_AddRefs(paramsArray));
-  aDBState->hostTable.EnumerateEntries(RebuildDBCallback, paramsArray.get());
+  for (auto iter = aDBState->hostTable.Iter(); !iter.Done(); iter.Next()) {
+    nsCookieEntry* entry = iter.Get();
+
+    const nsCookieEntry::ArrayType& cookies = entry->GetCookies();
+    for (nsCookieEntry::IndexType i = 0; i < cookies.Length(); ++i) {
+      nsCookie* cookie = cookies[i];
+
+      if (!cookie->IsSession()) {
+        bindCookieParameters(paramsArray, nsCookieKey(entry), cookie);
+      }
+    }
+  }
 
   // Make sure we've got something to write. If we don't, we're done.
   uint32_t length;
   paramsArray->GetLength(&length);
   if (length == 0) {
     COOKIE_LOGSTRING(LogLevel::Debug,
       ("RebuildCorruptDB(): nothing to write, rebuild complete"));
     mDefaultDBState->corruptFlag = DBState::OK;
@@ -1948,42 +1940,33 @@ nsCookieService::RemoveAll()
       HandleCorruptDB(mDefaultDBState);
     }
   }
 
   NotifyChanged(nullptr, MOZ_UTF16("cleared"));
   return NS_OK;
 }
 
-static PLDHashOperator
-COMArrayCallback(nsCookieEntry *aEntry,
-                 void          *aArg)
-{
-  nsCOMArray<nsICookie> *data = static_cast<nsCOMArray<nsICookie> *>(aArg);
-
-  const nsCookieEntry::ArrayType &cookies = aEntry->GetCookies();
-  for (nsCookieEntry::IndexType i = 0; i < cookies.Length(); ++i) {
-    data->AppendObject(cookies[i]);
-  }
-
-  return PL_DHASH_NEXT;
-}
-
 NS_IMETHODIMP
 nsCookieService::GetEnumerator(nsISimpleEnumerator **aEnumerator)
 {
   if (!mDBState) {
     NS_WARNING("No DBState! Profile already closed?");
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   EnsureReadComplete();
 
   nsCOMArray<nsICookie> cookieList(mDBState->cookieCount);
-  mDBState->hostTable.EnumerateEntries(COMArrayCallback, &cookieList);
+  for (auto iter = mDBState->hostTable.Iter(); !iter.Done(); iter.Next()) {
+    const nsCookieEntry::ArrayType& cookies = iter.Get()->GetCookies();
+    for (nsCookieEntry::IndexType i = 0; i < cookies.Length(); ++i) {
+      cookieList.AppendObject(cookies[i]);
+    }
+  }
 
   return NS_NewArrayEnumerator(aEnumerator, cookieList);
 }
 
 NS_IMETHODIMP
 nsCookieService::Add(const nsACString &aHost,
                      const nsACString &aPath,
                      const nsACString &aName,
@@ -3675,55 +3658,16 @@ nsCookieService::RemoveAllFromMemory()
 {
   // clearing the hashtable will call each nsCookieEntry's dtor,
   // which releases all their respective children.
   mDBState->hostTable.Clear();
   mDBState->cookieCount = 0;
   mDBState->cookieOldestTime = INT64_MAX;
 }
 
-// stores temporary data for enumerating over the hash entries,
-// since enumeration is done using callback functions
-struct nsPurgeData
-{
-  typedef nsTArray<nsListIter> ArrayType;
-
-  nsPurgeData(int64_t aCurrentTime,
-              int64_t aPurgeTime,
-              ArrayType &aPurgeList,
-              nsIMutableArray *aRemovedList,
-              mozIStorageBindingParamsArray *aParamsArray)
-   : currentTime(aCurrentTime)
-   , purgeTime(aPurgeTime)
-   , oldestTime(INT64_MAX)
-   , purgeList(aPurgeList)
-   , removedList(aRemovedList)
-   , paramsArray(aParamsArray)
-  {
-  }
-
-  // the current time, in seconds
-  int64_t currentTime;
-
-  // lastAccessed time older than which cookies are eligible for purge
-  int64_t purgeTime;
-
-  // lastAccessed time of the oldest cookie found during purge, to update our indicator
-  int64_t oldestTime;
-
-  // list of cookies over the age limit, for purging
-  ArrayType &purgeList;
-
-  // list of all cookies we've removed, for notification
-  nsIMutableArray *removedList;
-
-  // The array of parameters to be bound to the statement for deletion later.
-  mozIStorageBindingParamsArray *paramsArray;
-};
-
 // comparator class for lastaccessed times of cookies.
 class CompareCookiesByAge {
 public:
   bool Equals(const nsListIter &a, const nsListIter &b) const
   {
     return a.Cookie()->LastAccessed() == b.Cookie()->LastAccessed() &&
            a.Cookie()->CreationTime() == b.Cookie()->CreationTime();
   }
@@ -3754,101 +3698,97 @@ public:
     // compare by entryclass pointer, then by index.
     if (a.entry != b.entry)
       return a.entry < b.entry;
 
     return a.index < b.index;
   }
 };
 
-PLDHashOperator
-purgeCookiesCallback(nsCookieEntry *aEntry,
-                     void          *aArg)
-{
-  nsPurgeData &data = *static_cast<nsPurgeData*>(aArg);
-
-  const nsCookieEntry::ArrayType &cookies = aEntry->GetCookies();
-  mozIStorageBindingParamsArray *array = data.paramsArray;
-  for (nsCookieEntry::IndexType i = 0; i < cookies.Length(); ) {
-    nsListIter iter(aEntry, i);
-    nsCookie *cookie = cookies[i];
-
-    // check if the cookie has expired
-    if (cookie->Expiry() <= data.currentTime) {
-      data.removedList->AppendElement(cookie, false);
-      COOKIE_LOGEVICTED(cookie, "Cookie expired");
-
-      // remove from list; do not increment our iterator
-      gCookieService->RemoveCookieFromList(iter, array);
-
-    } else {
-      // check if the cookie is over the age limit
-      if (cookie->LastAccessed() <= data.purgeTime) {
-        data.purgeList.AppendElement(iter);
-
-      } else if (cookie->LastAccessed() < data.oldestTime) {
-        // reset our indicator
-        data.oldestTime = cookie->LastAccessed();
-      }
-
-      ++i;
-    }
-  }
-  return PL_DHASH_NEXT;
-}
-
 // purges expired and old cookies in a batch operation.
 already_AddRefed<nsIArray>
 nsCookieService::PurgeCookies(int64_t aCurrentTimeInUsec)
 {
   NS_ASSERTION(mDBState->hostTable.Count() > 0, "table is empty");
   EnsureReadComplete();
 
   uint32_t initialCookieCount = mDBState->cookieCount;
   COOKIE_LOGSTRING(LogLevel::Debug,
     ("PurgeCookies(): beginning purge with %ld cookies and %lld oldest age",
      mDBState->cookieCount, aCurrentTimeInUsec - mDBState->cookieOldestTime));
 
-  nsAutoTArray<nsListIter, kMaxNumberOfCookies> purgeList;
+  typedef nsAutoTArray<nsListIter, kMaxNumberOfCookies> PurgeList;
+  PurgeList purgeList;
 
   nsCOMPtr<nsIMutableArray> removedList = do_CreateInstance(NS_ARRAY_CONTRACTID);
 
   // Create a params array to batch the removals. This is OK here because
   // all the removals are in order, and there are no interleaved additions.
   mozIStorageAsyncStatement *stmt = mDBState->stmtDelete;
   nsCOMPtr<mozIStorageBindingParamsArray> paramsArray;
   if (mDBState->dbConn) {
     stmt->NewBindingParamsArray(getter_AddRefs(paramsArray));
   }
 
-  nsPurgeData data(aCurrentTimeInUsec / PR_USEC_PER_SEC,
-    aCurrentTimeInUsec - mCookiePurgeAge, purgeList, removedList, paramsArray);
-  mDBState->hostTable.EnumerateEntries(purgeCookiesCallback, &data);
+  int64_t currentTime = aCurrentTimeInUsec / PR_USEC_PER_SEC;
+  int64_t purgeTime = aCurrentTimeInUsec - mCookiePurgeAge;
+  int64_t oldestTime = INT64_MAX;
+
+  for (auto iter = mDBState->hostTable.Iter(); !iter.Done(); iter.Next()) {
+    nsCookieEntry* entry = iter.Get();
+
+    const nsCookieEntry::ArrayType &cookies = entry->GetCookies();
+    for (nsCookieEntry::IndexType i = 0; i < cookies.Length(); ) {
+      nsListIter iter(entry, i);
+      nsCookie* cookie = cookies[i];
+
+      // check if the cookie has expired
+      if (cookie->Expiry() <= currentTime) {
+        removedList->AppendElement(cookie, false);
+        COOKIE_LOGEVICTED(cookie, "Cookie expired");
+
+        // remove from list; do not increment our iterator
+        gCookieService->RemoveCookieFromList(iter, paramsArray);
+
+      } else {
+        // check if the cookie is over the age limit
+        if (cookie->LastAccessed() <= purgeTime) {
+          purgeList.AppendElement(iter);
+
+        } else if (cookie->LastAccessed() < oldestTime) {
+          // reset our indicator
+          oldestTime = cookie->LastAccessed();
+        }
+
+        ++i;
+      }
+    }
+  }
 
   uint32_t postExpiryCookieCount = mDBState->cookieCount;
 
   // now we have a list of iterators for cookies over the age limit.
   // sort them by age, and then we'll see how many to remove...
   purgeList.Sort(CompareCookiesByAge());
 
   // only remove old cookies until we reach the max cookie limit, no more.
   uint32_t excess = mDBState->cookieCount > mMaxNumberOfCookies ?
     mDBState->cookieCount - mMaxNumberOfCookies : 0;
   if (purgeList.Length() > excess) {
     // We're not purging everything in the list, so update our indicator.
-    data.oldestTime = purgeList[excess].Cookie()->LastAccessed();
+    oldestTime = purgeList[excess].Cookie()->LastAccessed();
 
     purgeList.SetLength(excess);
   }
 
   // sort the list again, this time grouping cookies with a common entryclass
   // together, and with ascending index. this allows us to iterate backwards
   // over the list removing cookies, without having to adjust indexes as we go.
   purgeList.Sort(CompareCookiesByIndex());
-  for (nsPurgeData::ArrayType::index_type i = purgeList.Length(); i--; ) {
+  for (PurgeList::index_type i = purgeList.Length(); i--; ) {
     nsCookie *cookie = purgeList[i].Cookie();
     removedList->AppendElement(cookie, false);
     COOKIE_LOGEVICTED(cookie, "Cookie too old");
 
     RemoveCookieFromList(purgeList[i], paramsArray);
   }
 
   // Update the database if we have entries to purge.
@@ -3860,17 +3800,17 @@ nsCookieService::PurgeCookies(int64_t aC
       NS_ASSERT_SUCCESS(rv);
       nsCOMPtr<mozIStoragePendingStatement> handle;
       rv = stmt->ExecuteAsync(mDBState->removeListener, getter_AddRefs(handle));
       NS_ASSERT_SUCCESS(rv);
     }
   }
 
   // reset the oldest time indicator
-  mDBState->cookieOldestTime = data.oldestTime;
+  mDBState->cookieOldestTime = oldestTime;
 
   COOKIE_LOGSTRING(LogLevel::Debug,
     ("PurgeCookies(): %ld expired; %ld purged; %ld remain; %lld oldest age",
      initialCookieCount - postExpiryCookieCount,
      postExpiryCookieCount - mDBState->cookieCount,
      mDBState->cookieCount,
      aCurrentTimeInUsec - mDBState->cookieOldestTime));
 
@@ -3997,72 +3937,44 @@ nsCookieService::GetCookiesFromHost(cons
   const nsCookieEntry::ArrayType &cookies = entry->GetCookies();
   for (nsCookieEntry::IndexType i = 0; i < cookies.Length(); ++i) {
     cookieList.AppendObject(cookies[i]);
   }
 
   return NS_NewArrayEnumerator(aEnumerator, cookieList);
 }
 
-namespace {
-
-/**
- * This structure is used as a in/out parameter when enumerating the cookies
- * for an app.
- * It will contain the app id and onlyBrowserElement flag information as input
- * and will contain the array of matching cookies as output.
- */
-struct GetCookiesForAppStruct {
-  uint32_t              appId;
-  bool                  onlyBrowserElement;
-  nsCOMArray<nsICookie> cookies;
-
-  GetCookiesForAppStruct() = delete;
-  GetCookiesForAppStruct(uint32_t aAppId, bool aOnlyBrowserElement)
-    : appId(aAppId)
-    , onlyBrowserElement(aOnlyBrowserElement)
-  {}
-};
-
-} // namespace
-
-/* static */ PLDHashOperator
-nsCookieService::GetCookiesForApp(nsCookieEntry* entry, void* arg)
-{
-  GetCookiesForAppStruct* data = static_cast<GetCookiesForAppStruct*>(arg);
-
-  if (entry->mAppId != data->appId ||
-      (data->onlyBrowserElement && !entry->mInBrowserElement)) {
-    return PL_DHASH_NEXT;
-  }
-
-  const nsCookieEntry::ArrayType& cookies = entry->GetCookies();
-
-  for (nsCookieEntry::IndexType i = 0; i < cookies.Length(); ++i) {
-    data->cookies.AppendObject(cookies[i]);
-  }
-
-  return PL_DHASH_NEXT;
-}
-
 NS_IMETHODIMP
 nsCookieService::GetCookiesForApp(uint32_t aAppId, bool aOnlyBrowserElement,
                                   nsISimpleEnumerator** aEnumerator)
 {
   if (!mDBState) {
     NS_WARNING("No DBState! Profile already closed?");
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   NS_ENSURE_TRUE(aAppId != NECKO_UNKNOWN_APP_ID, NS_ERROR_INVALID_ARG);
 
-  GetCookiesForAppStruct data(aAppId, aOnlyBrowserElement);
-  mDBState->hostTable.EnumerateEntries(GetCookiesForApp, &data);
-
-  return NS_NewArrayEnumerator(aEnumerator, data.cookies);
+  nsCOMArray<nsICookie> cookies;
+  for (auto iter = mDBState->hostTable.Iter(); !iter.Done(); iter.Next()) {
+    nsCookieEntry* entry = iter.Get();
+
+    if (entry->mAppId != aAppId ||
+        (aOnlyBrowserElement && !entry->mInBrowserElement)) {
+      continue;
+    }
+
+    const nsCookieEntry::ArrayType& entryCookies = entry->GetCookies();
+
+    for (nsCookieEntry::IndexType i = 0; i < entryCookies.Length(); ++i) {
+      cookies.AppendObject(entryCookies[i]);
+    }
+  }
+
+  return NS_NewArrayEnumerator(aEnumerator, cookies);
 }
 
 NS_IMETHODIMP
 nsCookieService::RemoveCookiesForApp(uint32_t aAppId, bool aOnlyBrowserElement)
 {
   nsCOMPtr<nsISimpleEnumerator> enumerator;
   nsresult rv = GetCookiesForApp(aAppId, aOnlyBrowserElement,
                                  getter_AddRefs(enumerator));
--- a/netwerk/cookie/nsCookieService.h
+++ b/netwerk/cookie/nsCookieService.h
@@ -312,22 +312,16 @@ class nsCookieService final : public nsI
     void                          NotifyRejected(nsIURI *aHostURI);
     void                          NotifyThirdParty(nsIURI *aHostURI, bool aAccepted, nsIChannel *aChannel);
     void                          NotifyChanged(nsISupports *aSubject, const char16_t *aData);
     void                          NotifyPurged(nsICookie2* aCookie);
     already_AddRefed<nsIArray>    CreatePurgeList(nsICookie2* aCookie);
     void                          UpdateCookieOldestTime(DBState* aDBState, nsCookie* aCookie);
 
     /**
-     * This method is used to iterate the cookie hash table and select the ones
-     * that are part of a specific app.
-     */
-    static PLDHashOperator GetCookiesForApp(nsCookieEntry* entry, void* arg);
-
-    /**
      * This method is a helper that allows calling nsICookieManager::Remove()
      * with appId/inBrowserElement parameters.
      * NOTE: this could be added to a public interface if we happen to need it.
      */
     nsresult Remove(const nsACString& aHost, uint32_t aAppId,
                     bool aInBrowserElement, const nsACString& aName,
                     const nsACString& aPath, bool aBlocked);
 
@@ -351,17 +345,16 @@ class nsCookieService final : public nsI
     // cached prefs
     uint8_t                       mCookieBehavior; // BEHAVIOR_{ACCEPT, REJECTFOREIGN, REJECT, LIMITFOREIGN}
     bool                          mThirdPartySession;
     uint16_t                      mMaxNumberOfCookies;
     uint16_t                      mMaxCookiesPerHost;
     int64_t                       mCookiePurgeAge;
 
     // friends!
-    friend PLDHashOperator purgeCookiesCallback(nsCookieEntry *aEntry, void *aArg);
     friend class DBListenerErrorHandler;
     friend class ReadCookieDBListener;
     friend class CloseCookieDBListener;
 
     static nsCookieService*       GetSingleton();
     friend class mozilla::net::CookieServiceParent;
 };