Bug 559682 - Ensure we create a valid transaction in batches before trying to commit it. r=sdwilsh
authorMarco Bonardo <mbonardo@mozilla.com>
Fri, 16 Apr 2010 14:31:21 +0200
changeset 40923 3e1263cff09e33598d533a7e3cbfec5afe960d72
parent 40922 7a19f732a07076ef72ad5b4323916da414804ab5
child 40924 88bea857d0f3a7613b56b3d1ece6007a22916cb0
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssdwilsh
bugs559682
milestone1.9.3a5pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
Bug 559682 - Ensure we create a valid transaction in batches before trying to commit it. r=sdwilsh
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/nsNavBookmarks.cpp
+++ b/toolkit/components/places/src/nsNavBookmarks.cpp
@@ -101,17 +101,17 @@ PLACES_FACTORY_SINGLETON_IMPLEMENTATION(
 #define READ_ONLY_ANNO NS_LITERAL_CSTRING("placesInternal/READ_ONLY")
 
 nsNavBookmarks::nsNavBookmarks() : mItemCount(0)
                                  , mRoot(0)
                                  , mBookmarksRoot(0)
                                  , mTagRoot(0)
                                  , mToolbarFolder(0)
                                  , mBatchLevel(0)
-                                 , mBatchHasTransaction(PR_FALSE)
+                                 , mBatchDBTransaction(nsnull)
                                  , mCanNotify(false)
                                  , mCacheObservers("bookmark-observers")
                                  , mShuttingDown(false)
 {
   NS_ASSERTION(!gBookmarksService,
                "Attempting to create two instances of the service!");
   gBookmarksService = this;
 }
@@ -2902,37 +2902,36 @@ nsNavBookmarks::GetURIForKeyword(const n
 }
 
 
 // See RunInBatchMode
 nsresult
 nsNavBookmarks::BeginUpdateBatch()
 {
   if (mBatchLevel++ == 0) {
-    mozIStorageConnection* conn = mDBConn;
-    PRBool transactionInProgress = PR_TRUE; // default to no transaction on err
-    conn->GetTransactionInProgress(&transactionInProgress);
-    mBatchHasTransaction = ! transactionInProgress;
-    if (mBatchHasTransaction)
-      conn->BeginTransaction();
+    mBatchDBTransaction = new mozStorageTransaction(mDBConn, PR_FALSE);
 
     NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
                      nsINavBookmarkObserver, OnBeginUpdateBatch());
   }
   return NS_OK;
 }
 
 
 nsresult
 nsNavBookmarks::EndUpdateBatch()
 {
   if (--mBatchLevel == 0) {
-    if (mBatchHasTransaction)
-      mDBConn->CommitTransaction();
-    mBatchHasTransaction = PR_FALSE;
+    if (mBatchDBTransaction) {
+      nsresult rv = mBatchDBTransaction->Commit();
+      NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Batch failed to commit transaction");
+      delete mBatchDBTransaction;
+      mBatchDBTransaction = nsnull;
+    }
+
     NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
                      nsINavBookmarkObserver, OnEndUpdateBatch());
   }
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
--- a/toolkit/components/places/src/nsNavBookmarks.h
+++ b/toolkit/components/places/src/nsNavBookmarks.h
@@ -229,22 +229,20 @@ private:
   PRInt64 mRoot;
   PRInt64 mBookmarksRoot;
   PRInt64 mTagRoot;
   PRInt64 mUnfiledRoot;
 
   // personal toolbar folder
   PRInt64 mToolbarFolder;
 
-  // the level of nesting of batches, 0 when no batches are open
+  // The level of batches' nesting, 0 when no batches are open.
   PRInt32 mBatchLevel;
