Bug 1087255 - Convert browser/components/places JS clients of RemovePage(s) to History.remove; Patch by Yoric, updated by Standard8. r=mak
authorDavid Rajchenbach-Teller <dteller@mozilla.com>
Fri, 31 Oct 2014 12:39:02 +0100
changeset 500824 7ceee02e947617160e58b44122ea016cea95ac0c
parent 500823 4975b3ae236b2f5842d2b24b17e6911f025123c2
child 500825 023faad1dc96fba84125e3ace46dfa0e1019b30e
push id49816
push userbmo:tchiovoloni@mozilla.com
push dateFri, 17 Mar 2017 20:44:02 +0000
reviewersmak
bugs1087255
milestone55.0a1
Bug 1087255 - Convert browser/components/places JS clients of RemovePage(s) to History.remove; Patch by Yoric, updated by Standard8. r=mak MozReview-Commit-ID: GcVajWOyvkT
browser/components/places/content/controller.js
browser/components/places/tests/chrome/test_bug549192.xul
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -18,20 +18,16 @@ const RELOAD_ACTION_NOTHING = 0;
 // Inserting items new to the view, select the inserted rows
 const RELOAD_ACTION_INSERT = 1;
 // Removing items from the view, select the first item after the last selected
 const RELOAD_ACTION_REMOVE = 2;
 // Moving items within a view, don't treat the dropped items as additional
 // rows.
 const RELOAD_ACTION_MOVE = 3;
 
-// When removing a bunch of pages we split them in chunks to give some breath
-// to the main-thread.
-const REMOVE_PAGES_CHUNKLEN = 300;
-
 /**
  * Represents an insertion point within a container where we can insert
  * items.
  * @param   aItemId
  *          The identifier of the parent container
  * @param   aIndex
  *          The index within the container where we should insert
  * @param   aOrientation
@@ -887,17 +883,17 @@ PlacesController.prototype = {
             transactions.push(txn);
           }
         }
       } else if (PlacesUtils.nodeIsURI(node) &&
                PlacesUtils.nodeIsQuery(node.parent) &&
                PlacesUtils.asQuery(node.parent).queryOptions.queryType ==
                  Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY) {
         // This is a uri node inside an history query.
-        PlacesUtils.bhistory.removePage(NetUtil.newURI(node.uri));
+        PlacesUtils.history.removePage(node.uri).catch(Components.utils.reportError);
         // History deletes are not undoable, so we don't have a transaction.
       } else if (node.itemId == -1 &&
                PlacesUtils.nodeIsQuery(node) &&
                PlacesUtils.asQuery(node).queryOptions.queryType ==
                  Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY) {
         // This is a dynamically generated history query, like queries
         // grouped by site, time or both.  Dynamically generated queries don't
         // have an itemId even if they are descendants of a bookmark.
@@ -940,50 +936,35 @@ PlacesController.prototype = {
       } else {
         var txn = new PlacesAggregatedTransaction(txnName, transactions);
         PlacesUtils.transactionManager.doTransaction(txn);
       }
     }
   }),
 
   /**
-   * Removes the set of selected ranges from history.
+   * Removes the set of selected ranges from history, asynchronously.
    *
    * @note history deletes are not undoable.
    */
   _removeRowsFromHistory: function PC__removeRowsFromHistory() {
     let nodes = this._view.selectedNodes;
-    let URIs = [];
+    let URIs = new Set();
     for (let i = 0; i < nodes.length; ++i) {
       let node = nodes[i];
       if (PlacesUtils.nodeIsURI(node)) {
-        let uri = NetUtil.newURI(node.uri);
-        // Avoid duplicates.
-        if (URIs.indexOf(uri) < 0) {
-          URIs.push(uri);
-        }
+        URIs.add(node.uri);
       } else if (PlacesUtils.nodeIsQuery(node) &&
                PlacesUtils.asQuery(node).queryOptions.queryType ==
                  Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY) {
         this._removeHistoryContainer(node);
       }
     }
 
-    // Do removal in chunks to give some breath to main-thread.
-    function* pagesChunkGenerator(aURIs) {
-      while (aURIs.length) {
-        let URIslice = aURIs.splice(0, REMOVE_PAGES_CHUNKLEN);
-        PlacesUtils.bhistory.removePages(URIslice, URIslice.length);
-        Services.tm.mainThread.dispatch(() => gen.next(),
-                                        Ci.nsIThread.DISPATCH_NORMAL);
-        yield undefined;
-      }
-    }
-    let gen = pagesChunkGenerator(URIs);
-    gen.next();
+    PlacesUtils.history.remove([...URIs]).catch(Components.utils.reportError);
   },
 
   /**
    * Removes history visits for an history container node.
    * @param   [in] aContainerNode
    *          The container node to remove.
    *
    * @note history deletes are not undoable.
--- a/browser/components/places/tests/chrome/test_bug549192.xul
+++ b/browser/components/places/tests/chrome/test_bug549192.xul
@@ -36,16 +36,36 @@
   <script type="application/javascript"><![CDATA[
     /**
      * Bug 874407
      * Ensures that history views are updated properly after visits.
      * Bug 549192
      * Ensures that history views are updated after deleting entries.
      */
 
+    function promiseURIDeleted() {
+     return new Promise(resolve => {
+       let historyObserver = {
+         onBeginUpdateBatch: function PEX_onBeginUpdateBatch() {},
+         onEndUpdateBatch: function PEX_onEndUpdateBatch() {},
+         onClearHistory() {},
+         onVisit() {},
+         onTitleChanged() {},
+         onDeleteURI(aURI, aGUID, aReason) {
+           PlacesUtils.history.removeObserver(historyObserver);
+           resolve();
+         },
+         onPageChanged() {},
+         onDeleteVisits(aURI, aTime) { },
+       };
+
+       PlacesUtils.history.addObserver(historyObserver, false);
+     });
+    }
+
     function runTest() {
       SimpleTest.waitForExplicitFinish();
 
       Task.spawn(function* () {
         yield PlacesTestUtils.clearHistory();
 
         // Add some visits.
         let timeInMicroseconds = PlacesUtils.toPRTime(Date.now() - 10000);
@@ -101,20 +121,22 @@
           is(node.uri, places[rc - i - 1].uri.spec,
              "Found expected node at position " + i + ".");
         }
 
         // Now remove the pages and verify live-update again.
         for (let i = 0; i < rc; i++) {
           selection.select(0);
           let node = tree.selectedNode;
+          let promiseDeleted = promiseURIDeleted();
           tree.controller.remove("Removing page");
+          yield promiseDeleted;
           ok(treeView.treeIndexForNode(node) == Ci.nsINavHistoryResultTreeViewer.INDEX_INVISIBLE,
             node.uri + " removed.");
-          ok(treeView.rowCount == rc - i - 1, "Rows count decreased");
+          is(treeView.rowCount, rc - i - 1, "Rows count decreased");
         }
 
         // Cleanup.
         yield PlacesTestUtils.clearHistory();
       }).then(() => SimpleTest.finish());
     }
   ]]></script>
 </window>