Bug 449884 - Stop using mozIStorageConnection::GetLastInsertRowID
authorShawn Wilsher <sdwilsh@shawnwilsher.com>
Sat, 16 Aug 2008 18:28:28 -0400
changeset 16751 5649accdc89b509617898ec2ac3817d3d7f9a601
parent 16750 bc658ebc5c77380c7c4e871b5a667bc112d3e1ac
child 16752 1997aa804c6c40846e2508ada0f800de6c99a905
push id1292
push usersdwilsh@shawnwilsher.com
push dateSat, 16 Aug 2008 22:29:03 +0000
treeherdermozilla-central@5649accdc89b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs449884
milestone1.9.1a2pre
Bug 449884 - Stop using mozIStorageConnection::GetLastInsertRowID It turns out that this is not a safe function to use at all when you start using the database connection on more than one thread. It also does not work as expected when triggers are used to insert data into a database. r=dietrich
toolkit/components/places/src/nsAnnotationService.cpp
toolkit/components/places/src/nsFaviconService.cpp
toolkit/components/places/src/nsNavBookmarks.cpp
toolkit/components/places/src/nsNavBookmarks.h
toolkit/components/places/src/nsNavHistory.cpp
toolkit/components/places/src/nsNavHistory.h
--- a/toolkit/components/places/src/nsAnnotationService.cpp
+++ b/toolkit/components/places/src/nsAnnotationService.cpp
@@ -1889,18 +1889,28 @@ nsAnnotationService::StartSetAnnotation(
         // add a new annotation name
         mDBGetAnnotationNameID->Reset();
         mozStorageStatementScoper addNameScoper(mDBAddAnnotationName);
         rv = mDBAddAnnotationName->BindUTF8StringParameter(0, aName);
         NS_ENSURE_SUCCESS(rv, rv);
         rv = mDBAddAnnotationName->Execute();
         NS_ENSURE_SUCCESS(rv, rv);
 
-        rv = mDBConn->GetLastInsertRowID(&nameID);
-        NS_ENSURE_SUCCESS(rv, rv);
+        {
+          mozStorageStatementScoper scoper(mDBGetAnnotationNameID);
+
+          rv = mDBGetAnnotationNameID->BindUTF8StringParameter(0, aName);
+          NS_ENSURE_SUCCESS(rv, rv);
+
+          PRBool hasResult;
+          rv = mDBGetAnnotationNameID->ExecuteStep(&hasResult);
+          NS_ENSURE_SUCCESS(rv, rv);
+          NS_ASSERTION(hasResult, "hasResult is false but the call succeeded?");
+          nameID = mDBGetAnnotationNameID->AsInt64(0);
+        }
       } else {
         nameID = mDBGetAnnotationNameID->AsInt64(0);
       }
       rv = (*aStatement)->BindInt64Parameter(kAnnoIndex_PageOrItem, aFkId);
       NS_ENSURE_SUCCESS(rv, rv);
       rv = (*aStatement)->BindInt64Parameter(kAnnoIndex_Name, nameID);
       NS_ENSURE_SUCCESS(rv, rv);
     }
--- a/toolkit/components/places/src/nsFaviconService.cpp
+++ b/toolkit/components/places/src/nsFaviconService.cpp
@@ -294,18 +294,28 @@ nsFaviconService::SetFaviconUrlForPageIn
     // not-binded params are automatically nullified by mozStorage
     mozStorageStatementScoper scoper(mDBInsertIcon);
     rv = BindStatementURI(mDBInsertIcon, 0, aFaviconURI);
     NS_ENSURE_SUCCESS(rv, rv);
 
     rv = mDBInsertIcon->Execute();
     NS_ENSURE_SUCCESS(rv, rv);
 
-    rv = mDBConn->GetLastInsertRowID(&iconId);
-    NS_ENSURE_SUCCESS(rv, rv);
+    {
+      mozStorageStatementScoper scoper(mDBGetIconInfo);
+
+      rv = BindStatementURI(mDBGetIconInfo, 0, aFaviconURI);
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      PRBool hasResult;
+      rv = mDBGetIconInfo->ExecuteStep(&hasResult);
+      NS_ENSURE_SUCCESS(rv, rv);
+      NS_ASSERTION(hasResult, "hasResult is false but the call succeeded?");
+      iconId = mDBGetIconInfo->AsInt64(0);
+    }
   }
 
   // now link our icon entry with the page
   nsNavHistory* historyService = nsNavHistory::GetHistoryService();
   NS_ENSURE_TRUE(historyService, NS_ERROR_OUT_OF_MEMORY);
 
   PRInt64 pageId;
   rv = historyService->GetUrlIdFor(aPageURI, &pageId, PR_TRUE);