-
-  // true if the outermost batch has an associated transaction that should
-  // be committed when our batch level reaches 0 again.
-  PRBool mBatchHasTransaction;
+  // Current active transaction for a batch.
+  mozStorageTransaction* mBatchDBTransaction;
 
   nsresult GetParentAndIndexOfFolder(PRInt64 aFolder,
                                      PRInt64* aParent,
                                      PRInt32* aIndex);
 
   nsresult IsBookmarkedInDatabase(PRInt64 aBookmarkID, PRBool* aIsBookmarked);
 
   nsresult SetItemDateInternal(mozIStorageStatement* aStatement,
--- a/toolkit/components/places/src/nsNavHistory.cpp
+++ b/toolkit/components/places/src/nsNavHistory.cpp
@@ -380,17 +380,17 @@ const PRInt32 nsNavHistory::kGetInfoInde
 const PRInt32 nsNavHistory::kGetInfoIndex_ItemTags = 12;
 
 
 PLACES_FACTORY_SINGLETON_IMPLEMENTATION(nsNavHistory, gHistoryService)
 
 
 nsNavHistory::nsNavHistory()
 : mBatchLevel(0)
-, mBatchHasTransaction(PR_FALSE)
+, mBatchDBTransaction(nsnull)
 , mCachedNow(0)
 , mExpireNowTimer(nsnull)
 , mLastSessionID(0)
 , mHistoryEnabled(PR_TRUE)
 , mNumVisitsForFrecency(10)
 , mTagsFolder(-1)
 , mInPrivateBrowsing(PRIVATEBROWSING_NOTINITED)
 , mDatabaseStatus(DATABASE_STATUS_OK)
@@ -4279,36 +4279,36 @@ nsNavHistory::RemoveObserver(nsINavHisto
 }
 
 // nsNavHistory::BeginUpdateBatch
 // See RunInBatchMode
 nsresult
 nsNavHistory::BeginUpdateBatch()
 {
   if (mBatchLevel++ == 0) {
-    PRBool transactionInProgress = PR_TRUE; // default to no transaction on err
-    mDBConn->GetTransactionInProgress(&transactionInProgress);
-    mBatchHasTransaction = ! transactionInProgress;
-    if (mBatchHasTransaction)
-      mDBConn->BeginTransaction();
+    mBatchDBTransaction = new mozStorageTransaction(mDBConn, PR_FALSE);
 
     NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
                      nsINavHistoryObserver, OnBeginUpdateBatch());
   }
   return NS_OK;
 }
 
 // nsNavHistory::EndUpdateBatch
 nsresult
 nsNavHistory::EndUpdateBatch()
 {
   if (--mBatchLevel == 0) {
-    if (mBatchHasTransaction)
-      mDBConn->CommitTransaction();
-    mBatchHasTransaction = PR_FALSE;
+    if (mBatchDBTransaction) {
+      nsresult rv = mBatchDBTransaction->Commit();
+      NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Batch failed to commit transaction");
+      delete mBatchDBTransaction;
+      mBatchDBTransaction = nsnull;
+    }
+
     NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
                      nsINavHistoryObserver, OnEndUpdateBatch());
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNavHistory::RunInBatchMode(nsINavHistoryBatchCallback* aCallback,
--- a/toolkit/components/places/src/nsNavHistory.h
+++ b/toolkit/components/places/src/nsNavHistory.h
@@ -308,22 +308,20 @@ public:
                          nsACString& aDomainName);
   static PRTime NormalizeTime(PRUint32 aRelative, PRTime aOffset);
 
   // Don't use these directly, inside nsNavHistory use UpdateBatchScoper,
   // else use nsINavHistoryService::RunInBatchMode
   nsresult BeginUpdateBatch();
   nsresult EndUpdateBatch();
 
-  // the level of nesting of batches, 0 when no batches are open
+  // The level of batches' nesting, 0 when no batches are open.
   PRInt32 mBatchLevel;
-
-  // true if the outermost batch has an associated transaction that should
-  // be committed when our batch level reaches 0 again.
-  PRBool mBatchHasTransaction;
+  // Current active transaction for a batch.
+  mozStorageTransaction* mBatchDBTransaction;
 
   // better alternative to QueryStringToQueries (in nsNavHistoryQuery.cpp)
   nsresult QueryStringToQueryArray(const nsACString& aQueryString,
                                    nsCOMArray<nsNavHistoryQuery>* aQueries,
                                    nsNavHistoryQueryOptions** aOptions);
 
   // Import-friendly version of AddVisit.
   // This method adds a page to history along with a single last visit.