Bug 614790 - Bookmarks roots init could be locking with the first visit addition.
authorMarco Bonardo <mbonardo@mozilla.com>
Mon, 29 Nov 2010 18:33:26 +0100
changeset 59362 c5c6f317e9b71b1f476737555ad4f85c188e6156
parent 59361 9d659e8d218d72158a46f2efce04a2cbcdb91f78
child 59363 c952bf835f6bc6416323d1a60b7c54635d2146ab
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
bugs614790
milestone2.0b8pre
Bug 614790 - Bookmarks roots init could be locking with the first visit addition. r=sdwilsh a=blocking
browser/base/content/browser-places.js
browser/base/content/browser.js
toolkit/components/places/src/nsAnnotationService.cpp
toolkit/components/places/src/nsNavBookmarks.cpp
toolkit/components/places/src/nsNavBookmarks.h
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -916,30 +916,22 @@ var PlacesMenuDNDHandler = {
                                 Ci.nsITreeView.DROP_ON);
     PlacesControllerDragHelper.onDrop(ip, event.dataTransfer);
     event.stopPropagation();
   }
 };
 
 
 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([
     Ci.nsINavBookmarkObserver
   ]),
 
   get _starredTooltip()
   {
@@ -966,16 +958,27 @@ var PlacesStarButton = {
     this._itemIds = [];
 
     // Hide the star while we update its state.
     this._starIcon.hidden = true;
 
     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);
   },
 
   _updateStateInternal: function PSB__updateStateInternal()
   {
     if (!this._starIcon) {
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -1383,18 +1383,16 @@ function delayedStartup(isLoadingBlank, 
   if (mustLoadSidebar) {
     let sidebar = document.getElementById("sidebar");
     let sidebarBox = document.getElementById("sidebar-box");
     sidebar.setAttribute("src", sidebarBox.getAttribute("src"));
   }
 
   UpdateUrlbarSearchSplitterState();
 
-  PlacesStarButton.init();
-
   if (isLoadingBlank && gURLBar && isElementVisible(gURLBar))
     gURLBar.focus();
   else
     gBrowser.selectedBrowser.focus();
 
   gNavToolbox.customizeDone = BrowserToolboxCustomizeDone;
   gNavToolbox.customizeChange = BrowserToolboxCustomizeChange;
 
--- a/toolkit/components/places/src/nsAnnotationService.cpp
+++ b/toolkit/components/places/src/nsAnnotationService.cpp
@@ -107,20 +107,16 @@ nsAnnotationService::Init()
 {
   // The history service will normally already be created and will call our
   // static InitTables function. It will get autocreated here if it hasn't
   // already been created.
   nsNavHistory* history = nsNavHistory::GetHistoryService();
   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;
 }
 
 
 mozIStorageStatement*
 nsAnnotationService::GetStatement(const nsCOMPtr<mozIStorageStatement>& aStmt)
 {
   if (mShuttingDown)
--- a/toolkit/components/places/src/nsNavBookmarks.cpp
+++ b/toolkit/components/places/src/nsNavBookmarks.cpp
@@ -156,20 +156,24 @@ nsresult
 nsNavBookmarks::Init()
 {
   NS_TIME_FUNCTION;
 
   nsNavHistory* history = nsNavHistory::GetHistoryService();
   NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
   mDBConn = history->GetStorageConnection();
   NS_ENSURE_STATE(mDBConn);
+
+  // Get our read-only cloned connection.
+  nsresult rv = mDBConn->Clone(PR_TRUE, getter_AddRefs(mDBReadOnlyConn));
+  NS_ENSURE_SUCCESS(rv, rv);
+
   PRUint16 dbStatus;
-  nsresult rv = history->GetDatabaseStatus(&dbStatus);
+  rv = history->GetDatabaseStatus(&dbStatus);
   NS_ENSURE_SUCCESS(rv, rv);
-
   rv = InitRoots(dbStatus != nsINavHistoryService::DATABASE_STATUS_OK);
   NS_ENSURE_SUCCESS(rv, rv);
 
   mCanNotify = true;
 
   // Observe annotations.
   nsAnnotationService* annosvc = nsAnnotationService::GetAnnotationService();
   NS_ENSURE_TRUE(annosvc, NS_ERROR_OUT_OF_MEMORY);
@@ -460,25 +464,28 @@ nsNavBookmarks::FinalizeStatements() {
     mDBFindRedirectedBookmark,
   };
 
   for (PRUint32 i = 0; i < NS_ARRAY_LENGTH(stmts); i++) {
     nsresult rv = nsNavHistory::FinalizeStatement(stmts[i]);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
+  // Since we are shutting down, close the read-only connection.
+  (void)mDBReadOnlyConn->AsyncClose(nsnull);
+
   return NS_OK;
 }
 
 
 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);
 
   PRBool hasResult;
   while (NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) {
     nsCAutoString rootName;
     rv = stmt->GetUTF8String(0, rootName);
--- a/toolkit/components/places/src/nsNavBookmarks.h
+++ b/toolkit/components/places/src/nsNavBookmarks.h
@@ -235,17 +235,25 @@ private:
    * @throws If folder does not exist.
    */
   nsresult FolderCount(PRInt64 aFolderId, PRInt32* aFolderCount);
 
   nsresult GetFolderType(PRInt64 aFolder, nsACString& aType);
 
   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);
 
   PRInt32 mItemCount;
 
   nsMaybeWeakPtrArray<nsINavBookmarkObserver> mObservers;