--- a/toolkit/components/places/src/nsNavBookmarks.cpp
+++ b/toolkit/components/places/src/nsNavBookmarks.cpp
@@ -222,16 +222,24 @@ nsNavBookmarks::Init()
   // mDBIsBookmarkedInDatabase
   // Just select position since it's just an int32 and may be faster.
   // We don't actually care about the data, just whether there is any.
   rv = DBConn()->CreateStatement(NS_LITERAL_CSTRING(
       "SELECT position FROM moz_bookmarks WHERE fk = ?1 AND type = ?2"),
     getter_AddRefs(mDBIsBookmarkedInDatabase));
   NS_ENSURE_SUCCESS(rv, rv);
 
+  // mDBGetLastBookmarkID
+  rv = DBConn()->CreateStatement(NS_LITERAL_CSTRING(
+      "SELECT id "
+      "FROM moz_bookmarks "
+      "ORDER BY ROWID DESC "
+      "LIMIT 1"),
+    getter_AddRefs(mDBGetLastBookmarkID));
+
   // mDBSetItemDateAdded
   rv = dbConn->CreateStatement(NS_LITERAL_CSTRING("UPDATE moz_bookmarks SET dateAdded = ?1 WHERE id = ?2"),
     getter_AddRefs(mDBSetItemDateAdded));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // mDBSetItemLastModified
   rv = dbConn->CreateStatement(NS_LITERAL_CSTRING("UPDATE moz_bookmarks SET lastModified = ?1 WHERE id = ?2"),
                                getter_AddRefs(mDBSetItemLastModified));
