Bug 993391 - Places async transactions: Implement "move bookmarks" command. r=mak.
authorAsaf Romano <mano@mozilla.com>
Tue, 22 Apr 2014 08:42:23 +0300
changeset 180023 8774539497c7a91e0ff0e66b0d629beb653a676d
parent 179897 d6cf23c13dde5fe7c605035b16a97345a0e1ea97
child 180024 f57788b66994e27c6bc79deaf5f6bca8effd62e2
push id272
push userpvanderbeken@mozilla.com
push dateMon, 05 May 2014 16:31:18 +0000
reviewersmak
bugs993391
milestone31.0a1
Bug 993391 - Places async transactions: Implement "move bookmarks" command. r=mak.
browser/components/places/content/controller.js
browser/components/places/content/moveBookmarks.js
toolkit/components/places/PlacesTransactions.jsm
toolkit/components/places/tests/unit/test_async_transactions.js
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -128,17 +128,16 @@ PlacesController.prototype = {
     if (PlacesUIUtils.useAsyncTransactions) {
       switch (aCommand) {
       case "cmd_cut":
       case "placesCmd_cut":
       case "cmd_copy":
       case "cmd_paste":
       case "cmd_delete":
       case "placesCmd_delete":
-      case "placesCmd_moveBookmarks":
       case "cmd_paste":
       case "placesCmd_paste":
       case "placesCmd_new:folder":
       case "placesCmd_new:bookmark":
       case "placesCmd_createBookmark":
         return false;
       }
     }
--- a/browser/components/places/content/moveBookmarks.js
+++ b/browser/components/places/content/moveBookmarks.js
@@ -18,37 +18,48 @@ var gMoveBookmarksDialog = {
     this._nodes = window.arguments[0];
 
     this.foldersTree.place =
       "place:excludeItems=1&excludeQueries=1&excludeReadOnlyFolders=1&folder=" +
       PlacesUIUtils.allBookmarksFolderId;
   },
 
   onOK: function MBD_onOK(aEvent) {
-    var selectedNode = this.foldersTree.selectedNode;
-    NS_ASSERT(selectedNode,
-              "selectedNode must be set in a single-selection tree with initial selection set");
-    var selectedFolderID = PlacesUtils.getConcreteItemId(selectedNode);
+    let selectedNode = this.foldersTree.selectedNode;
+    let selectedFolderId = PlacesUtils.getConcreteItemId(selectedNode);
+
+    if (!PlacesUIUtils.useAsyncTransactions) {
+      let transactions = [];
+      for (var i=0; i < this._nodes.length; i++) {
+        // Nothing to do if the node is already under the selected folder
+        if (this._nodes[i].parent.itemId == selectedFolderId)
+          continue;
 
-    var transactions = [];
-    for (var i=0; i < this._nodes.length; i++) {
-      // Nothing to do if the node is already under the selected folder
-      if (this._nodes[i].parent.itemId == selectedFolderID)
-        continue;
-
-      let txn = new PlacesMoveItemTransaction(this._nodes[i].itemId,
-                                              selectedFolderID,
-                                              PlacesUtils.bookmarks.DEFAULT_INDEX);
-      transactions.push(txn);
+        let txn = new PlacesMoveItemTransaction(this._nodes[i].itemId,
+                                                selectedFolderId,
+                                                PlacesUtils.bookmarks.DEFAULT_INDEX);
+        transactions.push(txn);
+      }
+      if (transactions.length != 0) {
+        let txn = new PlacesAggregatedTransaction("Move Items", transactions);
+        PlacesUtils.transactionManager.doTransaction(txn);
+      }
+      return;
     }
 
-    if (transactions.length != 0) {
-      let txn = new PlacesAggregatedTransaction("Move Items", transactions);
-      PlacesUtils.transactionManager.doTransaction(txn);
-    }
+    PlacesTransactions.transact(function* () {
+      let newParentGUID = yield PlacesUtils.promiseItemGUID(selectedFolderId);
+      for (let node of this._nodes) {
+        // Nothing to do if the node is already under the selected folder.
+        if (node.parent.itemId == selectedFolderId)
+          continue;
+        yield PlacesTransactions.MoveItem({ GUID: node.bookmarkGuid
+                                          , newParentGUID: newParentGUID });
+      }
+    }.bind(this)).then(null, Components.utils.reportError);
   },
 
   newFolder: function MBD_newFolder() {
     // The command is disabled when the tree is not focused
     this.foldersTree.focus();
     goDoCommand("placesCmd_new:folder");
   }
 };
--- a/toolkit/components/places/PlacesTransactions.jsm
+++ b/toolkit/components/places/PlacesTransactions.jsm
@@ -346,26 +346,30 @@ let PlacesTransactions = {
    * @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) {
-    let generatorMode = typeof(aToTransact) == "function";
-    if (generatorMode) {
-      if (!aToTransact.isGenerator())
+    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");
     }
-    else {
-      if (!TransactionsHistory.isProxifiedTransactionObject(aToTransact))
-        throw new Error("aToTransact is not a valid transaction object");
-      if (executedTransactions.has(aToTransact))
-        throw new Error("Transactions objects may not be recycled.");
+    else if (!TransactionsHistory.isProxifiedTransactionObject(aToTransact)) {
+      throw new Error("aToTransact is not a valid transaction object");
+    }
+    else if (executedTransactions.has(aToTransact)) {
+      throw new Error("Transactions objects may not be recycled.");
     }
 
     return Serialize(function* () {
       // The entry in the transactions history is created once the first
       // transaction is committed. This means that if |transact| is called
       // in its "generator mode" and no transactions are committed by the
       // generator, the transactions history is left unchanged.
       // Bug 982115: Depending on how this API is actually used we may revise
@@ -382,17 +386,17 @@ let PlacesTransactions = {
 
       function* transactBatch(aGenerator) {
         let error = false;
         let sendValue = undefined;
         while (true) {
           let next = error ?
                      aGenerator.throw(sendValue) : aGenerator.next(sendValue);
           sendValue = next.value;
-          if (Object.prototype.toString.call(sendValue) == "[object Generator]") {
+          if (isGeneratorObj(sendValue)) {
             sendValue = yield transactBatch(sendValue);
           }
           else if (typeof(sendValue) == "object" && sendValue) {
             if (TransactionsHistory.isProxifiedTransactionObject(sendValue)) {
               if (executedTransactions.has(sendValue)) {
                 sendValue = new Error("Transactions may not be recycled.");
                 error = true;
               }
@@ -405,18 +409,18 @@ let PlacesTransactions = {
             }
           }
           if (next.done)
             break;
         }
         return sendValue;
       }
 
-      if (generatorMode)
-        return yield transactBatch(aToTransact());
+      if (generator)
+        return yield transactBatch(generator);
       else
         return yield transactOneTransaction(aToTransact);
     }.bind(this));
   },
 
   /**
    * Asynchronously undo the transaction immediately after the current undo
    * position in the transactions history in the reverse order, if any, and
@@ -882,19 +886,20 @@ PT.NewLivemark.prototype = Object.seal({
     };
     return guid;
   }
 });
 
 /**
  * Transaction for moving an item.
  *
- * Required Input Properties: GUID, newParentGUID, newIndex.
+ * Required Input Properties: GUID, newParentGUID.
+ * Optional Input Properties  newIndex.
  */
-PT.MoveItem = DefineTransaction(["GUID", "newParentGUID", "newIndex"]);
+PT.MoveItem = DefineTransaction(["GUID", "newParentGUID"], ["newIndex"]);
 PT.MoveItem.prototype = Object.seal({
   execute: function* (aGUID, aNewParentGUID, aNewIndex) {
     let itemId = yield PlacesUtils.promiseItemId(aGUID),
         oldParentId = PlacesUtils.bookmarks.getFolderIdForItem(itemId),
         oldIndex = PlacesUtils.bookmarks.getItemIndex(itemId),
         newParentId = yield PlacesUtils.promiseItemId(aNewParentGUID);
 
     PlacesUtils.bookmarks.moveItem(itemId, newParentId, aNewIndex);
--- a/toolkit/components/places/tests/unit/test_async_transactions.js
+++ b/toolkit/components/places/tests/unit/test_async_transactions.js
@@ -449,18 +449,17 @@ add_task(function* test_move_items_to_fo
     let bkm_b_txn = PT.NewBookmark(bkm_b_info);
     bkm_b_info.GUID = yield bkm_b_txn;
     return [folder_a_txn, bkm_a_txn, bkm_b_txn];
   });
 
   ensureUndoState([[bkm_b_txn, bkm_a_txn, folder_a_txn]], 0);
 
   let moveTxn = PT.MoveItem({ GUID:          bkm_a_info.GUID
-                            , newParentGUID: folder_a_info.GUID
-                            , newIndex:      bmsvc.DEFAULT_INDEX });
+                            , newParentGUID: folder_a_info.GUID });
   yield PT.transact(moveTxn);
 
   let ensureDo = () => {
     ensureUndoState([[moveTxn], [bkm_b_txn, bkm_a_txn, folder_a_txn]], 0);
     ensureItemsMoved({ GUID:          bkm_a_info.GUID
                      , oldParentGUID: folder_a_info.GUID
                      , newParentGUID: folder_a_info.GUID
                      , oldIndex:      0