Bug 572223 - too much cookies.sqlite io. Part 7: fix creationid logic. r=sdwilsh
authorDan Witte <dwitte@mozilla.com>
Mon, 02 Aug 2010 17:03:24 -0700
changeset 48748 9fcd4bc10113bf181343610a37cd08d5ec7e1edd
parent 48747 2544af6f8f10125b596e217718f2540122c6c29a
child 48749 32a8a1f6c34ad1f1f726ebdf814a32078713c076
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewerssdwilsh
bugs572223
milestone2.0b3pre
Bug 572223 - too much cookies.sqlite io. Part 7: fix creationid logic. r=sdwilsh
netwerk/cookie/nsCookie.cpp
netwerk/cookie/nsCookie.h
netwerk/cookie/nsCookieService.cpp
--- a/netwerk/cookie/nsCookie.cpp
+++ b/netwerk/cookie/nsCookie.cpp
@@ -78,16 +78,30 @@ StrBlockCopy(const nsACString &aSource1,
 // This is a counter that keeps track of the last used creation id, each time we
 // create a new nsCookie. The creation id is nominally the time (in microseconds)
 // the cookie was created. This id also corresponds to the row id used in the
 // sqlite database, which must be unique. However, since it's possible two cookies
 // may be created at the same time, or the system clock isn't monotonic, we must
 // check each id to enforce monotonicity.
 static PRInt64 gLastCreationID;
 
+PRInt64
+nsCookie::GenerateCreationID(PRInt64 aCreationTime)
+{
+  // Check if the creation time given to us is greater than the running maximum
+  // (it should always be monotonically increasing).
+  if (aCreationTime > gLastCreationID) {
+    gLastCreationID = aCreationTime;
+    return aCreationTime;
+  }
+
+  // Make up our own.
+  return ++gLastCreationID;
+}
+
 nsCookie *
 nsCookie::Create(const nsACString &aName,
                  const nsACString &aValue,
                  const nsACString &aHost,
                  const nsACString &aPath,
                  PRInt64           aExpiry,
                  PRInt64           aLastAccessed,
                  PRInt64           aCreationID,
@@ -106,22 +120,19 @@ nsCookie::Create(const nsACString &aName
     return nsnull;
 
   // assign string members
   char *name, *value, *host, *path, *end;
   name = static_cast<char *>(place) + sizeof(nsCookie);
   StrBlockCopy(aName, aValue, aHost, aPath,
                name, value, host, path, end);
 
-  // check if the creation id given to us is greater than the running maximum
-  // (it should always be monotonically increasing). if it's not, make up our own.
+  // If the creationID given to us is higher than the running maximum, update it.
   if (aCreationID > gLastCreationID)
     gLastCreationID = aCreationID;
-  else
-    aCreationID = ++gLastCreationID;
 
   // construct the cookie. placement new, oh yeah!
   return new (place) nsCookie(name, value, host, path, end,
                               aExpiry, aLastAccessed, aCreationID,
                               aIsSession, aIsSecure, aIsHttpOnly);
 }
 
 /******************************************************************************
--- a/netwerk/cookie/nsCookie.h
+++ b/netwerk/cookie/nsCookie.h
@@ -85,16 +85,21 @@ class nsCookie : public nsICookie2
      , mCreationID(aCreationID)
      , mIsSession(aIsSession != PR_FALSE)
      , mIsSecure(aIsSecure != PR_FALSE)
      , mIsHttpOnly(aIsHttpOnly != PR_FALSE)
     {
     }
 
   public:
+    // Generate a unique creationID. This will usually be the same as the
+    // creation time, but with a guarantee of monotonicity such that it can
+    // be used as a sqlite rowid.
+    static PRInt64 GenerateCreationID(PRInt64 aCreationTime);
+
     // public helper to create an nsCookie object. use |operator delete|
     // to destroy an object created by this method.
     static nsCookie * Create(const nsACString &aName,
                              const nsACString &aValue,
                              const nsACString &aHost,
                              const nsACString &aPath,
                              PRInt64           aExpiry,
                              PRInt64           aLastAccessed,
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -847,16 +847,17 @@ nsCookieService::TryInitDB(PRBool aDelet
           "SELECT "
             "id, "
             "baseDomain, "
             "name, "
             "value, "
             "host, "
             "path, "
             "expiry, "
+            "lastAccessed, "
             "isSecure, "
             "isHttpOnly "
           "FROM moz_cookies"), getter_AddRefs(stmt));
         if (NS_SUCCEEDED(rv))
           break;
 
         // our columns aren't there - drop the table!
         rv = mDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING("DROP TABLE moz_cookies"));
@@ -1332,17 +1333,17 @@ nsCookieService::Add(const nsACString &a
   NS_ENSURE_SUCCESS(rv, rv);
 
   PRInt64 currentTimeInUsec = PR_Now();
 
   nsRefPtr<nsCookie> cookie =
     nsCookie::Create(aName, aValue, host, aPath,
                      aExpiry,
                      currentTimeInUsec,
-                     currentTimeInUsec,
+                     nsCookie::GenerateCreationID(currentTimeInUsec),
                      aIsSession,
                      aIsSecure,
                      aIsHttpOnly);
   if (!cookie) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   AddInternal(baseDomain, cookie, currentTimeInUsec, nsnull, nsnull, PR_TRUE);
@@ -1449,17 +1450,18 @@ nsCookieService::GetCookieFromRow(T &aRo
   rv = aRow->GetUTF8String(4, path);
   NS_ASSERT_SUCCESS(rv);
 
   PRInt64 expiry = aRow->AsInt64(5);
   PRInt64 lastAccessed = aRow->AsInt64(6);
   PRBool isSecure = 0 != aRow->AsInt32(7);
   PRBool isHttpOnly = 0 != aRow->AsInt32(8);
 
-  // create a new nsCookie and assign the data.
+  // Create a new nsCookie and assign the data. We are guaranteed that the
+  // creationID is unique, since we're reading it from the db itself.
   return nsCookie::Create(name, value, host, path,
                           expiry,
                           lastAccessed,
                           creationID,
                           PR_FALSE,
                           isSecure,
                           isHttpOnly);
 }
@@ -1779,28 +1781,27 @@ nsCookieService::ImportCookies(nsIFile *
       continue;
     }
 
     // compute the baseDomain from the host
     rv = GetBaseDomainFromHost(host, baseDomain);
     if (NS_FAILED(rv))
       continue;
 
-    // create a new nsCookie and assign the data.
-    // we don't know the cookie creation time, so just use the current time;
-    // this is okay, since nsCookie::Create() will make sure the creation id
-    // ends up monotonically increasing.
+    // Create a new nsCookie and assign the data.
+    // We don't know the cookie creation time, so just use the current time
+    // to generate a unique creationID.
     nsRefPtr<nsCookie> newCookie =
       nsCookie::Create(Substring(buffer, nameIndex, cookieIndex - nameIndex - 1),
                        Substring(buffer, cookieIndex, buffer.Length() - cookieIndex),
                        host,
                        Substring(buffer, pathIndex, secureIndex - pathIndex - 1),
                        expires,
                        lastAccessedCounter,
-                       currentTimeInUsec,
+                       nsCookie::GenerateCreationID(currentTimeInUsec),
                        PR_FALSE,
                        Substring(buffer, secureIndex, expiresIndex - secureIndex - 1).EqualsLiteral(kTrue),
                        isHttpOnly);
     if (!newCookie) {
       return NS_ERROR_OUT_OF_MEMORY;
     }
     
     // trick: preserve the most-recently-used cookie ordering,
@@ -2114,17 +2115,17 @@ nsCookieService::SetCookieInternal(nsIUR
   // create a new nsCookie and copy attributes
   nsRefPtr<nsCookie> cookie =
     nsCookie::Create(cookieAttributes.name,
                      cookieAttributes.value,
                      cookieAttributes.host,
                      cookieAttributes.path,
                      cookieAttributes.expiryTime,
                      currentTimeInUsec,
-                     currentTimeInUsec,
+                     nsCookie::GenerateCreationID(currentTimeInUsec),
                      cookieAttributes.isSession,
                      cookieAttributes.isSecure,
                      cookieAttributes.isHttpOnly);
   if (!cookie)
     return newCookie;
 
   // check permissions from site permission list, or ask the user,
   // to determine if we can set the cookie