Bug 681420 - Improve responsiveness of history deletion.
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 31 Aug 2011 13:46:22 +0200
changeset 77577 7a8b51e5be412ea0ac1662ed36b53ca4ee939f73
parent 77576 2ace1d703abeac6d947f85363c96d26ee4b35df7
child 77578 69c025d6d230192ebea521a1d24fbd7b1e4ed9ef
child 77631 5565603c0b7dcddbfe925625bf90ae708c247506
push id78
push userclegnitto@mozilla.com
push dateFri, 16 Dec 2011 17:32:24 +0000
treeherdermozilla-release@79d24e644fdd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs681420
milestone9.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 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.