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 179595 8774539497c7a91e0ff0e66b0d629beb653a676d
parent 179490 d6cf23c13dde5fe7c605035b16a97345a0e1ea97
child 179596 f57788b66994e27c6bc79deaf5f6bca8effd62e2
push id26631
push userryanvm@gmail.com
push dateTue, 22 Apr 2014 20:05:15 +0000
treeherdermozilla-central@1ab07aa4d004 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs993391
milestone31.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 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