Bug 612455 - History.cpp should finalize statementCache on the background thread
authorShawn Wilsher <me@shawnwilsher.com>
Thu, 18 Nov 2010 02:23:29 +0100
changeset 59344 af7e6f7cbf16f7ed07586d6ccc5c3f47d0fd6692
parent 59343 2a7a3641d67d3936cfae912fbcf42f23297bd92f
child 59345 6830eab89a7c3036d1ef944001b85cdf9bf4458a
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
bugs612455
milestone2.0b8pre
Bug 612455 - History.cpp should finalize statementCache on the background thread Also fixes the favicon service to do this in a safer manner. r=mak a=blocking
toolkit/components/places/src/Helpers.h
toolkit/components/places/src/History.cpp
toolkit/components/places/src/nsFaviconService.cpp
--- a/toolkit/components/places/src/Helpers.h
+++ b/toolkit/components/places/src/Helpers.h
@@ -41,16 +41,17 @@
 
 /**
  * This file contains helper classes used by various bits of Places code.
  */
 
 #include "mozilla/storage.h"
 #include "nsIURI.h"
 #include "nsThreadUtils.h"
+#include "nsProxyRelease.h"
 
 namespace mozilla {
 namespace places {
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Asynchronous Statement Callback Helper
 
 class AsyncStatementCallback : public mozIStorageStatementCallback
@@ -206,31 +207,41 @@ template<typename StatementType>
 class FinalizeStatementCacheProxy : public nsRunnable
 {
 public:
   /**
    * Constructor.
    *
    * @param aStatementCache
    *        The statementCache that should be finalized.
+   * @param aOwner
+   *        The object that owns the statement cache.  This runnable will hold
+   *        a strong reference to it so aStatementCache will not disappear from
+   *        under us.
    */
   FinalizeStatementCacheProxy(
-    mozilla::storage::StatementCache<StatementType>& aStatementCache
+    mozilla::storage::StatementCache<StatementType>& aStatementCache,
+    nsISupports* aOwner
   )
   : mStatementCache(aStatementCache)
+  , mOwner(aOwner)
+  , mCallingThread(do_GetCurrentThread())
   {
   }
 
-  NS_IMETHOD
-  Run()
+  NS_IMETHOD Run()
   {
     mStatementCache.FinalizeStatements();
+    // Release the owner back on the calling thread.
+    (void)NS_ProxyRelease(mCallingThread, mOwner);
     return NS_OK;
   }
 
 protected:
   mozilla::storage::StatementCache<StatementType>& mStatementCache;
+  nsCOMPtr<nsISupports> mOwner;
+  nsCOMPtr<nsIThread> mCallingThread;
 };
 
 } // namespace places
 } // namespace mozilla
 
 #endif // mozilla_places_Helpers_h_
--- a/toolkit/components/places/src/History.cpp
+++ b/toolkit/components/places/src/History.cpp
@@ -858,20 +858,16 @@ History::~History()
   gService = NULL;
 
 #ifdef DEBUG
   if (mObservers.IsInitialized()) {
     NS_ASSERTION(mObservers.Count() == 0,
                  "Not all Links were removed before we disappear!");
   }
 #endif
-
-  // Places shutdown event may not occur, but we *must* clean up before History
-  // goes away.
-  Shutdown();
 }
 
 void
 History::NotifyVisited(nsIURI* aURI)
 {
   NS_ASSERTION(aURI, "Ruh-roh!  A NULL URI was passed to us!");
 
 #ifdef MOZ_IPC
@@ -979,20 +975,28 @@ History::GetDBConn()
   NS_ENSURE_SUCCESS(rv, nsnull);
 
   return mDBConn;
 }
 
 void
 History::Shutdown()
 {
+  NS_ASSERTION(!mShuttingDown, "Shutdown was called more than once!");
+
   mShuttingDown = true;
 
   // Clean up our statements and connection.
-  syncStatements.FinalizeStatements();
+  nsISupports* obj = static_cast<IHistory*>(this);
+  nsCOMPtr<nsIRunnable> event =
+    new FinalizeStatementCacheProxy<mozIStorageStatement>(syncStatements, obj);
+  nsCOMPtr<nsIEventTarget> target = do_GetInterface(mDBConn);
+  if (target) {
+    (void)target->Dispatch(event, NS_DISPATCH_NORMAL);
+  }
 
   if (mReadOnlyDBConn) {
     if (mIsVisitedStatement) {
       (void)mIsVisitedStatement->Finalize();
     }
     (void)mReadOnlyDBConn->AsyncClose(nsnull);
   }
 }
--- a/toolkit/components/places/src/nsFaviconService.cpp
+++ b/toolkit/components/places/src/nsFaviconService.cpp
@@ -937,17 +937,17 @@ nsFaviconService::FinalizeStatements() {
 
   for (PRUint32 i = 0; i < NS_ARRAY_LENGTH(stmts); i++) {
     nsresult rv = nsNavHistory::FinalizeStatement(stmts[i]);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   // Finalize the statementCache on the correct thread.
   nsRefPtr<FinalizeStatementCacheProxy<mozIStorageStatement> > event =
-    new FinalizeStatementCacheProxy<mozIStorageStatement>(mSyncStatements);
+    new FinalizeStatementCacheProxy<mozIStorageStatement>(mSyncStatements, this);
   nsCOMPtr<nsIEventTarget> target = do_GetInterface(mDBConn);
   NS_ENSURE_TRUE(target, NS_ERROR_OUT_OF_MEMORY);
   nsresult rv = target->Dispatch(event, NS_DISPATCH_NORMAL);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }