--- a/toolkit/components/places/History.jsm
+++ b/toolkit/components/places/History.jsm
@@ -323,16 +323,19 @@ this.History = Object.freeze({
*
* @param filter: (object)
* The `object` may contain some of the following
* properties:
* - beginDate: (Date) Remove visits that have
* been added since this date (inclusive).
* - endDate: (Date) Remove visits that have
* been added before this date (inclusive).
+ * - limit: (Number) Limit the number of visits
+ * we remove to this number
+ * - url: (URL) Only remove visits to this URL
* If both `beginDate` and `endDate` are specified,
* visits between `beginDate` (inclusive) and `end`
* (inclusive) are removed.
*
* @param onResult: (function(VisitInfo), [optional])
* A callback invoked for each visit found and removed.
* Note that the referrer property of `VisitInfo`
* is NOT populated.
@@ -347,29 +350,43 @@ this.History = Object.freeze({
*/
removeVisitsByFilter: function(filter, onResult = null) {
if (!filter || typeof filter != "object") {
throw new TypeError("Expected a filter");
}
let hasBeginDate = "beginDate" in filter;
let hasEndDate = "endDate" in filter;
+ let hasURL = "url" in filter;
+ let hasLimit = "limit" in filter;
if (hasBeginDate) {
ensureDate(filter.beginDate);
}
if (hasEndDate) {
ensureDate(filter.endDate);
}
if (hasBeginDate && hasEndDate && filter.beginDate > filter.endDate) {
throw new TypeError("`beginDate` should be at least as old as `endDate`");
}
- if (!hasBeginDate && !hasEndDate) {
+ if (!hasBeginDate && !hasEndDate && !hasURL && !hasLimit) {
throw new TypeError("Expected a non-empty filter");
}
+ if (hasURL && !(filter.url instanceof URL) && typeof filter.url != "string" &&
+ !(filter.url instanceof Ci.nsIURI)) {
+ throw new TypeError("Expected a valid URL for `url`");
+ }
+
+ if (hasLimit &&
+ (typeof filter.limit != "number" ||
+ filter.limit <= 0 ||
+ !Number.isInteger(filter.limit))) {
+ throw new TypeError("Expected a non-zero positive integer as a limit");
+ }
+
if (onResult && typeof onResult != "function") {
throw new TypeError("Invalid function: " + onResult);
}
return PlacesUtils.withConnectionWrapper("History.jsm: removeVisitsByFilter",
db => removeVisitsByFilter(db, filter, onResult)
);
},
@@ -775,37 +792,53 @@ var notifyOnResult = Task.async(function
}
});
// Inner implementation of History.removeVisitsByFilter.
var removeVisitsByFilter = Task.async(function*(db, filter, onResult = null) {
// 1. Determine visits that took place during the interval. Note
// that the database uses microseconds, while JS uses milliseconds,
// so we need to *1000 one way and /1000 the other way.
- let dates = {
- conditions: [],
- args: {},
- };
+ let conditions = [];
+ let args = {};
if ("beginDate" in filter) {
- dates.conditions.push("visit_date >= :begin * 1000");
- dates.args.begin = Number(filter.beginDate);
+ conditions.push("v.visit_date >= :begin * 1000");
+ args.begin = Number(filter.beginDate);
}
if ("endDate" in filter) {
- dates.conditions.push("visit_date <= :end * 1000");
- dates.args.end = Number(filter.endDate);
+ conditions.push("v.visit_date <= :end * 1000");
+ args.end = Number(filter.endDate);
+ }
+ if ("limit" in filter) {
+ args.limit = Number(filter.limit);
}
+ let optionalJoin = "";
+ if ("url" in filter) {
+ let url = filter.url;
+ if (url instanceof Ci.nsIURI) {
+ url = filter.url.spec;
+ } else {
+ url = new URL(url).href;
+ }
+ optionalJoin = `JOIN moz_places h ON h.id = v.place_id`;
+ conditions.push("h.url_hash = hash(:url)", "h.url = :url");
+ args.url = url;
+ }
+
+
let visitsToRemove = [];
let pagesToInspect = new Set();
let onResultData = onResult ? [] : null;
yield db.executeCached(
- `SELECT id, place_id, visit_date / 1000 AS date, visit_type FROM moz_historyvisits
- WHERE ${ dates.conditions.join(" AND ") }`,
- dates.args,
+ `SELECT v.id, place_id, visit_date / 1000 AS date, visit_type FROM moz_historyvisits v
+ ${optionalJoin}
+ WHERE ${ conditions.join(" AND ") }${ args.limit ? " LIMIT :limit" : "" }`,
+ args,
row => {
let id = row.getResultByName("id");
let place_id = row.getResultByName("place_id");
visitsToRemove.push(id);
pagesToInspect.add(place_id);
if (onResult) {
onResultData.push({
--- a/toolkit/components/places/tests/history/test_removeVisitsByFilter.js
+++ b/toolkit/components/places/tests/history/test_removeVisitsByFilter.js
@@ -19,25 +19,35 @@ add_task(function* test_removeVisitsByFi
let SAMPLE_SIZE = options.sampleSize;
yield PlacesTestUtils.clearHistory();
yield PlacesUtils.bookmarks.eraseEverything();
// Populate the database.
// Create `SAMPLE_SIZE` visits, from the oldest to the newest.
- let bookmarks = new Set(options.bookmarks);
+ let bookmarkIndices = new Set(options.bookmarks);
let visits = [];
- let visitByURI = new Map();
+ let frecencyChangePromises = new Map();
+ let uriDeletePromises = new Map();
+ let getURL = options.url ?
+ i => "http://mozilla.com/test_browserhistory/test_removeVisitsByFilter/removeme/byurl/" + Math.floor(i / (SAMPLE_SIZE / 5)) + "/" :
+ i => "http://mozilla.com/test_browserhistory/test_removeVisitsByFilter/removeme/" + i + "/" + Math.random();
for (let i = 0; i < SAMPLE_SIZE; ++i) {
- let spec = "http://mozilla.com/test_browserhistory/test_removeVisitsByFilter/removeme/" + i + "/" + Math.random();
+ let spec = getURL(i);
let uri = NetUtil.newURI(spec);
let jsDate = new Date(Number(referenceDate) + 3600 * 1000 * i);
let dbDate = jsDate * 1000;
- let hasBookmark = bookmarks.has(i);
+ let hasBookmark = bookmarkIndices.has(i);
+ let hasOwnBookmark = hasBookmark;
+ if (!hasOwnBookmark && options.url) {
+ // Also mark as bookmarked if one of the earlier bookmarked items has the same URL.
+ hasBookmark =
+ options.bookmarks.filter(n => n < i).some(n => visits[n].uri.spec == spec && visits[n].test.hasBookmark);
+ }
do_print("Generating " + uri.spec + ", " + dbDate);
let visit = {
uri,
title: "visit " + i,
visitDate: dbDate,
test: {
// `visitDate`, as a Date
jsDate: jsDate,
@@ -48,18 +58,17 @@ add_task(function* test_removeVisitsByFi
// `true` if there is a bookmark for this URI, i.e. of the page
// should not be entirely removed.
hasBookmark: hasBookmark,
onFrecencyChanged: null,
onDeleteURI: null,
},
};
visits.push(visit);
- visitByURI.set(spec, visit);
- if (hasBookmark) {
+ if (hasOwnBookmark) {
do_print("Adding a bookmark to visit " + i);
yield PlacesUtils.bookmarks.insert({
url: uri,
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
title: "test bookmark"
});
do_print("Bookmark added");
}
@@ -78,24 +87,51 @@ add_task(function* test_removeVisitsByFi
filter.beginDate = new Date(ms);
beginIndex = options.begin;
}
if ("end" in options) {
let ms = Number(visits[options.end].test.jsDate) + 1000;
filter.endDate = new Date(ms);
endIndex = options.end;
}
- for (let i = beginIndex; i <= endIndex; ++i) {
- let test = visits[i].test;
- do_print("Marking visit " + i + " as expecting removal");
+ if ("limit" in options) {
+ endIndex = beginIndex + options.limit - 1; // -1 because the start index is inclusive.
+ filter.limit = options.limit;
+ }
+ let removedItems = visits.slice(beginIndex);
+ endIndex -= beginIndex;
+ if (options.url) {
+ let rawURL = "";
+ switch (options.url) {
+ case 1:
+ filter.url = new URL(removedItems[0].uri.spec);
+ rawURL = filter.url.href;
+ break;
+ case 2:
+ filter.url = removedItems[0].uri;
+ rawURL = filter.url.spec;
+ break;
+ case 3:
+ filter.url = removedItems[0].uri.spec;
+ rawURL = filter.url;
+ break;
+ }
+ endIndex = Math.min(endIndex, removedItems.findIndex((v, index) => v.uri.spec != rawURL) - 1);
+ }
+ removedItems.splice(endIndex + 1);
+ let remainingItems = visits.filter(v => !removedItems.includes(v));
+ for (let i = 0; i < removedItems.length; i++) {
+ let test = removedItems[i].test;
+ do_print("Marking visit " + (beginIndex + i) + " as expecting removal");
test.toRemove = true;
- if (test.hasBookmark) {
- test.onFrecencyChanged = PromiseUtils.defer();
- } else {
- test.onDeleteURI = PromiseUtils.defer();
+ if (test.hasBookmark ||
+ (options.url && remainingItems.some(v => v.uri.spec == removedItems[i].uri.spec))) {
+ frecencyChangePromises.set(removedItems[i].uri.spec, PromiseUtils.defer());
+ } else if (!options.url || i == 0) {
+ uriDeletePromises.set(removedItems[i].uri.spec, PromiseUtils.defer());
}
}
let observer = {
deferred: PromiseUtils.defer(),
onBeginUpdateBatch: function() {},
onEndUpdateBatch: function() {},
onVisit: function(uri) {
@@ -107,33 +143,31 @@ add_task(function* test_removeVisitsByFi
onClearHistory: function() {
this.deferred.reject("Unexpected call to onClearHistory");
},
onPageChanged: function(uri) {
this.deferred.reject(new Error("Unexpected call to onPageChanged " + uri.spec));
},
onFrecencyChanged: function(aURI) {
do_print("onFrecencyChanged " + aURI.spec);
- let visit = visitByURI.get(aURI.spec);
- Assert.ok(!!visit.test.onFrecencyChanged, "Observing onFrecencyChanged");
- visit.test.onFrecencyChanged.resolve();
+ let deferred = frecencyChangePromises.get(aURI.spec);
+ Assert.ok(!!deferred, "Observing onFrecencyChanged");
+ deferred.resolve();
},
onManyFrecenciesChanged: function() {
do_print("Many frecencies changed");
- for (let visit of visits) {
- if (visit.onFrecencyChanged) {
- visit.onFrecencyChanged.resolve();
- }
+ for (let [, deferred] of frecencyChangePromises) {
+ deferred.resolve();
}
},
onDeleteURI: function(aURI) {
do_print("onDeleteURI " + aURI.spec);
- let visit = visitByURI.get(aURI.spec);
- Assert.ok(!!visit.test.onDeleteURI, "Observing onDeleteURI");
- visit.test.onDeleteURI.resolve();
+ let deferred = uriDeletePromises.get(aURI.spec);
+ Assert.ok(!!deferred, "Observing onDeleteURI");
+ deferred.resolve();
},
onDeleteVisits: function(aURI) {
// Not sure we can test anything.
}
};
PlacesUtils.history.addObserver(observer, false);
let cbarg;
@@ -161,67 +195,71 @@ add_task(function* test_removeVisitsByFi
Assert.ok(result, "Removal succeeded");
// Make sure that we have eliminated exactly the entries we expected
// to eliminate.
for (let i = 0; i < visits.length; ++i) {
let visit = visits[i];
do_print("Controlling the results on visit " + i);
+ let remainingVisitsForURI = remainingItems.filter(v => visit.uri.spec == v.uri.spec).length;
Assert.equal(
- visits_in_database(visit.uri) == 0,
- visit.test.toRemove,
+ visits_in_database(visit.uri),
+ remainingVisitsForURI,
"Visit is still present iff expected");
if (options.useCallback) {
Assert.equal(
visit.test.toRemove,
visit.test.announcedByOnRow,
"Visit removal has been announced by onResult iff expected");
}
- if (visit.test.hasBookmark || !visit.test.toRemove) {
- Assert.notEqual(page_in_database(visit.uri), 0, "The page is should still appear in the db");
+ if (visit.test.hasBookmark || remainingVisitsForURI) {
+ Assert.notEqual(page_in_database(visit.uri), 0, "The page should still appear in the db");
} else {
Assert.equal(page_in_database(visit.uri), 0, "The page should have been removed from the db");
}
}
// Make sure that the observer has been called wherever applicable.
- for (let visit of visits) {
- do_print("Making sure that the observer has been called for " + visit.uri.spec);
- let test = visit.test;
- do_print("Checking onFrecencyChanged");
- if (test.onFrecencyChanged) {
- yield test.onFrecencyChanged.promise;
- }
- do_print("Checking onDeleteURI");
- if (test.onDeleteURI) {
- yield test.onDeleteURI.promise;
- }
- }
+ do_print("Checking URI delete promises.");
+ yield Promise.all(Array.from(uriDeletePromises.values()));
+ do_print("Checking frecency change promises.");
+ yield Promise.all(Array.from(frecencyChangePromises.values()));
PlacesUtils.history.removeObserver(observer);
});
let size = 20;
for (let range of [
{begin: 0},
{end: 19},
{begin: 0, end: 10},
{begin: 3, end: 4},
+ {begin: 5, end: 8, limit: 2},
+ {begin: 10, end: 18, limit: 5},
]) {
for (let bookmarks of [[], [5, 6]]) {
let options = {
sampleSize: size,
bookmarks: bookmarks,
};
if ("begin" in range) {
options.begin = range.begin;
}
if ("end" in range) {
options.end = range.end;
}
+ if ("limit" in range) {
+ options.limit = range.limit;
+ }
+ yield remover(options);
+ options.url = 1;
+ yield remover(options);
+ options.url = 2;
+ yield remover(options);
+ options.url = 3;
yield remover(options);
}
}
yield PlacesTestUtils.clearHistory();
});
// Test the various error cases
add_task(function* test_error_cases() {
@@ -248,16 +286,48 @@ add_task(function* test_error_cases() {
Assert.throws(
() => PlacesUtils.history.removeVisitsByFilter({beginDate: new Date()}, "obviously, not a callback"),
/TypeError: Invalid function/
);
Assert.throws(
() => PlacesUtils.history.removeVisitsByFilter({beginDate: new Date(1000), endDate: new Date(0)}),
/TypeError: `beginDate` should be at least as old/
);
+ Assert.throws(
+ () => PlacesUtils.history.removeVisitsByFilter({limit: {}}),
+ /Expected a non-zero positive integer as a limit/
+ );
+ Assert.throws(
+ () => PlacesUtils.history.removeVisitsByFilter({limit: -1}),
+ /Expected a non-zero positive integer as a limit/
+ );
+ Assert.throws(
+ () => PlacesUtils.history.removeVisitsByFilter({limit: 0.1}),
+ /Expected a non-zero positive integer as a limit/
+ );
+ Assert.throws(
+ () => PlacesUtils.history.removeVisitsByFilter({limit: Infinity}),
+ /Expected a non-zero positive integer as a limit/
+ );
+ Assert.throws(
+ () => PlacesUtils.history.removeVisitsByFilter({url: {}}),
+ /Expected a valid URL for `url`/
+ );
+ Assert.throws(
+ () => PlacesUtils.history.removeVisitsByFilter({url: 0}),
+ /Expected a valid URL for `url`/
+ );
+ Assert.throws(
+ () => PlacesUtils.history.removeVisitsByFilter({beginDate: new Date(1000), endDate: new Date(0)}),
+ /TypeError: `beginDate` should be at least as old/
+ );
+ Assert.throws(
+ () => PlacesUtils.history.removeVisitsByFilter({beginDate: new Date(1000), endDate: new Date(0)}),
+ /TypeError: `beginDate` should be at least as old/
+ );
});
add_task(function* test_orphans() {
let uri = NetUtil.newURI("http://moz.org/");
yield PlacesTestUtils.addVisits({ uri });
PlacesUtils.favicons.setAndFetchFaviconForPage(
uri, SMALLPNG_DATA_URI, true, PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE,