Bug 614790 - Bookmarks roots init could be locking with the first visit addition.
r=sdwilsh a=blocking
--- 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;