Bug 1378711 - Ensure that current places transactions have completed before clearing batch processing mode to avoid messing up the undo queue. r?mak draft
authorMark Banner <standard8@mozilla.com>
Thu, 06 Jul 2017 11:57:44 +0100
changeset 608914 2cdfae2a524e2d32e876f256ae9bd8f8d842e967
parent 608658 67cd1ee26f2661fa5efe3d952485ab3c89af4271
child 637447 f5b32e05ba791713d80b46c00ceb1e4da0eb0ed8
push id68441
push userbmo:standard8@mozilla.com
push dateFri, 14 Jul 2017 09:41:17 +0000
reviewersmak
bugs1378711
milestone56.0a1
Bug 1378711 - Ensure that current places transactions have completed before clearing batch processing mode to avoid messing up the undo queue. r?mak MozReview-Commit-ID: 7k0sgZIaUyB
browser/components/places/tests/browser/browser_bookmarksProperties.js
toolkit/components/places/PlacesTransactions.jsm
--- a/browser/components/places/tests/browser/browser_bookmarksProperties.js
+++ b/browser/components/places/tests/browser/browser_bookmarksProperties.js
@@ -312,37 +312,29 @@ gTests.push({
       if (event.attrName != "place")
         return;
       folderTree.removeEventListener("DOMAttrModified", arguments.callee);
       executeSoon(async function() {
         await self._addObserver;
         let bookmark = await PlacesUtils.bookmarks.fetch({url: TEST_URL});
         self._bookmarkGuid = bookmark.guid;
 
-        // TODO: Bug 1378711. We shouldn't need to wait for onItemChanged here,
-        // however we are working around an async bug in the PlacesTransactions
-        // manager and ensuring that async functions have completed before moving
-        // on.
-        let promiseItemChanged = PlacesTestUtils.waitForNotification("onItemChanged",
-          (itemId, property, isAnnotationProperty, newValue, lastModified, itemType) =>
-            itemType === PlacesUtils.bookmarks.TYPE_FOLDER && isAnnotationProperty);
-
         // Create a new folder.
         var newFolderButton = self.window.document.getElementById("editBMPanel_newFolderButton");
         newFolderButton.doCommand();
 
         // Wait for the folder to be created and for editing to start.
         await BrowserTestUtils.waitForCondition(() => folderTree.hasAttribute("editing"),
            "We are editing new folder name in folder tree");
 
         // Press Escape to discard editing new folder name.
         EventUtils.synthesizeKey("VK_ESCAPE", {}, self.window);
         Assert.ok(!folderTree.hasAttribute("editing"),
            "We have finished editing folder name in folder tree");
-        await promiseItemChanged;
+
         self._cleanShutdown = true;
         self._removeObserver = PlacesTestUtils.waitForNotification("onItemRemoved",
           (itemId, parentId, index, type, uri, guid) => guid == self._bookmarkGuid);
 
         self.window.document.documentElement.cancelDialog();
       });
     });
     foldersExpander.doCommand();
--- a/toolkit/components/places/PlacesTransactions.jsm
+++ b/toolkit/components/places/PlacesTransactions.jsm
@@ -558,21 +558,24 @@ var TransactionsManager = {
   },
 
   batch(task) {
     return this._mainEnqueuer.enqueue(async () => {
       this._batching = true;
       this._createdBatchEntry = false;
       let rv;
       try {
-        // We should return here, but bug 958949 makes that impossible.
         rv = await task();
       } finally {
-        this._batching = false;
-        this._createdBatchEntry = false;
+        // We must enqueue clearing batching mode to ensure that any existing
+        // transactions have completed before we clear the batching mode.
+        this._mainEnqueuer.enqueue(async () => {
+          this._batching = false;
+          this._createdBatchEntry = false;
+        });
       }
       return rv;
     });
   },
 
   /**
    * Undo the top undo entry, if any, and update the undo position accordingly.
    */