Bug 681420 - Improve responsiveness of history deletion.

r=dietrich sr=rstrong
This commit is contained in:
Marco Bonardo 2011-08-31 13:46:22 +02:00
parent e85785bdd4
commit 22ac610317
5 changed files with 182 additions and 181 deletions

View File

@ -39,6 +39,8 @@
* ***** 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";
@ -54,16 +56,9 @@ const RELOAD_ACTION_REMOVE = 2;
// 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
@ -269,7 +264,7 @@ PlacesController.prototype = {
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":
@ -316,7 +311,7 @@ PlacesController.prototype = {
, "keyword"
, "location"
, "loadInSidebar" ]
, uri: PlacesUtils._uri(node.uri)
, uri: NetUtil.newURI(node.uri)
, title: node.title
}, window.top, true);
break;
@ -510,7 +505,7 @@ PlacesController.prototype = {
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)
@ -887,7 +882,7 @@ PlacesController.prototype = {
// 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 &&
@ -908,8 +903,7 @@ PlacesController.prototype = {
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 &&
@ -955,73 +949,69 @@ PlacesController.prototype = {
/**
* 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);
}
},
@ -1299,7 +1289,7 @@ PlacesController.prototype = {
// 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;
@ -1549,7 +1539,7 @@ let PlacesControllerDragHelper = {
// 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]));
}

View File

@ -42,146 +42,144 @@
#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
* @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 pages that becomes unvisited as a result will also be deleted.
*
* 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.
* @param aBeginTime
* Microseconds from epoch, representing the initial time.
* @param aEndTime
* Microseconds from epoch, representing the final time.
*
* 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.
* @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
* @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
*
* Designate the url as coming from a link explicitly followed by
* Designates 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);
};

View File

@ -4259,12 +4259,10 @@ nsNavHistory::CleanupPlacesOnVisitsDelete(const nsCString& aPlaceIdsQueryString)
//
// 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);
@ -4284,8 +4282,6 @@ nsNavHistory::RemovePages(nsIURI **aURIs, PRUint32 aLength, PRBool aDoBatchNotif
}
}
// force a full refresh calling onEndUpdateBatch (will call Refresh())
if (aDoBatchNotify)
UpdateBatchScoper batch(*this); // sends Begin/EndUpdateBatch to observers
rv = RemovePagesInternal(deletePlaceIdsQueryString);
@ -4309,9 +4305,22 @@ 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);
// 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;
}
@ -4673,40 +4682,6 @@ nsNavHistory::MarkPageAsFollowedLink(nsIURI *aURI)
}
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.

View File

@ -92,6 +92,10 @@
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)
{
@ -2293,7 +2297,8 @@ nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode(
PR_TRUE, EmptyCString(), nsnull),
mLiveUpdate(QUERYUPDATE_COMPLEX_WITH_BOOKMARKS),
mHasSearchTerms(PR_FALSE),
mContentsValid(PR_FALSE)
mContentsValid(PR_FALSE),
mBatchChanges(0)
{
}
@ -2305,7 +2310,8 @@ nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode(
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");
@ -2326,7 +2332,8 @@ nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode(
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");
@ -2867,6 +2874,8 @@ nsNavHistoryQueryResultNode::OnEndUpdateBatch()
nsresult rv = Refresh();
NS_ENSURE_SUCCESS(rv, rv);
}
mBatchChanges = 0;
return NS_OK;
}
@ -2885,6 +2894,15 @@ nsNavHistoryQueryResultNode::OnVisit(nsIURI* aURI, PRInt64 aVisitId,
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);
@ -3010,6 +3028,15 @@ nsNavHistoryQueryResultNode::OnTitleChanged(nsIURI* aURI,
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);
@ -3082,6 +3109,15 @@ 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

View File

@ -790,6 +790,8 @@ public:
nsCOMPtr<nsIURI> mRemovingURI;
nsresult NotifyIfTagsChanged(nsIURI* aURI);
PRUint32 mBatchChanges;
};