Bug 681420 - Improve responsiveness of history deletion.
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 31 Aug 2011 13:46:22 +0200
changeset 76266 7a8b51e5be412ea0ac1662ed36b53ca4ee939f73
parent 76265 2ace1d703abeac6d947f85363c96d26ee4b35df7
child 76267 69c025d6d230192ebea521a1d24fbd7b1e4ed9ef
child 76320 5565603c0b7dcddbfe925625bf90ae708c247506
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
bugs681420
milestone9.0a1
Bug 681420 - Improve responsiveness of history deletion. r=dietrich sr=rstrong
browser/components/places/content/controller.js
toolkit/components/places/nsIBrowserHistory.idl
toolkit/components/places/nsNavHistory.cpp
toolkit/components/places/nsNavHistoryResult.cpp
toolkit/components/places/nsNavHistoryResult.h
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -34,41 +34,36 @@
  * decision by deleting the provisions above and replace them with the notice
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
+                                  "resource://gre/modules/NetUtil.jsm");
 
 // XXXmano: we should move most/all of these constants to PlacesUtils
 const ORGANIZER_ROOT_BOOKMARKS = "place:folder=BOOKMARKS_MENU&excludeItems=1&queryType=1";
 const ORGANIZER_SUBSCRIPTIONS_QUERY = "place:annotation=livemark%2FfeedURI";
 
 // No change to the view, preserve current selection
 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 avoid passing
-// a too big array to RemovePages
-// 300 is the best choice with an history of about 150000 visits
-// smaller chunks could cause a Slow Script warning with a huge history
+// When removing a bunch of pages we split them in chunks to give some breath
+// to the main-thread.
 const REMOVE_PAGES_CHUNKLEN = 300;
