Bug 1067956 - Async Places Transactions: Drop functionality. r=mak.
authorAsaf Romano <mano@mozilla.com>
Fri, 19 Sep 2014 17:04:51 +0300
changeset 206242 913568167ec54fb13c34285495275de26638cf2d
parent 206241 6ef192784187d7a477dfa402ce0a5b3a755d542e
child 206243 6c2940e4d2834ef25572649998c0232b204761b7
push id27517
push userryanvm@gmail.com
push dateFri, 19 Sep 2014 18:13:39 +0000
treeherdermozilla-central@a084c4cfd8a1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1067956
milestone35.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
Bug 1067956 - Async Places Transactions: Drop functionality. r=mak.
browser/components/places/content/browserPlacesViews.js
browser/components/places/content/controller.js
browser/components/places/content/menu.xml
browser/components/places/content/treeView.js
toolkit/components/places/PlacesTransactions.jsm
toolkit/components/places/tests/unit/test_async_transactions.js
--- 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();
+});