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

@@ -1894,8 +1894,18 @@ nsAnnotationService::StartSetAnnotation(PRInt64 aFkId,
         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);
       }

toolkit/components/places/src/nsFaviconService.cpp

@@ -299,8 +299,18 @@ nsFaviconService::SetFaviconUrlForPageInternal(nsIURI* aPageURI,
     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

toolkit/components/places/src/nsNavBookmarks.cpp

@@ -227,6 +227,14 @@ nsNavBookmarks::Init()
     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));
@@ -857,6 +865,7 @@ nsNavBookmarks::InsertBookmark(PRInt64 aFolder, nsIURI *aItem, PRInt32 aIndex,
   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);
@@ -870,33 +879,41 @@ nsNavBookmarks::InsertBookmark(PRInt64 aFolder, nsIURI *aItem, PRInt32 aIndex,
     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?
@@ -1193,8 +1210,15 @@ nsNavBookmarks::CreateContainerWithID(PRInt64 aItemId, PRInt64 aParent,
   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);
@@ -1251,9 +1275,15 @@ nsNavBookmarks::InsertSeparator(PRInt64 aParent, PRInt32 aIndex,
   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);
@@ -2428,8 +2458,21 @@ nsNavBookmarks::SetKeywordForBookmark(PRInt64 aBookmarkId, const nsAString& aKey
       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);
     }
   }
 

toolkit/components/places/src/nsNavBookmarks.h

@@ -203,6 +203,7 @@ private:
   nsCOMPtr<mozIStorageStatement> mDBGetRedirectDestinations;
   nsCOMPtr<mozIStorageStatement> mDBInsertBookmark;
   nsCOMPtr<mozIStorageStatement> mDBIsBookmarkedInDatabase;
+  nsCOMPtr<mozIStorageStatement> mDBGetLastBookmarkID;
   nsCOMPtr<mozIStorageStatement> mDBSetItemDateAdded;
   nsCOMPtr<mozIStorageStatement> mDBSetItemLastModified;
   nsCOMPtr<mozIStorageStatement> mDBSetItemIndex;

toolkit/components/places/src/nsNavHistory.cpp

@@ -995,6 +995,17 @@ nsNavHistory::InitStatements()
     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 "
@@ -1717,8 +1728,17 @@ nsNavHistory::InternalAddNewPage(nsIURI* aURI,
 
   // If the caller wants the page ID, go get it
   if (aPageID) {
-    rv = mDBConn->GetLastInsertRowID(aPageID);
+    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;
@@ -1734,23 +1754,43 @@ nsNavHistory::InternalAddVisit(PRInt64 aPageID, PRInt64 aReferringVisit,
                                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);
+  {
+    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);
+  }
 
-  rv = mDBInsertVisit->Execute();
-  NS_ENSURE_SUCCESS(rv, rv);
+  {
+    mozStorageStatementScoper scoper(mDBRecentVisitOfPlace);
 
-  return mDBConn->GetLastInsertRowID(visitID);
+    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;
 }
 
 

toolkit/components/places/src/nsNavHistory.h

@@ -396,6 +396,7 @@ protected:
   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