-// if we are removing less than this pages we will remove them one by one
-// since it will be reflected faster on the UI
-// 10 is a good compromise, since allows the user to delete a little amount of
-// urls for privacy reasons, but does not cause heavy disk access
-const REMOVE_PAGES_MAX_SINGLEREMOVES = 10;
 
 /**
  * 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
@@ -264,17 +259,17 @@ PlacesController.prototype = {
       break;
     case "placesCmd_deleteDataHost":
       var host;
       if (PlacesUtils.nodeIsHost(this._view.selectedNode)) {
         var queries = this._view.selectedNode.getQueries();
         host = queries[0].domain;
       }
       else
-        host = PlacesUtils._uri(this._view.selectedNode.uri).host;
+        host = NetUtil.newURI(this._view.selectedNode.uri).host;
       PlacesUIUtils.privateBrowsing.removeDataFromDomain(host);
       break;
     case "cmd_selectAll":
       this.selectAll();
       break;
     case "placesCmd_open":
       PlacesUIUtils.openNodeIn(this._view.selectedNode, "current", this._view);
       break;
@@ -311,17 +306,17 @@ PlacesController.prototype = {
     case "placesCmd_createBookmark":
       let node = this._view.selectedNode;
       PlacesUIUtils.showBookmarkDialog({ action: "add"
                                        , type: "bookmark"
                                        , hiddenRows: [ "description"
                                                      , "keyword"
                                                      , "location"
                                                      , "loadInSidebar" ]
-                                       , uri: PlacesUtils._uri(node.uri)
+                                       , uri: NetUtil.newURI(node.uri)
                                        , title: node.title
                                        }, window.top, true);
       break;
     }
   },
 
   onEvent: function PC_onEvent(eventName) { },
 
@@ -505,17 +500,17 @@ PlacesController.prototype = {
           break;
         case Ci.nsINavHistoryResultNode.RESULT_TYPE_SEPARATOR:
           nodeData["separator"] = true;
           break;
         case Ci.nsINavHistoryResultNode.RESULT_TYPE_URI:
         case Ci.nsINavHistoryResultNode.RESULT_TYPE_VISIT:
         case Ci.nsINavHistoryResultNode.RESULT_TYPE_FULL_VISIT:
           nodeData["link"] = true;
-          uri = PlacesUtils._uri(node.uri);
+          uri = NetUtil.newURI(node.uri);
           if (PlacesUtils.nodeIsBookmark(node)) {
             nodeData["bookmark"] = true;
             PlacesUtils.nodeIsTagQuery(node.parent)
 
             var parentNode = node.parent;
             if (parentNode) {
               if (PlacesUtils.nodeIsTagQuery(parentNode))
                 nodeData["tagChild"] = true;
@@ -882,17 +877,17 @@ PlacesController.prototype = {
       var node = range[i];
       if (this._shouldSkipNode(node, removedFolders))
         continue;
 
       if (PlacesUtils.nodeIsTagQuery(node.parent)) {
         // This is a uri node inside a tag container.  It needs a special
         // untag transaction.
         var tagItemId = PlacesUtils.getConcreteItemId(node.parent);
-        var uri = PlacesUtils._uri(node.uri);
+        var uri = NetUtil.newURI(node.uri);
         transactions.push(PlacesUIUtils.ptm.untagURI(uri, [tagItemId]));
       }
       else if (PlacesUtils.nodeIsTagQuery(node) && node.parent &&
                PlacesUtils.nodeIsQuery(node.parent) &&
                PlacesUtils.asQuery(node.parent).queryOptions.resultType ==
                  Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY) {
         // This is a tag container.
         // Untag all URIs tagged with this tag only if the tag container is
@@ -903,18 +898,17 @@ PlacesController.prototype = {
         for (var j = 0; j < URIs.length; j++)
           transactions.push(PlacesUIUtils.ptm.untagURI(URIs[j], [tag]));
       }
       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.
-        var bhist = PlacesUtils.history.QueryInterface(Ci.nsIBrowserHistory);
-        bhist.removePage(PlacesUtils._uri(node.uri));
+        PlacesUtils.bhistory.removePage(NetUtil.newURI(node.uri));
         // 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
@@ -950,83 +944,79 @@ PlacesController.prototype = {
     if (transactions.length > 0) {
       var txn = PlacesUIUtils.ptm.aggregateTransactions(txnName, transactions);
       PlacesUIUtils.ptm.doTransaction(txn);
     }
   },
 
   /**
    * Removes the set of selected ranges from history.
+   *
+   * @note history deletes are not undoable.
    */
   _removeRowsFromHistory: function PC__removeRowsFromHistory() {
-    // Other containers are history queries, just delete from history
-    // history deletes are not undoable.
-    var nodes = this._view.selectedNodes;
-    var URIs = [];
-    var bhist = PlacesUtils.history.QueryInterface(Ci.nsIBrowserHistory);
-    var root = this._view.result.root;
-
-    for (var i = 0; i < nodes.length; ++i) {
-      var node = nodes[i];
+    let nodes = this._view.selectedNodes;
+    let URIs = [];
+    for (let i = 0; i < nodes.length; ++i) {
+      let node = nodes[i];
       if (PlacesUtils.nodeIsURI(node)) {
-        var uri = PlacesUtils._uri(node.uri);
-        // avoid trying to delete the same url twice
+        let uri = NetUtil.newURI(node.uri);
+        // Avoid duplicates.
         if (URIs.indexOf(uri) < 0) {
           URIs.push(uri);
         }
       }
       else if (PlacesUtils.nodeIsQuery(node) &&
                PlacesUtils.asQuery(node).queryOptions.queryType ==
-                 Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY)
+                 Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY) {
         this._removeHistoryContainer(node);
+      }
     }
 
-    // if we have to delete a lot of urls RemovePage will be slow, it's better
-    // to delete them in bunch and rebuild the full treeView
-    if (URIs.length > REMOVE_PAGES_MAX_SINGLEREMOVES) {
-      // do removal in chunks to avoid passing a too big array to removePages
-      for (var i = 0; i < URIs.length; i += REMOVE_PAGES_CHUNKLEN) {
-        var URIslice = URIs.slice(i, i + REMOVE_PAGES_CHUNKLEN);
-        // set DoBatchNotify (third param) only on the last chunk, so we update
-        // the treeView when we are done.
-        bhist.removePages(URIslice, URIslice.length,
-                          (i + REMOVE_PAGES_CHUNKLEN) >= URIs.length);
+    // 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(function() {
+          try {
+            gen.next();
+          } catch (ex if ex instanceof StopIteration) {}
+        }, Ci.nsIThread.DISPATCH_NORMAL); 
+        yield;
       }
     }