@@ -852,56 +860,65 @@ nsNavBookmarks::InsertBookmark(PRInt64 a
   // You can pass -1 to indicate append, but no other negative number is allowed
   if (aIndex < nsINavBookmarksService::DEFAULT_INDEX)
     return NS_ERROR_INVALID_ARG;
   NS_ENSURE_ARG_POINTER(aNewBookmarkId);
 
   mozIStorageConnection *dbConn = DBConn();
   mozStorageTransaction transaction(dbConn, PR_FALSE);
 
+  // This is really a place ID
   PRInt64 childID;
   nsresult rv = History()->GetUrlIdFor(aItem, &childID, PR_TRUE);
   NS_ENSURE_SUCCESS(rv, rv);
 
   PRInt32 index;
   if (aIndex == nsINavBookmarksService::DEFAULT_INDEX) {
     index = FolderCount(aFolder);
   } else {
     index = aIndex;
     rv = AdjustIndices(aFolder, index, PR_INT32_MAX, 1);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  mozStorageStatementScoper scope(mDBInsertBookmark);
-  rv = mDBInsertBookmark->BindInt64Parameter(0, childID);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = mDBInsertBookmark->BindInt32Parameter(1, TYPE_BOOKMARK);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = mDBInsertBookmark->BindInt64Parameter(2, aFolder);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = mDBInsertBookmark->BindInt32Parameter(3, index);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (aTitle.IsVoid())
-    rv = mDBInsertBookmark->BindNullParameter(4);
-  else
-    rv = mDBInsertBookmark->BindUTF8StringParameter(4, aTitle);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  rv = mDBInsertBookmark->BindInt64Parameter(5, PR_Now());
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  rv = mDBInsertBookmark->Execute();
-  NS_ENSURE_SUCCESS(rv, rv);
+  {
+    mozStorageStatementScoper scope(mDBInsertBookmark);
+    rv = mDBInsertBookmark->BindInt64Parameter(0, childID);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = mDBInsertBookmark->BindInt32Parameter(1, TYPE_BOOKMARK);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = mDBInsertBookmark->BindInt64Parameter(2, aFolder);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = mDBInsertBookmark->BindInt32Parameter(3, index);
+    NS_ENSURE_SUCCESS(rv, rv);
+  
+    if (aTitle.IsVoid())
+      rv = mDBInsertBookmark->BindNullParameter(4);
+    else
+      rv = mDBInsertBookmark->BindUTF8StringParameter(4, aTitle);
+    NS_ENSURE_SUCCESS(rv, rv);
+  
+    rv = mDBInsertBookmark->BindInt64Parameter(5, PR_Now());
+    NS_ENSURE_SUCCESS(rv, rv);
+  
+    rv = mDBInsertBookmark->Execute();
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
 
   // get row id of the new bookmark
   PRInt64 rowId;
-  rv = dbConn->GetLastInsertRowID(&rowId);
-  NS_ENSURE_SUCCESS(rv, rv);
-  *aNewBookmarkId = rowId;
+  {
+    mozStorageStatementScoper scoper(mDBGetLastBookmarkID);
+    
+    PRBool hasResult;
+    rv = mDBGetLastBookmarkID->ExecuteStep(&hasResult);
+    NS_ENSURE_SUCCESS(rv, rv);
+    NS_ASSERTION(hasResult, "hasResult is false but the call succeeded?");
+    rowId = *aNewBookmarkId = mDBGetLastBookmarkID->AsInt64(0);
+  }
 
   // XXX
   // 0n import / fx 2 migration, is the frecency work going to slow us down?
   // We might want to skip this stuff, as well as the frecency work
   // caused by GetUrlIdFor() which calls InternalAddNewPage().
   // If we do skip this, after import, we will
   // need to call FixInvalidFrecenciesForExcludedPlaces().
   // We might need to call it anyways, if items aren't properly annotated
@@ -1188,18 +1205,25 @@ nsNavBookmarks::CreateContainerWithID(PR
   NS_ENSURE_SUCCESS(rv, rv);
   rv = statement->BindInt64Parameter(5, PR_Now());
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = statement->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
   PRInt64 id;
-  rv = dbConn->GetLastInsertRowID(&id);
-  NS_ENSURE_SUCCESS(rv, rv);
+  {
+    mozStorageStatementScoper scoper(mDBGetLastBookmarkID);
+    
+    PRBool hasResult;
+    rv = mDBGetLastBookmarkID->ExecuteStep(&hasResult);
+    NS_ENSURE_SUCCESS(rv, rv);
+    NS_ASSERTION(hasResult, "hasResult is false but the call succeeded?");
+    id = mDBGetLastBookmarkID->AsInt64(0);
+  }
 
   rv = SetItemDateInternal(mDBSetItemLastModified, aParent, PR_Now());
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = transaction.Commit();
   NS_ENSURE_SUCCESS(rv, rv);
 
   ENUMERATE_WEAKARRAY(mObservers, nsINavBookmarkObserver,
@@ -1246,19 +1270,25 @@ nsNavBookmarks::InsertSeparator(PRInt64 
   NS_ENSURE_SUCCESS(rv, rv);
   rv = statement->BindInt64Parameter(3, PR_Now()); 
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = statement->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
   PRInt64 rowId;
-  rv = dbConn->GetLastInsertRowID(&rowId);
-  NS_ENSURE_SUCCESS(rv, rv);
-  *aNewItemId = rowId;
+  {
+    mozStorageStatementScoper scoper(mDBGetLastBookmarkID);
+    
+    PRBool hasResult;
+    rv = mDBGetLastBookmarkID->ExecuteStep(&hasResult);
+    NS_ENSURE_SUCCESS(rv, rv);
+    NS_ASSERTION(hasResult, "hasResult is false but the call succeeded?");
+    rowId = *aNewItemId = mDBGetLastBookmarkID->AsInt64(0);
+  }
 
   rv = SetItemDateInternal(mDBSetItemLastModified, aParent, PR_Now());
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = transaction.Commit();
   NS_ENSURE_SUCCESS(rv, rv);
 
   ENUMERATE_WEAKARRAY(mObservers, nsINavBookmarkObserver,
@@ -2423,18 +2453,31 @@ nsNavBookmarks::SetKeywordForBookmark(PR
       rv = DBConn()->CreateStatement(NS_LITERAL_CSTRING(
           "INSERT INTO moz_keywords (keyword) VALUES (?1)"),
         getter_AddRefs(addKeywordStmnt));
       NS_ENSURE_SUCCESS(rv, rv);
       rv = addKeywordStmnt->BindStringParameter(0, kwd);
       NS_ENSURE_SUCCESS(rv, rv);
       rv = addKeywordStmnt->Execute();
       NS_ENSURE_SUCCESS(rv, rv);
-      rv = DBConn()->GetLastInsertRowID(&keywordId);
+
+      nsCOMPtr<mozIStorageStatement> idStmt;
+      rv = DBConn()->CreateStatement(NS_LITERAL_CSTRING(
+          "SELECT id "
+          "FROM moz_keywords "
+          "ORDER BY ROWID DESC "
+          "LIMIT 1"),
+        getter_AddRefs(idStmt));
       NS_ENSURE_SUCCESS(rv, rv);
+
+      PRBool hasResult;
+      rv = idStmt->ExecuteStep(&hasResult);
+      NS_ENSURE_SUCCESS(rv, rv);
+      NS_ASSERTION(hasResult, "hasResult is false but the call succeeded?");
+      keywordId = idStmt->AsInt64(0);
     }
   }
 
   // Update bookmark record w/ the keyword's id, or null
   nsCOMPtr<mozIStorageStatement> updateKeywordStmnt;
   rv = DBConn()->CreateStatement(NS_LITERAL_CSTRING(
       "UPDATE moz_bookmarks SET keyword_id = ?1 WHERE id = ?2"),
     getter_AddRefs(updateKeywordStmnt));
--- a/toolkit/components/places/src/nsNavBookmarks.h
+++ b/toolkit/components/places/src/nsNavBookmarks.h
@@ -198,16 +198,17 @@ private:
   static const PRInt32 kGetItemPropertiesIndex_ServiceContractId;
   static const PRInt32 kGetItemPropertiesIndex_DateAdded;
   static const PRInt32 kGetItemPropertiesIndex_LastModified;
 
   nsCOMPtr<mozIStorageStatement> mDBGetItemIdForGUID;
   nsCOMPtr<mozIStorageStatement> mDBGetRedirectDestinations;
   nsCOMPtr<mozIStorageStatement> mDBInsertBookmark;
   nsCOMPtr<mozIStorageStatement> mDBIsBookmarkedInDatabase;
+  nsCOMPtr<mozIStorageStatement> mDBGetLastBookmarkID;
   nsCOMPtr<mozIStorageStatement> mDBSetItemDateAdded;
   nsCOMPtr<mozIStorageStatement> mDBSetItemLastModified;
   nsCOMPtr<mozIStorageStatement> mDBSetItemIndex;
 
   // keywords
   nsCOMPtr<mozIStorageStatement> mDBGetKeywordForURI;
   nsCOMPtr<mozIStorageStatement> mDBGetKeywordForBookmark;
   nsCOMPtr<mozIStorageStatement> mDBGetURIForKeyword;
--- a/toolkit/components/places/src/nsNavHistory.cpp
+++ b/toolkit/components/places/src/nsNavHistory.cpp
@@ -990,16 +990,27 @@ nsNavHistory::InitStatements()
       "SELECT v.id, v.session "
       "FROM moz_places h JOIN moz_historyvisits v ON h.id = v.place_id "
       "WHERE h.url = ?1 "
       "ORDER BY v.visit_date DESC "
       "LIMIT 1"),
     getter_AddRefs(mDBRecentVisitOfURL));
   NS_ENSURE_SUCCESS(rv, rv);
 
+  // mDBRecentVisitOfPlace
+  rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
+      "SELECT id "
+      "FROM moz_historyvisits "
+      "WHERE place_id = ?1 "
+      "AND visit_date = ?2 "
+      "AND session = ?3 "
+      "LIMIT 1"),
+    getter_AddRefs(mDBRecentVisitOfPlace));
+  NS_ENSURE_SUCCESS(rv, rv);
+
   // mDBInsertVisit
   rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
       "INSERT INTO moz_historyvisits "
       "(from_visit, place_id, visit_date, visit_type, session) "
       "VALUES (?1, ?2, ?3, ?4, ?5)"),
     getter_AddRefs(mDBInsertVisit));
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -1712,50 +1723,79 @@ nsNavHistory::InternalAddNewPage(nsIURI*
   rv = mDBAddNewPage->BindInt32Parameter(5, frecency);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = mDBAddNewPage->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
   // If the caller wants the page ID, go get it
   if (aPageID) {
-    rv = mDBConn->GetLastInsertRowID(aPageID);
-    NS_ENSURE_SUCCESS(rv, rv);
+    mozStorageStatementScoper scoper(mDBGetURLPageInfo);
+
+    rv = BindStatementURI(mDBGetURLPageInfo, 0, aURI);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    PRBool hasResult = PR_FALSE;
+    rv = mDBGetURLPageInfo->ExecuteStep(&hasResult);
+    NS_ENSURE_SUCCESS(rv, rv);
+    NS_ASSERTION(hasResult, "hasResult is false but the call succeeded?");
+
+    *aPageID = mDBGetURLPageInfo->AsInt64(0);
   }
 
   return NS_OK;
 }
 
 // nsNavHistory::InternalAddVisit
 //
 //    Just a wrapper for inserting a new visit in the DB.
 
 nsresult
 nsNavHistory::InternalAddVisit(PRInt64 aPageID, PRInt64 aReferringVisit,
                                PRInt64 aSessionID, PRTime aTime,
                                PRInt32 aTransitionType, PRInt64* visitID)
 {
   nsresult rv;
-  mozStorageStatementScoper scoper(mDBInsertVisit);
-
-  rv = mDBInsertVisit->BindInt64Parameter(0, aReferringVisit);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = mDBInsertVisit->BindInt64Parameter(1, aPageID);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = mDBInsertVisit->BindInt64Parameter(2, aTime);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = mDBInsertVisit->BindInt32Parameter(3, aTransitionType);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = mDBInsertVisit->BindInt64Parameter(4, aSessionID);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  rv = mDBInsertVisit->Execute();
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  return mDBConn->GetLastInsertRowID(visitID);
+
+  {
+    mozStorageStatementScoper scoper(mDBInsertVisit);
+  
+    rv = mDBInsertVisit->BindInt64Parameter(0, aReferringVisit);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = mDBInsertVisit->BindInt64Parameter(1, aPageID);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = mDBInsertVisit->BindInt64Parameter(2, aTime);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = mDBInsertVisit->BindInt32Parameter(3, aTransitionType);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = mDBInsertVisit->BindInt64Parameter(4, aSessionID);
+    NS_ENSURE_SUCCESS(rv, rv);
+  
+    rv = mDBInsertVisit->Execute();
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  {
+    mozStorageStatementScoper scoper(mDBRecentVisitOfPlace);
+
+    rv = mDBRecentVisitOfPlace->BindInt64Parameter(0, aPageID);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = mDBRecentVisitOfPlace->BindInt64Parameter(1, aTime);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = mDBRecentVisitOfPlace->BindInt64Parameter(2, aSessionID);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    PRBool hasResult;
+    rv = mDBRecentVisitOfPlace->ExecuteStep(&hasResult);
+    NS_ENSURE_SUCCESS(rv, rv);
+    NS_ASSERTION(hasResult, "hasResult is false but the call succeeded?");
+
+    *visitID = mDBRecentVisitOfPlace->AsInt64(0);
+  }
+  return NS_OK;
 }
 
 
 // nsNavHistory::FindLastVisit
 //
 //    This finds the most recent visit to the given URL. If found, it will put
 //    that visit's ID and session into the respective out parameters and return
 //    true. Returns false if no visit is found.
--- a/toolkit/components/places/src/nsNavHistory.h
+++ b/toolkit/components/places/src/nsNavHistory.h
@@ -391,16 +391,17 @@ protected:
   nsCOMPtr<mozIStorageService> mDBService;
   nsCOMPtr<mozIStorageConnection> mDBConn;
   nsCOMPtr<nsIFile> mDBFile;
 
   nsCOMPtr<mozIStorageStatement> mDBGetURLPageInfo;   // kGetInfoIndex_* results
   nsCOMPtr<mozIStorageStatement> mDBGetIdPageInfo;     // kGetInfoIndex_* results
 
   nsCOMPtr<mozIStorageStatement> mDBRecentVisitOfURL; // converts URL into most recent visit ID/session ID
+  nsCOMPtr<mozIStorageStatement> mDBRecentVisitOfPlace; // converts placeID into most recent visit ID/session ID
   nsCOMPtr<mozIStorageStatement> mDBInsertVisit; // used by AddVisit
   nsCOMPtr<mozIStorageStatement> mDBGetPageVisitStats; // used by AddVisit
   nsCOMPtr<mozIStorageStatement> mDBIsPageVisited; // used by IsURIStringVisited
   nsCOMPtr<mozIStorageStatement> mDBUpdatePageVisitStats; // used by AddVisit
   nsCOMPtr<mozIStorageStatement> mDBAddNewPage; // used by InternalAddNewPage
   nsCOMPtr<mozIStorageStatement> mDBGetTags; // used by FilterResultSet
   nsCOMPtr<mozIStorageStatement> mFoldersWithAnnotationQuery;  // used by StartSearch and FilterResultSet