Bug 342296 Switch bookmark transaction system from JS to C++ to prevent leaks r=IanN
--- a/suite/browser/public/nsIBookmarksService.idl
+++ b/suite/browser/public/nsIBookmarksService.idl
@@ -38,20 +38,22 @@
* ***** END LICENSE BLOCK ***** */
/**
* The Browser Bookmarks service
*/
#include "nsISupports.idl"
+interface nsIRDFNode;
interface nsIRDFResource;
+interface nsITransaction;
interface nsITransactionManager;
-[scriptable, uuid(4342a6ac-1b43-4121-b606-4bdf62de71ff)]
+[scriptable, uuid(b3b8d09d-71b1-4654-b807-9288c2f16300)]
interface nsIBookmarksService : nsISupports
{
const unsigned long BOOKMARK_DEFAULT_TYPE = 0;
const unsigned long BOOKMARK_SEARCH_TYPE = 1;
const unsigned long BOOKMARK_FIND_TYPE = 2;
const long SORT_DESCENDING = -1;
const long SORT_ASCENDING = 1;
@@ -100,16 +102,21 @@ interface nsIBookmarksService : nsISuppo
AString getLastCharset(in AUTF8String aURL);
string resolveKeyword(in wstring aName);
void importSystemBookmarks(in nsIRDFResource aParentFolder);
readonly attribute nsITransactionManager transactionManager;
+
+ nsITransaction createTransaction(in nsIRDFResource parent,
+ in nsIRDFNode item,
+ in unsigned long index,
+ in boolean remove);
};
%{C++
// {E638D760-8687-11d2-B530-000000000000}
#define NS_BOOKMARKS_SERVICE_CID \
{ 0xe638d760, 0x8687, 0x11d2, { 0xb5, 0x30, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 } }
--- a/suite/browser/src/nsBookmarksService.cpp
+++ b/suite/browser/src/nsBookmarksService.cpp
@@ -1617,16 +1617,117 @@ ElementArray::Clear()
while (--index >= 0)
{
ElementInfo* elementInfo = ElementAt(index);
delete elementInfo;
}
nsAutoVoidArray::Clear();
}
+/**
+ * The bookmark transaction knows how to assert and unassert arcs
+ */
+class BookmarkTransaction: public nsITransaction,
+ public nsIInterfaceRequestor
+{
+ NS_DECL_ISUPPORTS
+ NS_DECL_NSITRANSACTION
+ NS_DECL_NSIINTERFACEREQUESTOR
+
+private:
+ nsBookmarksService* mBookmarksService;
+ nsCOMPtr<nsIRDFResource> mParent;
+ nsCOMPtr<nsIRDFNode> mItem;
+ PRUint32 mIndex;
+ PRBool mRemove;
+ nsresult performTransaction(PRBool aRemove, PRBool aRenumber);
+
+public:
+ BookmarkTransaction(nsBookmarksService* aBookmarksService,
+ nsIRDFResource* aParent, nsIRDFNode* aItem,
+ PRUint32 aIndex, PRBool aRemove)
+ : mBookmarksService(aBookmarksService),
+ mParent(aParent),
+ mItem(aItem),
+ mIndex(aIndex),
+ mRemove(aRemove) {}
+};
+
+NS_IMPL_ISUPPORTS2(BookmarkTransaction, nsITransaction, nsIInterfaceRequestor)
+
+nsresult
+BookmarkTransaction::performTransaction(PRBool aRemove, PRBool aRenumber)
+{
+ nsresult rv;
+ nsCOMPtr<nsIRDFContainer> container(do_CreateInstance("@mozilla.org/rdf/container;1", &rv));
+ if (NS_FAILED(rv))
+ return rv;
+
+ rv = container->Init(mBookmarksService, mParent);
+ if (NS_FAILED(rv))
+ return rv;
+
+ if (!aRemove)
+ return container->InsertElementAt(mItem, mIndex, aRenumber);
+
+ // We don't renumber the container when deleting bookmarks.
+ // This makes it easy to undo the transaction by restoring the same index.
+ return container->RemoveElement(mItem, PR_FALSE);
+}
+
+NS_IMETHODIMP
+BookmarkTransaction::DoTransaction()
+{
+ return performTransaction(mRemove, PR_TRUE);
+}
+
+NS_IMETHODIMP
+BookmarkTransaction::UndoTransaction()
+{
+ // We don't renumber the container when undoing transactions.
+ // This makes it easy to redo the transaction by restoring the same index.
+ return performTransaction(!mRemove, PR_FALSE);
+}
+
+NS_IMETHODIMP
+BookmarkTransaction::RedoTransaction()
+{
+ // We don't renumber the container when redoing transactions.
+ // This makes it easy to undo the transaction by restoring the same index.
+ return performTransaction(mRemove, PR_FALSE);
+}
+
+NS_IMETHODIMP
+BookmarkTransaction::GetIsTransient(PRBool *aIsTransient)
+{
+ NS_ENSURE_ARG_POINTER(aIsTransient);
+ *aIsTransient = PR_FALSE;
+ return NS_OK;
+}
+
+NS_IMETHODIMP
+BookmarkTransaction::Merge(nsITransaction* aTransaction, PRBool* aResult)
+{
+ NS_ENSURE_ARG_POINTER(aResult);
+ *aResult = PR_FALSE;
+ return NS_OK;
+}
+
+NS_IMETHODIMP
+BookmarkTransaction::GetInterface(REFNSIID aIID, void** aResult)
+{
+ NS_ENSURE_ARG_POINTER(aResult);
+ nsISupports *result = nsnull;
+ if (aIID.Equals(NS_GET_IID(nsIRDFNode))) {
+ NS_ADDREF(result = mItem);
+ *aResult = result;
+ return NS_OK;
+ }
+ return NS_NOINTERFACE;
+}
////////////////////////////////////////////////////////////////////////
// nsBookmarksService implementation
nsBookmarksService::nsBookmarksService() :
mUpdateBatchNest(0),
mDirty(PR_FALSE)
@@ -3974,16 +4075,33 @@ nsBookmarksService::GetTransactionManage
{
NS_ENSURE_ARG_POINTER(aTransactionManager);
NS_ADDREF(*aTransactionManager = mTransactionManager);
return NS_OK;
}
+NS_IMETHODIMP
+nsBookmarksService::CreateTransaction(nsIRDFResource* aParent,
+ nsIRDFNode* aItem,
+ PRUint32 aIndex,
+ PRBool aRemove,
+ nsITransaction** aTransaction)
+{
+ NS_ENSURE_ARG_POINTER(aTransaction);
+ NS_ENSURE_ARG(aItem);
+ *aTransaction = new BookmarkTransaction(this, aParent, aItem, aIndex, aRemove);
+ if (!*aTransaction)
+ return NS_ERROR_OUT_OF_MEMORY;
+
+ NS_ADDREF(*aTransaction);
+ return NS_OK;
+}
+
////////////////////////////////////////////////////////////////////////
// nsIRDFDataSource
NS_IMETHODIMP
nsBookmarksService::GetURI(char* *aURI)
{
*aURI = NS_strdup("rdf:bookmarks");
if (! *aURI)
--- a/suite/common/bookmarks/bookmarks.js
+++ b/suite/common/bookmarks/bookmarks.js
@@ -121,18 +121,16 @@ function initServices()
} catch (e) {}
}
function initBMService()
{
kBMSVCIID = Components.interfaces.nsIBookmarksService;
BMDS = RDF.GetDataSource("rdf:bookmarks");
BMSVC = BMDS.QueryInterface(kBMSVCIID);
- BookmarkTransaction.prototype.RDFC = RDFC;
- BookmarkTransaction.prototype.BMDS = BMDS;
}
/**
* XXX - 04/16/01
* ACK! massive command name collision problems are causing big issues
* in getting this stuff to work in the Navigator window. For sanity's
* sake, we need to rename all the commands to be of the form cmd_bm_*
* otherwise there'll continue to be problems. For now, we're just
@@ -670,37 +668,35 @@ var BookmarksCommand = {
fileName = kFilePicker.file.path;
if (!fileName) return;
}
else return;
}
catch (e) {
return;
}
- rTarget = RDF.GetResource("NC:BookmarksRoot");
+ var rTarget = BookmarksUtils.getNewBookmarkFolder();
RDFC.Init(BMDS, rTarget);
- var countBefore = parseInt(BookmarksUtils.getProperty(rTarget, RDF_NS+"nextVal"));
+ var index = RDFC.GetCount();
var args = [{ property: NC_NS+"URL", literal: fileName}];
this.doBookmarksCommand(rTarget, NC_NS_CMD+"import", args);
- var countAfter = parseInt(BookmarksUtils.getProperty(rTarget, RDF_NS+"nextVal"));
+ RDFC.Init(BMDS, rTarget); // changing the selection clobbers RDFC
+ var count = RDFC.GetCount();
+ if (index == count)
+ return;
- var transaction = new BookmarkImportTransaction("import");
- for (var index = countBefore; index < countAfter; index++) {
- var nChildArc = RDFCU.IndexToOrdinalResource(index);
- var rChild = BMDS.GetTarget(rTarget, nChildArc, true);
- transaction.item .push(rChild);
- transaction.parent .push(rTarget);
- transaction.index .push(index);
- transaction.isValid.push(true);
+ var txmgr = BMSVC.transactionManager;
+ txmgr.beginBatch();
+ while (++index <= count) {
+ var rChild = RDFC.RemoveElementAt(index, true);
+ var txn = BMSVC.createTransaction(rTarget, rChild, index, false);
+ txmgr.doTransaction(txn);
}
- var isCancelled = !BookmarksUtils.any(transaction.isValid);
- if (!isCancelled) {
- BMSVC.transactionManager.doTransaction(transaction);
- BookmarksUtils.flushDataSource();
- }
+ txmgr.endBatch();
+ BookmarksUtils.flushDataSource();
},
exportBookmarks: function ()
{
try {
const kFilePickerContractID = "@mozilla.org/filepicker;1";
const kFilePickerIID = Components.interfaces.nsIFilePicker;
const kFilePicker = Components.classes[kFilePickerContractID].createInstance(kFilePickerIID);
@@ -1313,39 +1309,38 @@ var BookmarksUtils = {
return true;
}
return false;
},
/////////////////////////////////////////////////////////////////////////////
removeSelection: function (aAction, aSelection)
{
- var transaction = new BookmarkRemoveTransaction(aAction);
- transaction.item = new Array(aSelection.length);
- transaction.parent = new Array(aSelection.length);
- transaction.index = new Array(aSelection.length);
- transaction.isValid = BookmarksUtils.isSelectionValidForDeletion(aSelection);
+ var isValid = BookmarksUtils.isSelectionValidForDeletion(aSelection);
+ if (SOUND && !BookmarksUtils.all(isValid))
+ SOUND.beep();
+ if (!BookmarksUtils.any(isValid))
+ return false;
+
+ var txmgr = BMSVC.transactionManager;
+ txmgr.beginBatch();
for (var i = 0; i < aSelection.length; ++i) {
- transaction.item [i] = aSelection.item [i];
- transaction.parent[i] = aSelection.parent[i];
- if (transaction.isValid[i]) {
+ if (isValid[i]) {
RDFC.Init(BMDS, aSelection.parent[i]);
- transaction.index[i] = RDFC.IndexOf(aSelection.item[i]);
+ var index = RDFC.IndexOf(aSelection.item[i]);
+ var txn = BMSVC.createTransaction(aSelection.parent[i], aSelection.item[i], index, true);
+ txmgr.doTransaction(txn);
}
}
- if (SOUND && aAction != "move" && !BookmarksUtils.all(transaction.isValid))
- SOUND.beep();
- var isCancelled = !BookmarksUtils.any(transaction.isValid);
- if (!isCancelled) {
- BMSVC.transactionManager.doTransaction(transaction);
- if (aAction != "move")
- BookmarksUtils.notifyBookmarksChanged();
- BookmarksUtils.flushDataSource();
+ txmgr.endBatch();
+ if (aAction != "move") {
+ BookmarksUtils.notifyBookmarksChanged();
+ BookmarksUtils.flushDataSource();
}
- return !isCancelled;
+ return true;
},
// If the current bookmark is the IE Favorites folder, we have a little
// extra work to do - set the pref |browser.bookmarks.import_system_favorites|
// to ensure that we don't re-import next time.
//if (aSelection[count].getAttribute("type") == (NC_NS + "IEFavoriteFolder")) {
// const kPrefSvcContractID = "@mozilla.org/preferences-service;1";
// const kPrefSvcIID = Components.interfaces.nsIPrefBranch;
// const kPrefSvc = Components.classes[kPrefSvcContractID].getService(kPrefSvcIID);
@@ -1356,60 +1351,61 @@ var BookmarksUtils = {
{
Components.classes["@mozilla.org/observer-service;1"]
.getService(Components.interfaces.nsIObserverService)
.notifyObservers(null, "bookmarks-changed", "");
},
insertSelection: function (aAction, aSelection, aTarget)
{
- var transaction = new BookmarkInsertTransaction(aAction);
- transaction.item = new Array(aSelection.length);
- transaction.parent = new Array(aSelection.length);
- transaction.index = new Array(aSelection.length);
- if (aAction != "move")
- transaction.isValid = BookmarksUtils.isSelectionValidForInsertion(aSelection, aTarget, aAction);
- else // isValid has already been determined in moveSelection
- transaction.isValid = aSelection.isValid;
+ var isValid = BookmarksUtils.isSelectionValidForInsertion(aSelection, aTarget, aAction);
+ if (SOUND && !BookmarksUtils.all(isValid))
+ SOUND.beep();
+ if (!BookmarksUtils.any(isValid))
+ return false;
+
+ var txmgr = BMSVC.transactionManager;
+ txmgr.beginBatch();
var index = aTarget.index;
- for (var i=0; i<aSelection.length; ++i) {
- var rSource = aSelection.item[i];
-
- if (transaction.isValid[i]) {
- if (BMSVC.isBookmarkedResource(rSource))
- rSource = BMSVC.cloneResource(rSource);
- transaction.item [i] = rSource;
- transaction.parent[i] = aTarget.parent;
- transaction.index [i] = index++;
- } else {
- transaction.item [i] = rSource;
- transaction.parent[i] = aSelection.parent[i];
- }
+ for (var i = 0; i < aSelection.length; ++i) {
+ if (isValid[i]) {
+ var txn = BMSVC.createTransaction(aTarget.parent, aSelection.item[i], index++, false);
+ txmgr.doTransaction(txn);
+ }
}
- if (SOUND && !BookmarksUtils.all(transaction.isValid))
- SOUND.beep();
- var isCancelled = !BookmarksUtils.any(transaction.isValid);
- if (!isCancelled) {
- BMSVC.transactionManager.doTransaction(transaction);
- BookmarksUtils.notifyBookmarksChanged();
- BookmarksUtils.flushDataSource();
- }
- return !isCancelled;
+ txmgr.endBatch();
+ BookmarksUtils.notifyBookmarksChanged();
+ BookmarksUtils.flushDataSource();
+ return true;
},
moveSelection: function (aAction, aSelection, aTarget)
{
- aSelection.isValid = BookmarksUtils.isSelectionValidForInsertion(aSelection, aTarget, "move");
- var transaction = new BookmarkMoveTransaction(aAction, aSelection, aTarget);
- var isCancelled = !BookmarksUtils.any(transaction.isValid);
- if (!isCancelled) {
- BMSVC.transactionManager.doTransaction(transaction);
- } else if (SOUND)
+ var isValid = BookmarksUtils.isSelectionValidForInsertion(aSelection, aTarget, aAction);
+ if (SOUND && !BookmarksUtils.all(isValid))
SOUND.beep();
- return !isCancelled;
+ if (!BookmarksUtils.any(isValid))
+ return false;
+
+ var txmgr = BMSVC.transactionManager;
+ txmgr.beginBatch();
+ var index = aTarget.index;
+ for (var i = 0; i < aSelection.length; ++i) {
+ if (isValid[i]) {
+ RDFC.Init(BMDS, aSelection.parent[i]);
+ var deleteIndex = RDFC.IndexOf(aSelection.item[i]);
+ var deleteTxn = BMSVC.createTransaction(aSelection.parent[i], aSelection.item[i], deleteIndex, true);
+ txmgr.doTransaction(deleteTxn);
+ var insertTxn = BMSVC.createTransaction(aTarget.parent, aSelection.item[i], index++, false);
+ txmgr.doTransaction(insertTxn);
+ }
+ }
+ txmgr.endBatch();
+ BookmarksUtils.flushDataSource();
+ return true;
},
getXferDataFromSelection: function (aSelection)
{
if (aSelection.length == 0)
return null;
var dataSet = new TransferDataSet();
var data, item, itemUrl, itemName, parent, name;
@@ -1646,204 +1642,8 @@ var BookmarksUtils = {
if (!target)
return;
var rSource = RDF.GetResource(aEvent.target.id);
var selection = BookmarksUtils.getSelectionFromResource(rSource);
BookmarksCommand.openBookmark(selection, target, aDS, aEvent)
}
}
-
-
-function BookmarkTransaction()
-{
-}
-
-BookmarkTransaction.prototype = {
- BATCH_LIMIT : 8,
- RDFC : null,
- BMDS : null,
-
- beginUpdateBatch: function()
- {
- if (this.item.length > this.BATCH_LIMIT) {
- this.BMDS.beginUpdateBatch();
- }
- },
-
- endUpdateBatch: function()
- {
- if (this.item.length > this.BATCH_LIMIT) {
- this.BMDS.endUpdateBatch();
- }
- },
-
- // nsITransaction method stubs
- doTransaction: function() {},
- undoTransaction: function() {},
- redoTransaction: function() { this.doTransaction(); },
- get isTransient() { return false; },
- merge: function(aTransaction) { return false; },
-
- // debugging helper
- get wrappedJSObject() { return this; }
-}
-
-function BookmarkInsertTransaction (aAction)
-{
- this.type = "insert";
- this.action = aAction;
- this.item = null;
- this.parent = null;
- this.index = null;
- this.isValid = null;
-}
-
-BookmarkInsertTransaction.prototype =
-{
- __proto__: BookmarkTransaction.prototype,
-
- doTransaction: function ()
- {
- this.beginUpdateBatch();
- for (var i=0; i<this.item.length; ++i) {
- if (this.isValid[i]) {
- this.RDFC.Init(this.BMDS, this.parent[i]);
- this.RDFC.InsertElementAt(this.item[i], this.index[i], true);
- }
- }
- this.endUpdateBatch();
- },
-
- undoTransaction: function ()
- {
- this.beginUpdateBatch();
- // XXXvarga Can't use |RDFC| here because it's being "reused" elsewhere.
- var container = Components.classes[kRDFCContractID].createInstance(kRDFCIID);
- for (var i=this.item.length-1; i>=0; i--) {
- if (this.isValid[i]) {
- container.Init(this.BMDS, this.parent[i]);
- container.RemoveElementAt(this.index[i], true);
- }
- }
- this.endUpdateBatch();
- }
-
-}
-
-function BookmarkRemoveTransaction (aAction)
-{
- this.type = "remove";
- this.action = aAction;
- this.item = null;
- this.parent = null;
- this.index = null;
- this.isValid = null;
-}
-
-BookmarkRemoveTransaction.prototype =
-{
- __proto__: BookmarkTransaction.prototype,
-
- doTransaction: function ()
- {
- this.beginUpdateBatch();
- for (var i=0; i<this.item.length; ++i) {
- if (this.isValid[i]) {
- this.RDFC.Init(this.BMDS, this.parent[i]);
- this.RDFC.RemoveElementAt(this.index[i], false);
- }
- }
- this.endUpdateBatch();
- },
-
- undoTransaction: function ()
- {
- this.beginUpdateBatch();
- for (var i=this.item.length-1; i>=0; i--) {
- if (this.isValid[i]) {
- this.RDFC.Init(this.BMDS, this.parent[i]);
- this.RDFC.InsertElementAt(this.item[i], this.index[i], false);
- }
- }
- this.endUpdateBatch();
- }
-
-}
-
-function BookmarkMoveTransaction (aAction, aSelection, aTarget)
-{
- this.type = "move";
- this.action = aAction;
- this.selection = aSelection;
- this.target = aTarget;
- this.isValid = aSelection.isValid;
-}
-
-BookmarkMoveTransaction.prototype =
-{
- __proto__: BookmarkTransaction.prototype,
-
- beginUpdateBatch: function()
- {
- if (this.selection.length > this.BATCH_LIMIT) {
- this.BMDS.beginUpdateBatch();
- }
- },
-
- endUpdateBatch: function()
- {
- if (this.selection.length > this.BATCH_LIMIT) {
- this.BMDS.endUpdateBatch();
- }
- },
-
- doTransaction: function ()
- {
- this.beginUpdateBatch();
- BookmarksUtils.removeSelection("move", this.selection);
- BookmarksUtils.insertSelection("move", this.selection, this.target);
- this.endUpdateBatch();
- },
-
- redoTransaction : function () {}
-
-}
-
-function BookmarkImportTransaction (aAction)
-{
- this.type = "import";
- this.action = aAction;
- this.item = [];
- this.parent = [];
- this.index = [];
- this.isValid = [];
-}
-
-BookmarkImportTransaction.prototype =
-{
- __proto__: BookmarkTransaction.prototype,
-
- undoTransaction: function ()
- {
- this.beginUpdateBatch();
- for (var i=this.item.length-1; i>=0; i--) {
- if (this.isValid[i]) {
- this.RDFC.Init(this.BMDS, this.parent[i]);
- this.RDFC.RemoveElementAt(this.index[i], true);
- }
- }
- this.endUpdateBatch();
- },
-
- redoTransaction: function ()
- {
- this.beginUpdateBatch();
- for (var i=0; i<this.item.length; ++i) {
- if (this.isValid[i]) {
- this.RDFC.Init(this.BMDS, this.parent[i]);
- this.RDFC.InsertElementAt(this.item[i], this.index[i], true);
- }
- }
- this.endUpdateBatch();
- }
-
-}
--- a/suite/common/bookmarks/bookmarksTree.xml
+++ b/suite/common/bookmarks/bookmarksTree.xml
@@ -359,38 +359,31 @@
}
selection.selectEventsSuppressed = false;
}
]]></body>
</method>
<field name="_itemToBeToggled"> []</field>
- <field name="_parentToBeToggled">[]</field>
<method name="preUpdateTreeSelection">
<parameter name="aTxn"/>
<body><![CDATA[
- aTxn = aTxn.wrappedJSObject;
- var type = aTxn.type;
- // Skip transactions that aggregates nested "insert" or "remove" transactions.
- if (type != "insert" && type != "remove")
- return;
- for (var i=0; i<aTxn.item.length; ++i) {
- this._itemToBeToggled .push(aTxn.item [i]);
- this._parentToBeToggled.push(aTxn.parent[i]);
- }
+ // aTxn could be null, or possibly some third-party transaction?
+ if (aTxn instanceof Components.interfaces.nsIInterfaceRequestor)
+ this._itemToBeToggled.push(aTxn.getInterface(Components.interfaces.nsIRDFNode));
]]></body>
</method>
<method name="updateTreeSelection">
<body><![CDATA[
this.treeBoxObject.view.selection.selectEventsSuppressed = true;
this.treeBoxObject.view.selection.clearSelection();
for (var i=0; i<this._itemToBeToggled.length; ++i) {
- index = this.treeBuilder.getIndexOfResource(this._itemToBeToggled[i]);
+ var index = this.treeBuilder.getIndexOfResource(this._itemToBeToggled[i]);
if (index >=0)
this.treeBoxObject.view.selection.toggleSelect(index);
}
this.treeBoxObject.view.selection.selectEventsSuppressed = false;
]]></body>
</method>
<method name="createTreeContextMenu">
@@ -686,35 +679,32 @@
<!-- nsITransactionManager listener -->
<field name="transactionListener"><![CDATA[
({
mOuter: this,
willDo: function (aTxmgr, aTxn) {
this.mOuter._itemToBeToggled = [];
- this.mOuter._parentToBeToggled = [];
},
didDo: function (aTxmgr, aTxn) {
this.mOuter.preUpdateTreeSelection(aTxn);
},
willUndo: function (aTxmgr, aTxn) {
this.mOuter._itemToBeToggled = [];
- this.mOuter._parentToBeToggled = [];
},
didUndo: function (aTxmgr, aTxn) {
this.mOuter.preUpdateTreeSelection(aTxn);
},
willRedo: function (aTxmgr, aTxn) {
this.mOuter._itemToBeToggled = [];
- this.mOuter._parentToBeToggled = [];
},
didRedo: function (aTxmgr, aTxn) {
this.mOuter.preUpdateTreeSelection(aTxn);
},
didMerge : function (aTxmgr, aTxn) {},
didBeginBatch : function (aTxmgr, aTxn) {},