-    else {
-      // if we have to delete fewer urls, removepage will allow us to avoid
-      // rebuilding the full treeView
-      for (var i = 0; i < URIs.length; ++i)
-        bhist.removePage(URIs[i]);
-    }
+    let gen = pagesChunkGenerator(URIs);
+    gen.next();
   },
 
   /**
    * Removes history visits for an history container node.
    * @param   [in] aContainerNode
    *          The container node to remove.
+   *
+   * @note history deletes are not undoable.
    */
-  _removeHistoryContainer: function PC_removeHistoryContainer(aContainerNode) {
-    var bhist = PlacesUtils.history.QueryInterface(Ci.nsIBrowserHistory);
+  _removeHistoryContainer: function PC__removeHistoryContainer(aContainerNode) {
     if (PlacesUtils.nodeIsHost(aContainerNode)) {
       // Site container.
-      bhist.removePagesFromHost(aContainerNode.title, true);
+      PlacesUtils.bhistory.removePagesFromHost(aContainerNode.title, true);
     }
     else if (PlacesUtils.nodeIsDay(aContainerNode)) {
       // Day container.
-      var query = aContainerNode.getQueries()[0];
-      var beginTime = query.beginTime;
-      var endTime = query.endTime;
+      let query = aContainerNode.getQueries()[0];
+      let beginTime = query.beginTime;
+      let endTime = query.endTime;
       NS_ASSERT(query && beginTime && endTime,
                 "A valid date container query should exist!");
       // We want to exclude beginTime from the removal because
       // removePagesByTimeframe includes both extremes, while date containers
       // exclude the lower extreme.  So, if we would not exclude it, we would
       // end up removing more history than requested.
-      bhist.removePagesByTimeframe(beginTime+1, endTime);
+      PlacesUtils.bhistory.removePagesByTimeframe(beginTime + 1, endTime);
     }
   },
 
   /**
    * Removes the selection
    * @param   aTxnName
    *          A name for the transaction if this is being performed
    *          as part of another operation.
@@ -1294,17 +1284,17 @@ PlacesController.prototype = {
 
     let transactions = [];
     let insertionIndex = ip.index;
     for (let i = 0; i < items.length; ++i) {
       if (ip.isTag) {
         // Pasting into a tag container means tagging the item, regardless of
         // the requested action.
         transactions.push(
-          new PlacesTagURITransaction(PlacesUtils._uri(items[i].uri),
+          new PlacesTagURITransaction(NetUtil.newURI(items[i].uri),
                                       [ip.itemId])
         );
         continue;
       }
 
       // Adjust index to make sure items are pasted in the correct position.
       // If index is DEFAULT_INDEX, items are just appended.
       if (ip.index != PlacesUtils.bookmarks.DEFAULT_INDEX)
@@ -1544,17 +1534,17 @@ let PlacesControllerDragHelper = {
       let dragginUp = insertionPoint.itemId == unwrapped.parent &&
                       index < PlacesUtils.bookmarks.getItemIndex(unwrapped.id);
       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 = PlacesUtils._uri(unwrapped.uri);
+        let uri = NetUtil.newURI(unwrapped.uri);
         let tagItemId = insertionPoint.itemId;
         transactions.push(PlacesUIUtils.ptm.tagURI(uri,[tagItemId]));
       }
       else {
         transactions.push(PlacesUIUtils.makeTransaction(unwrapped,
                           flavor, insertionPoint.itemId,
                           index, doCopy));
       }
--- a/toolkit/components/places/nsIBrowserHistory.idl
+++ b/toolkit/components/places/nsIBrowserHistory.idl
@@ -37,151 +37,149 @@
 
 /*
  * browser-specific interface to global history
  */
 
 #include "nsISupports.idl"
 #include "nsIGlobalHistory2.idl"
 
-[scriptable, uuid(540aca25-1e01-467f-b24c-df89cbe40f8d)]
+[scriptable, uuid(212371ab-d8b9-4835-b867-d0eb78c0cb18)]
 interface nsIBrowserHistory : nsIGlobalHistory2
 {
     /**
-     * addPageWithDetails
+     * Used by the History migrator to add a page to global history, with a
+     * specific title and last visit time.
      *
-     * Adds a page to history with specific time stamp information. This is used in
-     * the History migrator. 
+     * @param aURI
+     *        URI of the page to be added.
+     * @param aTitle
+     *        Title of the page.
+     * @param aLastvisited
+     *        Microseconds from epoch representing the last visit time.
      */
-    void addPageWithDetails(in nsIURI aURI, in wstring aTitle, in long long aLastVisited);
+    void addPageWithDetails(in nsIURI aURI,
+                            in wstring aTitle,
+                            in long long aLastVisited);
 
     /**
-     * lastPageVisited
-     *
      * The last page that was visited in a top-level window.
      */
     readonly attribute AUTF8String lastPageVisited;
 
     /**
-     * count
+     * Indicates if there are entries in global history.
      *
-     * Indicate if there are entries in global history
-     * For performance reasons this does not return the real number of entries
+     * @note For performance reasons this is not the real number of entries.
+     *       It will instead evaluate to 0 for no entries, 1 otherwise.
      */
     readonly attribute PRUint32 count;
 
     /**
-     * removePage
+     * Removes a page from global history.
      *
-     * Remove a page from history
+     * @note It is preferrable to use this one rather then RemovePages when
+     *       removing less than 10 pages, since it won't start a full batch
+     *       operation.
      */
     void removePage(in nsIURI aURI);
 
     /**
-     * removePages
+     * Removes a list of pages from global history.
      *
-     * Remove a bunch of pages from history
-     * Notice that this does not call observers for performance reasons,
-     * instead setting aDoBatchNotify true will send Begin/EndUpdateBatch
+     * @param aURIs
+     *        Array of URIs to be removed.
+     * @param aLength
+     *        Length of the array.
+     *
+     * @note the removal happens in a batch.
      */
     void removePages([array, size_is(aLength)] in nsIURI aURIs,
-                     in unsigned long aLength, in boolean aDoBatchNotify);
+                     in unsigned long aLength);
 
     /**
-     * removePagesFromHost
+     * Removes all global history information about pages for a given host.
      *
-     * Removes all history information about pages from a given host. If
-     * aEntireDomain is set, we will also delete pages from sub hosts (so if
-     * we are passed in "microsoft.com" we delete "www.microsoft.com",
-     * "msdn.microsoft.com", etc.). An empty host name means local files and
-     * anything else with no host name. You can also pass in the localized
-     * "(local files)" title given to you from a history query to remove all
-     * history information from local files.
+     * @param aHost
+     *        Hostname to be removed.
+     *        An empty host name means local files and anything else with no
+     *        hostname.  You can also pass in the localized "(local files)"
+     *        title given to you from a history query to remove all
+     *        history information from local files.
+     * @param aEntireDomain
+     *        If true, will also delete pages from sub hosts (so if
+     *        passed in "microsoft.com" will delete "www.microsoft.com",
+     *        "msdn.microsoft.com", etc.).
      *
-     * Note that this does not call observers for single deleted uris,
-     * but will send Begin/EndUpdateBatch.
+     * @note The removal happens in a batch.
      */
-    void removePagesFromHost(in AUTF8String aHost, in boolean aEntireDomain);
+    void removePagesFromHost(in AUTF8String aHost,
+                             in boolean aEntireDomain);
 
     /**
-     * removePagesByTimeframe
-     *
-     * Remove all pages for a given timeframe.
+     * Removes all pages for a given timeframe.
      * Limits are included: aBeginTime <= timeframe <= aEndTime
-     * Notice that this does not call observers for single deleted uris,
-     * instead it will send Begin/EndUpdateBatch
+     *
+     * @param aBeginTime
+     *        Microseconds from epoch, representing the initial time.
+     * @param aEndTime
+     *        Microseconds from epoch, representing the final time.
+     *
+     * @note The removal happens in a batch.
      */
-    void removePagesByTimeframe(in long long aBeginTime, in long long aEndTime);
+    void removePagesByTimeframe(in long long aBeginTime,
+                                in long long aEndTime);
 
     /**
-     * removeVisitsByTimeframe
-     *
-     * Removes all visits in a given timeframe.  Limits are included:
-     * aBeginTime <= timeframe <= aEndTime.  Any place that becomes unvisited
-     * as a result will also be deleted.
+     * Removes all visits in a given timeframe.
+     * Limits are included: aBeginTime <= timeframe <= aEndTime.
+     * Any pages that becomes unvisited as a result will also be deleted.
      *
-     * Note that removal is performed in batch, so observers will not be
-     * notified of individual places that are deleted.  Instead they will be
-     * notified onBeginUpdateBatch and onEndUpdateBatch.
+     * @param aBeginTime
+     *        Microseconds from epoch, representing the initial time.
+     * @param aEndTime
+     *        Microseconds from epoch, representing the final time.
+     *
+     * @note The removal happens in a batch.
      */
-    void removeVisitsByTimeframe(in long long aBeginTime, in long long aEndTime);
+    void removeVisitsByTimeframe(in long long aBeginTime,
+                                 in long long aEndTime);
 
     /**
-     * removeAllPages
+     * Removes all existing pages from global history.
+     * Visits are removed synchronously, but pages are expired asynchronously
+     * off the main-thread.
      *
-     * Remove all pages from global history
+     * @note The removal happens in a batch. Single removals are not notified,
+     *       instead an onClearHistory notification is sent to
+     *       nsINavHistoryObserver implementers.
      */
     void removeAllPages();
 
     /**
-     * hidePage
+     * Hides the specified URL from being enumerated (and thus displayed in
+     * the UI).
      *
-     * Hide the specified URL from being enumerated (and thus
-     * displayed in the UI)
-     * If the page hasn't been visited yet, then it will be added
-     * as if it was visited, and then marked as hidden
+     * @param aURI
+     *        URI of the page to be marked.
+     *
+     * @note If the page hasn't been visited yet, then it will be added
+     *       as if it was visited, and then marked as hidden
      */
     void hidePage(in nsIURI aURI);
 
     /**
-     * markPageAsTyped
+     * Designates the url as having been explicitly typed in by the user.
      *
-     * Designate the url as having been explicitly typed in by
-     * the user, so it's okay to be an autocomplete result.
+     * @param aURI
+     *        URI of the page to be marked.
      */
     void markPageAsTyped(in nsIURI aURI);
 
     /**
-     * markPageAsFollowedLink
+     * Designates the url as coming from a link explicitly followed by
+     * the user (for example by clicking on it).
      *
-     * Designate the url as coming from a link explicitly followed by
-     * the user (for example by clicking on it).
+     * @param aURI
+     *        URI of the page to be marked.
      */
     void markPageAsFollowedLink(in nsIURI aURI);
-
-    /**
-     * Mark a page as being currently open.
-     *
-     * @note Pages will not be automatically unregistered when Private Browsing
-     *       mode is entered or exited.  Therefore, consumers MUST unregister or
-     *       register themselves.
-     *
-     * @note This is just an alias for mozIPlacesAutoComplete::registerOpenPage.
-     *
-     * @status DEPRECATED
-     */
-    [deprecated] void registerOpenPage(in nsIURI aURI);
-
-    /**
-     * Mark a page as no longer being open (either by closing the window or tab,
-     * or by navigating away from that page).
-     *
-     * @note Pages will not be automatically unregistered when Private Browsing
-     *       mode is entered or exited.  Therefore, consumers MUST unregister or
-     *       register themselves.
-     *
-     * @note This is just an alias for
-     *       mozIPlacesAutoComplete::unregisterOpenPage.
-     *
-     * @status DEPRECATED
-     */
-    [deprecated] void unregisterOpenPage(in nsIURI aURI);
 };
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -4254,22 +4254,20 @@ nsNavHistory::CleanupPlacesOnVisitsDelet
   return NS_OK;
 }
 
 
 // nsNavHistory::RemovePages
 //
 //    Removes a bunch of uris from history.
 //    Has better performance than RemovePage when deleting a lot of history.
