author | Asaf Romano <mano@mozilla.com> |
Fri, 19 Sep 2014 17:04:51 +0300 | |
changeset 206252 | 913568167ec54fb13c34285495275de26638cf2d |
parent 206251 | 6ef192784187d7a477dfa402ce0a5b3a755d542e |
child 206253 | 6c2940e4d2834ef25572649998c0232b204761b7 |
push id | 49381 |
push user | ryanvm@gmail.com |
push date | Fri, 19 Sep 2014 18:14:36 +0000 |
treeherder | mozilla-inbound@206e963fb3dd [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | mak |
bugs | 1067956 |
milestone | 35.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/browser/components/places/content/browserPlacesViews.js +++ b/browser/components/places/content/browserPlacesViews.js @@ -1635,16 +1635,17 @@ PlacesToolbar.prototype = { }, _onDrop: function PT__onDrop(aEvent) { PlacesControllerDragHelper.currentDropTarget = aEvent.target; let dropPoint = this._getDropPoint(aEvent); if (dropPoint && dropPoint.ip) { PlacesControllerDragHelper.onDrop(dropPoint.ip, aEvent.dataTransfer) + .then(null, Components.utils.reportError); aEvent.preventDefault(); } this._cleanupDragDetails(); aEvent.stopPropagation(); }, _onDragExit: function PT__onDragExit(aEvent) {
--- a/browser/components/places/content/controller.js +++ b/browser/components/places/content/controller.js @@ -52,17 +52,17 @@ function InsertionPoint(aItemId, aIndex, this.dropNearItemId = aDropNearItemId; } InsertionPoint.prototype = { set index(val) { return this._index = val; }, - promiseGUID: function () PlacesUtils.promiseItemGUID(this.itemId), + promiseGuid: function () PlacesUtils.promiseItemGUID(this.itemId), get index() { if (this.dropNearItemId > 0) { // If dropNearItemId is set up we must calculate the real index of // the item near which we will drop. var index = PlacesUtils.bookmarks.getItemIndex(this.dropNearItemId); return this.orientation == Ci.nsITreeView.DROP_BEFORE ? index : index + 1; } @@ -776,17 +776,17 @@ PlacesController.prototype = { PlacesUtils.transactionManager.doTransaction(txn); // Select the new item. let insertedNodeId = PlacesUtils.bookmarks .getIdForItemAt(ip.itemId, ip.index); this._view.selectItems([insertedNodeId], false); return; } - let txn = PlacesTransactions.NewSeparator({ parentGUID: yield ip.promiseGUID() + let txn = PlacesTransactions.NewSeparator({ parentGUID: yield ip.promiseGuid() , index: ip.index }); let guid = yield PlacesTransactions.transact(txn); let itemId = yield PlacesUtils.promiseItemId(guid); // Select the new item. this._view.selectItems([itemId], false); }), /** @@ -1288,17 +1288,17 @@ PlacesController.prototype = { let uris = [for (item of items) if ("uri" in item) NetUtil.newURI(item.uri)]; yield PlacesTransactions.transact( PlacesTransactions.Tag({ uris: uris, tag: ip.tagName })); } else { yield PlacesTransactions.transact(function *() { let insertionIndex = ip.index; - let parent = yield ip.promiseGUID(); + let parent = yield ip.promiseGuid(); for (let item of items) { let doCopy = action == "copy"; // If this is not a copy, check for safety that we can move the // source, otherwise report an error and fallback to a copy. if (!doCopy && !PlacesControllerDragHelper.canMoveUnwrappedNode(item)) { @@ -1598,22 +1598,25 @@ let PlacesControllerDragHelper = { return true; }, /** * Handles the drop of one or more items onto a view. * @param insertionPoint * The insertion point where the items should be dropped */ - onDrop: function PCDH_onDrop(insertionPoint, dt) { + onDrop: Task.async(function* (insertionPoint, dt) { let doCopy = ["copy", "link"].indexOf(dt.dropEffect) != -1; let transactions = []; let dropCount = dt.mozItemCount; let movedCount = 0; + let parentGuid = PlacesUIUtils.useAsyncTransactions ? + (yield insertionPoint.promiseGuid()) : null; + let tagName = insertionPoint.tagName; for (let i = 0; i < dropCount; ++i) { let flavor = this.getFirstValidFlavor(dt.mozTypesAt(i)); if (!flavor) return; let data = dt.mozGetDataAt(flavor, i); let unwrapped; if (flavor != TAB_DROP_TYPE) { @@ -1642,36 +1645,53 @@ let PlacesControllerDragHelper = { if (index != -1 && dragginUp) index+= movedCount++; // If dragging over a tag container we should tag the item. if (insertionPoint.isTag && insertionPoint.orientation == Ci.nsITreeView.DROP_ON) { let uri = NetUtil.newURI(unwrapped.uri); let tagItemId = insertionPoint.itemId; - let tagTxn = new PlacesTagURITransaction(uri, [tagItemId]); - transactions.push(tagTxn); + if (PlacesUIUtils.useAsyncTransactions) + transactions.push(PlacesTransactions.Tag({ uri: uri, tag: tagName })); + else + transactions.push(new PlacesTagURITransaction(uri, [tagItemId])); } else { // If this is not a copy, check for safety that we can move the source, // otherwise report an error and fallback to a copy. if (!doCopy && !PlacesControllerDragHelper.canMoveUnwrappedNode(unwrapped)) { Components.utils.reportError("Tried to move an unmovable Places node, " + "reverting to a copy operation."); doCopy = true; } - transactions.push(PlacesUIUtils.makeTransaction(unwrapped, - flavor, insertionPoint.itemId, - index, doCopy)); + if (PlacesUIUtils.useAsyncTransactions) { + transactions.push( + PlacesUIUtils.getTransactionForData(unwrapped, + flavor, + parentGuid, + index, + doCopy)); + } + else { + transactions.push(PlacesUIUtils.makeTransaction(unwrapped, + flavor, insertionPoint.itemId, + index, doCopy)); + } } } - let txn = new PlacesAggregatedTransaction("DropItems", transactions); - PlacesUtils.transactionManager.doTransaction(txn); - }, + if (PlacesUIUtils.useAsyncTransactions) { + yield PlacesTransactions.transact(transactions); + } + else { + let txn = new PlacesAggregatedTransaction("DropItems", transactions); + PlacesUtils.transactionManager.doTransaction(txn); + } + }), /** * Checks if we can insert into a container. * @param aContainer * The container were we are want to drop */ disallowInsertion: function(aContainer) { NS_ASSERT(aContainer, "empty container");
--- a/browser/components/places/content/menu.xml +++ b/browser/components/places/content/menu.xml @@ -362,17 +362,18 @@ event.stopPropagation(); ]]></handler> <handler event="drop"><![CDATA[ PlacesControllerDragHelper.currentDropTarget = event.target; let dropPoint = this._getDropPoint(event); if (dropPoint && dropPoint.ip) { - PlacesControllerDragHelper.onDrop(dropPoint.ip, event.dataTransfer); + PlacesControllerDragHelper.onDrop(dropPoint.ip, event.dataTransfer) + .then(null, Components.utils.reportError); event.preventDefault(); } this._cleanupDragDetails(); event.stopPropagation(); ]]></handler> <handler event="dragover"><![CDATA[
--- a/browser/components/places/content/treeView.js +++ b/browser/components/places/content/treeView.js @@ -1350,18 +1350,20 @@ PlacesTreeView.prototype = { dropNearItemId); }, drop: function PTV_drop(aRow, aOrientation, aDataTransfer) { // We are responsible for translating the |index| and |orientation| // parameters into a container id and index within the container, // since this information is specific to the tree view. let ip = this._getInsertionPoint(aRow, aOrientation); - if (ip) - PlacesControllerDragHelper.onDrop(ip, aDataTransfer); + if (ip) { + PlacesControllerDragHelper.onDrop(ip, aDataTransfer) + .then(null, Components.utils.reportError); + } PlacesControllerDragHelper.currentDropTarget = null; }, getParentIndex: function PTV_getParentIndex(aRow) { let [parentNode, parentRow] = this._getParentByChildRow(aRow); return parentRow; },
--- a/toolkit/components/places/PlacesTransactions.jsm +++ b/toolkit/components/places/PlacesTransactions.jsm @@ -317,45 +317,61 @@ executedTransactions.add = k => executed let PlacesTransactions = { /** * Asynchronously transact either a single transaction, or a sequence of * transactions that would be treated as a single entry in the transactions * history. * * @param aToTransact - * Either a transaction object or a generator function (ES6-style only) - * that yields transaction objects. + * Either a transaction object or an array of transaction objects, or a + * generator function (ES6-style only) that yields transaction objects. * * Generator mode how-to: when a transaction is yielded, it's executed. * Then, if it was executed successfully, the resolution of |execute| * is sent to the generator. If |execute| threw or rejected, the * exception is propagated to the generator. * Any other value yielded by a generator function is handled the * same way as in a Task (see Task.jsm). * * @return {Promise} - * @resolves either to the resolution of |execute|, in single-transaction mode, - * or to the return value of the generator, in generator-mode. + * @resolves either to the resolution of |execute|, in single-transaction + * mode, or to the return value of the generator, in generator-mode. For an + * array of transactions, there's no resolution value. + * * @rejects either if |execute| threw, in single-transaction mode, or if * the generator function threw (or didn't handle) an exception, in generator * mode. * @throws if aTransactionOrGeneratorFunction is neither a transaction object - * created by this module or a generator function. + * created by this module, nor an array of such object, nor a generator + * function. * @note If no transaction was executed successfully, the transactions history * is not affected. * * @note All PlacesTransactions operations are serialized. This means that the * transactions history state may change by the time the transaction/generator * is processed. It's guaranteed, however, that a generator function "blocks" * the queue: that is, it is assured that no other operations are performed * by or on PlacesTransactions until the generator returns. Keep in mind you * are not protected from consumers who use the raw places APIs directly. */ transact: function (aToTransact) { + if (Array.isArray(aToTransact)) { + if (aToTransact.some( + o => !TransactionsHistory.isProxifiedTransactionObject(o))) { + throw new Error("aToTransact contains non-transaction element"); + } + // Proceed as if a generator yielding the transactions was passed in. + return this.transact(function* () { + for (let t of aToTransact) { + yield t; + } + }); + } + let isGeneratorObj = o => Object.prototype.toString.call(o) == "[object Generator]"; let generator = null; if (typeof(aToTransact) == "function") { generator = aToTransact(); if (!isGeneratorObj(generator)) throw new Error("aToTransact is not a generator function");
--- a/toolkit/components/places/tests/unit/test_async_transactions.js +++ b/toolkit/components/places/tests/unit/test_async_transactions.js @@ -1478,9 +1478,46 @@ add_task(function* test_copy() { yield PT.NewSeparator({ parentGUID: folderGUID }); // And another bookmark. yield PT.NewBookmark({ uri: NetUtil.newURI("http://nested.bookmark") , parentGUID: folderGUID }); return folderGUID; }); yield duplicate_and_test(filledFolderGUID); + + // Cleanup + yield PT.clearTransactionsHistory(); }); + +add_task(function* test_array_input_for_transact() { + let rootGuid = yield PlacesUtils.promiseItemGUID(root); + + let folderTxn = PT.NewFolder(yield createTestFolderInfo()); + let folderGuid = yield PT.transact(folderTxn); + + let sep1_txn = PT.NewSeparator({ parentGUID: folderGuid }); + let sep2_txn = PT.NewSeparator({ parentGUID: folderGuid }); + yield PT.transact([sep1_txn, sep2_txn]); + ensureUndoState([[sep2_txn, sep1_txn], [folderTxn]], 0); + + let ensureChildCount = function* (count) { + let tree = yield PlacesUtils.promiseBookmarksTree(folderGuid); + if (count == 0) + Assert.ok(!("children" in tree)); + else + Assert.equal(tree.children.length, count); + }; + + yield ensureChildCount(2); + yield PT.undo(); + yield ensureChildCount(0); + yield PT.redo() + yield ensureChildCount(2); + yield PT.undo(); + yield ensureChildCount(0); + + yield PT.undo(); + Assert.equal((yield PlacesUtils.promiseBookmarksTree(folderGuid)), null); + + // Cleanup + yield PT.clearTransactionsHistory(); +});