Bug 614790 - Bookmarks roots init could be locking with the first visit addition.

r=sdwilsh a=blocking

browser/base/content/browser-places.js

@@ -921,20 +921,12 @@ var PlacesMenuDNDHandler = {
 
 
 var PlacesStarButton = {
-  init: function PSB_init()
-  {
-    try {
-      PlacesUtils.bookmarks.addObserver(this, false);
-    } catch(ex) {
-      Components.utils.reportError("PlacesStarButton.init(): error adding bookmark observer: " + ex);
-    }
-  },
-
+  _hasBookmarksObserver: false,
   uninit: function PSB_uninit()
   {
-    try {
+    if (this._hasBookmarksObserver) {
       PlacesUtils.bookmarks.removeObserver(this);
-    } catch(ex) {}
+    }
   },
 
   QueryInterface: XPCOMUtils.generateQI([
@@ -971,6 +963,17 @@ var PlacesStarButton = {
     PlacesUtils.asyncGetBookmarkIds(this._uri, function (aItemIds) {
       this._itemIds = aItemIds;
       this._updateStateInternal();
+
+      // Start observing bookmarks if needed.
+      if (!this._hasBookmarksObserver) {
+        try {
+          PlacesUtils.bookmarks.addObserver(this, false);
+          this._hasBookmarksObserver = true;
+        } catch(ex) {
+          Components.utils.reportError("PlacesStarButton failed adding a bookmarks observer: " + ex);
+        }
+      }
+
       // Finally show the star.
       this._starIcon.hidden = false;
     }, this);

browser/base/content/browser.js

@@ -1388,8 +1388,6 @@ function delayedStartup(isLoadingBlank, mustLoadSidebar) {
 
   UpdateUrlbarSearchSplitterState();
 
-  PlacesStarButton.init();
-
   if (isLoadingBlank && gURLBar && isElementVisible(gURLBar))
     gURLBar.focus();
   else

toolkit/components/places/src/nsAnnotationService.cpp

@@ -112,10 +112,6 @@ nsAnnotationService::Init()
   NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
   mDBConn = history->GetStorageConnection();
 
-  // These statements should be responsive, so we init them immediately.
-  (void*)GetStatement(mDBCheckPageAnnotation);
-  (void*)GetStatement(mDBCheckItemAnnotation);
-
   return NS_OK;
 }
 

toolkit/components/places/src/nsNavBookmarks.cpp

@@ -161,10 +161,14 @@ nsNavBookmarks::Init()
   NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
   mDBConn = history->GetStorageConnection();
   NS_ENSURE_STATE(mDBConn);
-  PRUint16 dbStatus;
-  nsresult rv = history->GetDatabaseStatus(&dbStatus);
+
+  // Get our read-only cloned connection.
+  nsresult rv = mDBConn->Clone(PR_TRUE, getter_AddRefs(mDBReadOnlyConn));
   NS_ENSURE_SUCCESS(rv, rv);
 
+  PRUint16 dbStatus;
+  rv = history->GetDatabaseStatus(&dbStatus);
+  NS_ENSURE_SUCCESS(rv, rv);
   rv = InitRoots(dbStatus != nsINavHistoryService::DATABASE_STATUS_OK);
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -465,6 +469,9 @@ nsNavBookmarks::FinalizeStatements() {
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
+  // Since we are shutting down, close the read-only connection.
+  (void)mDBReadOnlyConn->AsyncClose(nsnull);
+
   return NS_OK;
 }
 
@@ -473,7 +480,7 @@ nsresult
 nsNavBookmarks::InitRoots(bool aForceCreate)
 {
   nsCOMPtr<mozIStorageStatement> stmt;
-  nsresult rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
+  nsresult rv = mDBReadOnlyConn->CreateStatement(NS_LITERAL_CSTRING(
     "SELECT root_name, folder_id FROM moz_bookmarks_roots"
   ), getter_AddRefs(stmt));
   NS_ENSURE_SUCCESS(rv, rv);

toolkit/components/places/src/nsNavBookmarks.h

@@ -240,7 +240,15 @@ private:
 
   nsresult GetLastChildId(PRInt64 aFolder, PRInt64* aItemId);
 
+  /**
+   * This is the basic Places read-write connection, obtained from history.
+   */
   nsCOMPtr<mozIStorageConnection> mDBConn;
+  /**
+   * Cloned read-only connection.  Can be used to read from the database
+   * without being locked out by writers.
+   */
+  nsCOMPtr<mozIStorageConnection> mDBReadOnlyConn;
 
   nsString mGUIDBase;
   nsresult GetGUIDBase(nsAString& aGUIDBase);