-//    Notice that this function does not call the onDeleteURI observers,
-//    instead, if aDoBatchNotify is true, we call OnBegin/EndUpdateBatch.
 //    We don't do duplicates removal, URIs array should be cleaned-up before.
 
 NS_IMETHODIMP
-nsNavHistory::RemovePages(nsIURI **aURIs, PRUint32 aLength, PRBool aDoBatchNotify)
+nsNavHistory::RemovePages(nsIURI **aURIs, PRUint32 aLength)
 {
   NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");
   NS_ENSURE_ARG(aURIs);
 
   nsresult rv;
   // build a list of place ids to delete
   nsCString deletePlaceIdsQueryString;
   for (PRUint32 i = 0; i < aLength; i++) {
@@ -4279,19 +4277,17 @@ nsNavHistory::RemovePages(nsIURI **aURIs
     NS_ENSURE_SUCCESS(rv, rv);
     if (placeId != 0) {
       if (!deletePlaceIdsQueryString.IsEmpty())
         deletePlaceIdsQueryString.AppendLiteral(",");
       deletePlaceIdsQueryString.AppendInt(placeId);
     }
   }
 
-  // force a full refresh calling onEndUpdateBatch (will call Refresh())
-  if (aDoBatchNotify)
-    UpdateBatchScoper batch(*this); // sends Begin/EndUpdateBatch to observers
+  UpdateBatchScoper batch(*this); // sends Begin/EndUpdateBatch to observers
 
   rv = RemovePagesInternal(deletePlaceIdsQueryString);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Clear the registered embed visits.
   clearEmbedVisits();
 
   return NS_OK;
@@ -4304,19 +4300,32 @@ nsNavHistory::RemovePages(nsIURI **aURIs
 //    Silently fails if we have no knowledge of the page.
 
 NS_IMETHODIMP
 nsNavHistory::RemovePage(nsIURI *aURI)
 {
   NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");
   NS_ENSURE_ARG(aURI);
 
-  nsIURI** URIs = &aURI;
-  nsresult rv = RemovePages(URIs, 1, PR_FALSE);
-  NS_ENSURE_SUCCESS(rv, rv);
+  // Build a list of place ids to delete.
+  PRInt64 placeId;
+  nsCAutoString guid;
+  nsresult rv = GetIdForPage(aURI, &placeId, guid);
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (placeId == 0) {
+    return NS_OK;
+  }
+  nsCAutoString deletePlaceIdQueryString;
+  deletePlaceIdQueryString.AppendInt(placeId);
+
+  rv = RemovePagesInternal(deletePlaceIdQueryString);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Clear the registered embed visits.
+  clearEmbedVisits();
 
   return NS_OK;
 }
 
 
 // nsNavHistory::RemovePagesFromHost
 //
 //    This function will delete all history information about pages from a
@@ -4668,50 +4677,16 @@ nsNavHistory::MarkPageAsFollowedLink(nsI
   if (mRecentLink.Count() > RECENT_EVENT_QUEUE_MAX_LENGTH)
     ExpireNonrecentEvents(&mRecentLink);
 
   mRecentLink.Put(uriString, GetNow());
   return NS_OK;
 }
 
 
-NS_IMETHODIMP
-nsNavHistory::RegisterOpenPage(nsIURI* aURI)
-{
-  NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");
-  NS_ENSURE_ARG(aURI);
-
-  nsCOMPtr<mozIPlacesAutoComplete> ac =
-    do_GetService("@mozilla.org/autocomplete/search;1?name=history");
-  NS_ENSURE_STATE(ac);
-
-  nsresult rv = ac->RegisterOpenPage(aURI);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  return NS_OK;
-}
-
-
-NS_IMETHODIMP
-nsNavHistory::UnregisterOpenPage(nsIURI* aURI)
-{
-  NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");
-  NS_ENSURE_ARG(aURI);
-
-  nsCOMPtr<mozIPlacesAutoComplete> ac =
-    do_GetService("@mozilla.org/autocomplete/search;1?name=history");
-  NS_ENSURE_STATE(ac);
-
-  nsresult rv = ac->UnregisterOpenPage(aURI);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  return NS_OK;
-}
-
-
 // nsNavHistory::SetCharsetForURI
 //
 // Sets the character-set for a URI.
 // If aCharset is empty remove character-set annotation for aURI.
 
 NS_IMETHODIMP
 nsNavHistory::SetCharsetForURI(nsIURI* aURI,
                                const nsAString& aCharset)
--- a/toolkit/components/places/nsNavHistoryResult.cpp
+++ b/toolkit/components/places/nsNavHistoryResult.cpp
@@ -87,16 +87,20 @@
 // expand the macro here and change it so that it works. Yuck.
 #define NS_INTERFACE_MAP_STATIC_AMBIGUOUS(_class) \
   if (aIID.Equals(NS_GET_IID(_class))) { \
     NS_ADDREF(this); \
     *aInstancePtr = this; \
     return NS_OK; \
   } else
 
+// Number of changes to handle separately in a batch.  If more changes are
+// requested the node will switch to full refresh mode.
+#define MAX_BATCH_CHANGES_BEFORE_REFRESH 5
+
 // Emulate string comparison (used for sorting) for PRTime and int.
 inline PRInt32 ComparePRTime(PRTime a, PRTime b)
 {
   if (LL_CMP(a, <, b))
     return -1;
   else if (LL_CMP(a, >, b))
     return 1;
   return 0;
@@ -2288,29 +2292,31 @@ NS_IMPL_ISUPPORTS_INHERITED1(nsNavHistor
 nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode(
     const nsACString& aTitle, const nsACString& aIconURI,
     const nsACString& aQueryURI) :
   nsNavHistoryContainerResultNode(aQueryURI, aTitle, aIconURI,
                                   nsNavHistoryResultNode::RESULT_TYPE_QUERY,
                                   PR_TRUE, EmptyCString(), nsnull),
   mLiveUpdate(QUERYUPDATE_COMPLEX_WITH_BOOKMARKS),
   mHasSearchTerms(PR_FALSE),
-  mContentsValid(PR_FALSE)
+  mContentsValid(PR_FALSE),
+  mBatchChanges(0)
 {
 }
 
 nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode(
     const nsACString& aTitle, const nsACString& aIconURI,
     const nsCOMArray<nsNavHistoryQuery>& aQueries,
     nsNavHistoryQueryOptions* aOptions) :
   nsNavHistoryContainerResultNode(EmptyCString(), aTitle, aIconURI,
                                   nsNavHistoryResultNode::RESULT_TYPE_QUERY,
                                   PR_TRUE, EmptyCString(), aOptions),
   mQueries(aQueries),
-  mContentsValid(PR_FALSE)
+  mContentsValid(PR_FALSE),
+  mBatchChanges(0)
 {
   NS_ASSERTION(aQueries.Count() > 0, "Must have at least one query");
 
   nsNavHistory* history = nsNavHistory::GetHistoryService();
   NS_ASSERTION(history, "History service missing");
   if (history) {
     mLiveUpdate = history->GetUpdateRequirements(mQueries, mOptions,
                                                  &mHasSearchTerms);
@@ -2321,17 +2327,18 @@ nsNavHistoryQueryResultNode::nsNavHistor
     const nsACString& aTitle, const nsACString& aIconURI,
     PRTime aTime,
     const nsCOMArray<nsNavHistoryQuery>& aQueries,
     nsNavHistoryQueryOptions* aOptions) :
   nsNavHistoryContainerResultNode(EmptyCString(), aTitle, aTime, aIconURI,
                                   nsNavHistoryResultNode::RESULT_TYPE_QUERY,
                                   PR_TRUE, EmptyCString(), aOptions),
   mQueries(aQueries),
-  mContentsValid(PR_FALSE)
+  mContentsValid(PR_FALSE),
+  mBatchChanges(0)
 {
   NS_ASSERTION(aQueries.Count() > 0, "Must have at least one query");
 
   nsNavHistory* history = nsNavHistory::GetHistoryService();
   NS_ASSERTION(history, "History service missing");
   if (history) {
     mLiveUpdate = history->GetUpdateRequirements(mQueries, mOptions,
                                                  &mHasSearchTerms);
@@ -2862,16 +2869,18 @@ nsNavHistoryQueryResultNode::OnEndUpdate
 {
   // If the query has no children it's possible it's not yet listening to
   // bookmarks changes, in such a case it's safer to force a refresh to gather
   // eventual new nodes matching query options.
   if (mChildren.Count() == 0) {
     nsresult rv = Refresh();
     NS_ENSURE_SUCCESS(rv, rv);
   }
+
+  mBatchChanges = 0;
   return NS_OK;
 }
 
 
 /**
  * Here we need to update all copies of the URI we have with the new visit
  * count, and potentially add a new entry in our query.  This is the most
  * common update operation and it is important that it be as efficient as
@@ -2880,16 +2889,25 @@ nsNavHistoryQueryResultNode::OnEndUpdate
 NS_IMETHODIMP
 nsNavHistoryQueryResultNode::OnVisit(nsIURI* aURI, PRInt64 aVisitId,
                                      PRTime aTime, PRInt64 aSessionId,
                                      PRInt64 aReferringId,
                                      PRUint32 aTransitionType,
                                      const nsACString& aGUID,
                                      PRUint32* aAdded)
 {
+  nsNavHistoryResult* result = GetResult();
+  NS_ENSURE_STATE(result);
+  if (result->mBatchInProgress &&
+      ++mBatchChanges > MAX_BATCH_CHANGES_BEFORE_REFRESH) {
+    nsresult rv = Refresh();
+    NS_ENSURE_SUCCESS(rv, rv);
+    return NS_OK;
+  }
+
   nsNavHistory* history = nsNavHistory::GetHistoryService();
   NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
 
   nsresult rv;
   nsRefPtr<nsNavHistoryResultNode> addition;
   switch(mLiveUpdate) {
 
     case QUERYUPDATE_HOST: {
@@ -3005,16 +3023,25 @@ nsNavHistoryQueryResultNode::OnTitleChan
     // When we are not expanded, we don't update, just invalidate and unhook.
     // It would still be pretty easy to traverse the results and update the
     // titles, but when a title changes, its unlikely that it will be the only
     // thing.  Therefore, we just give up.
     ClearChildren(PR_TRUE);
     return NS_OK; // no updates in tree state
   }
 
+  nsNavHistoryResult* result = GetResult();
+  NS_ENSURE_STATE(result);
+  if (result->mBatchInProgress &&
+      ++mBatchChanges > MAX_BATCH_CHANGES_BEFORE_REFRESH) {
+    nsresult rv = Refresh();
+    NS_ENSURE_SUCCESS(rv, rv);
+    return NS_OK;
+  }
+
   // compute what the new title should be
   NS_ConvertUTF16toUTF8 newTitle(aPageTitle);
 
   PRBool onlyOneEntry = (mOptions->ResultType() ==
                          nsINavHistoryQueryOptions::RESULTS_AS_URI ||
                          mOptions->ResultType() ==
                          nsINavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS
                          );
@@ -3077,16 +3104,25 @@ nsNavHistoryQueryResultNode::OnBeforeDel
  * Here, we can always live update by just deleting all occurrences of
  * the given URI.
  */
 NS_IMETHODIMP
 nsNavHistoryQueryResultNode::OnDeleteURI(nsIURI* aURI,
                                          const nsACString& aGUID,
                                          PRUint16 aReason)
 {
+  nsNavHistoryResult* result = GetResult();
+  NS_ENSURE_STATE(result);
+  if (result->mBatchInProgress &&
+      ++mBatchChanges > MAX_BATCH_CHANGES_BEFORE_REFRESH) {
+    nsresult rv = Refresh();
+    NS_ENSURE_SUCCESS(rv, rv);
+    return NS_OK;
+  }
+
   if (IsContainersQuery()) {
     // Incremental updates of query returning queries are pretty much
     // complicated.  In this case it's possible one of the child queries has
     // no more children and it should be removed.  Unfortunately there is no
     // way to know that without executing the child query and counting results.
     nsresult rv = Refresh();
     NS_ENSURE_SUCCESS(rv, rv);
     return NS_OK;
--- a/toolkit/components/places/nsNavHistoryResult.h
+++ b/toolkit/components/places/nsNavHistoryResult.h
@@ -785,16 +785,18 @@ public:
 
   virtual PRUint16 GetSortType();
   virtual void GetSortingAnnotation(nsACString& aSortingAnnotation);
   virtual void RecursiveSort(const char* aData,
                              SortComparator aComparator);
 
   nsCOMPtr<nsIURI> mRemovingURI;
   nsresult NotifyIfTagsChanged(nsIURI* aURI);
+
+  PRUint32 mBatchChanges;
 };
 
 
 // nsNavHistoryFolderResultNode
 //
 //    Overridden container type for bookmark folders. It will keep the contents
 //    of the folder in sync with the bookmark service.