Bug 1829139 - Optimizations to old visits expiration. r=Standard8 (3e6600cc) · Commits · clairehurst / Tor Browser

toolkit/components/places/PlacesExpiration.sys.mjs

+29 −20

Original line number	Diff line number	Diff line
		@@ -120,21 +120,28 @@ const ACTION = {
		const EXPIRATION_QUERIES = {
		// Some visits can be expired more often than others, cause they are less
		// useful to the user and can pollute awesomebar results:
		// 1. visits to urls over 255 chars
		// 1. urls over 255 chars having only one visit
		// 2. downloads
		// 3. non-typed hidden single-visit urls
		// We never expire redirect targets, because they are currently necessary to
		// recognize redirect sources (see Bug 468710 for better options).
		// Note: due to the REPLACE option, this should be executed before
		// QUERY_FIND_VISITS_TO_EXPIRE, that has a more complete result.
		QUERY_FIND_EXOTIC_VISITS_TO_EXPIRE: {
		sql: `INSERT INTO expiration_notify (v_id, url, guid, visit_date, reason)
		SELECT v.id, h.url, h.guid, v.visit_date, "exotic"
		WITH visits AS (
		SELECT v.id, url, guid, visit_type, visit_date, visit_count, hidden, typed
		FROM moz_historyvisits v
		JOIN moz_places h ON h.id = v.place_id
		WHERE visit_date < strftime('%s','now','localtime','start of day','-60 days','utc') * 1000000
		WHERE visit_date < strftime('%s','now','localtime','start of day','-90 days','utc') * 1000000
		AND visit_type NOT IN (5,6)
		AND ( LENGTH(h.url) > 255 OR v.visit_type = 7 )
		ORDER BY v.visit_date ASC
		)
		SELECT id, url, guid, visit_date, "exotic"
		FROM visits
		WHERE (hidden = 1 AND typed = 0 AND visit_count <= 1) OR visit_type = 7
		UNION ALL
		SELECT id, url, guid, visit_date, "exotic"
		FROM visits
		WHERE visit_count = 1 AND LENGTH(url) > 255
		ORDER BY visit_date ASC
		LIMIT :limit_visits`,
		actions:
		ACTION.TIMED_OVERLIMIT \|
		@@ -555,7 +562,7 @@ nsPlacesExpiration.prototype = {
		}, 300000);
		},

		_onQueryResult(row) {
		_handleQueryResultAndAddNotification(row, notifications) {
		// We don't want to notify after shutdown.
		if (this._shuttingDown) {
		return;
		@@ -597,15 +604,15 @@ nsPlacesExpiration.prototype = {

		// Dispatch expiration notifications to history.
		const isRemovedFromStore = !!wholeEntry;
		PlacesObservers.notifyListeners([
		notifications.push(
		new PlacesVisitRemoved({
		url: uri.spec,
		pageGuid: guid,
		reason: PlacesVisitRemoved.REASON_EXPIRED,
		isRemovedFromStore,
		isPartialVisistsRemoval: !isRemovedFromStore && visitDate > 0,
		}),
		]);
		})
		);
		},

		_shuttingDown: false,
		@@ -748,6 +755,7 @@ nsPlacesExpiration.prototype = {
		await this._dbInitializedPromise;

		try {
		let notifications = [];
		await lazy.PlacesUtils.withConnectionWrapper(
		"PlacesExpiration.jsm: expire",
		async db => {
		@@ -760,16 +768,17 @@ nsPlacesExpiration.prototype = {
		aLimit,
		aAction
		);
		await db.executeCached(
		query.sql,
		params,
		this._onQueryResult.bind(this)
		);
		await db.executeCached(query.sql, params, row => {
		this._handleQueryResultAndAddNotification(row, notifications);
		});
		}
		}
		});
		}
		);
		if (notifications.length) {
		PlacesObservers.notifyListeners(notifications);
		}
		} catch (ex) {
		console.error(ex);
		return;

toolkit/components/places/tests/expiration/test_debug_expiration.js

+88 −30

Original line number	Diff line number	Diff line
		@@ -8,17 +8,19 @@
		* only expire orphan entries, unless -1 is passed as limit.
		*/

		var gNow = getExpirablePRTime(60);
		const EXPIRE_DAYS = 90;
		var gExpirableTime = getExpirablePRTime(EXPIRE_DAYS);
		var gNonExpirableTime = getExpirablePRTime(EXPIRE_DAYS - 2);

		add_task(async function test_expire_orphans() {
		// Add visits to 2 pages and force a orphan expiration. Visits should survive.
		await PlacesTestUtils.addVisits({
		uri: uri("http://page1.mozilla.org/"),
		visitDate: gNow++,
		visitDate: gExpirableTime++,
		});
		await PlacesTestUtils.addVisits({
		uri: uri("http://page2.mozilla.org/"),
		visitDate: gNow++,
		visitDate: gExpirableTime++,
		});
		// Create a orphan place.
		let bm = await PlacesUtils.bookmarks.insert({
		@@ -44,11 +46,11 @@ add_task(async function test_expire_orphans_optionalarg() {
		// Add visits to 2 pages and force a orphan expiration. Visits should survive.
		await PlacesTestUtils.addVisits({
		uri: uri("http://page1.mozilla.org/"),
		visitDate: gNow++,
		visitDate: gExpirableTime++,
		});
		await PlacesTestUtils.addVisits({
		uri: uri("http://page2.mozilla.org/"),
		visitDate: gNow++,
		visitDate: gExpirableTime++,
		});
		// Create a orphan place.
		let bm = await PlacesUtils.bookmarks.insert({
		@@ -75,12 +77,12 @@ add_task(async function test_expire_limited() {
		{
		// Should be expired cause it's the oldest visit
		uri: "http://old.mozilla.org/",
		visitDate: gNow++,
		visitDate: gExpirableTime++,
		},
		{
		// Should not be expired cause we limit 1
		uri: "http://new.mozilla.org/",
		visitDate: gNow++,
		visitDate: gExpirableTime++,
		},
		]);

		@@ -96,30 +98,38 @@ add_task(async function test_expire_limited() {
		await PlacesUtils.history.clear();
		});

		add_task(async function test_expire_limited_longurl() {
		add_task(async function test_expire_visitcount_longurl() {
		let longurl = "http://long.mozilla.org/" + "a".repeat(232);
		let longurl2 = "http://long2.mozilla.org/" + "a".repeat(232);
		await PlacesTestUtils.addVisits([
		{
		// Should be expired cause it's the oldest visit
		uri: "http://old.mozilla.org/",
		visitDate: gNow++,
		visitDate: gExpirableTime++,
		},
		{
		// Should be expired cause it's a long url older than 60 days.
		// Should not be expired cause it has 2 visits.
		uri: longurl,
		visitDate: gNow++,
		visitDate: gExpirableTime++,
		},
		{
		// Should not be expired cause younger than 60 days.
		uri: longurl,
		visitDate: getExpirablePRTime(58),
		visitDate: gNonExpirableTime,
		},
		{
		// Should be expired cause it has 1 old visit.
		uri: longurl2,
		visitDate: gExpirableTime++,
		},
		]);

		await promiseForceExpirationStep(1);

		// Check that some visits survived.
		Assert.equal(visits_in_database(longurl), 1);
		Assert.equal(visits_in_database(longurl), 2);
		// Check visit has been removed.
		Assert.equal(visits_in_database(longurl2), 0);

		// Other visits should have been expired.
		Assert.ok(!page_in_database("http://old.mozilla.org/"));

		@@ -132,18 +142,18 @@ add_task(async function test_expire_limited_exoticurl() {
		{
		// Should be expired cause it's the oldest visit
		uri: "http://old.mozilla.org/",
		visitDate: gNow++,
		visitDate: gExpirableTime++,
		},
		{
		// Should be expired cause it's a long url older than 60 days.
		uri: "http://download.mozilla.org",
		visitDate: gNow++,
		transition: 7,
		// Should not be expired cause younger than EXPIRE_DAYS.
		uri: "http://nonexpirable-download.mozilla.org",
		visitDate: gNonExpirableTime,
		transition: PlacesUtils.history.TRANSITIONS.DOWNLOAD,
		},
		{
		// Should not be expired cause younger than 60 days.
		uri: "http://nonexpirable-download.mozilla.org",
		visitDate: getExpirablePRTime(58),
		// Should be expired cause it's a long url older than EXPIRE_DAYS.
		uri: "http://download.mozilla.org",
		visitDate: gExpirableTime++,
		transition: 7,
		},
		]);
		@@ -166,26 +176,76 @@ add_task(async function test_expire_limited_exoticurl() {
		await PlacesUtils.history.clear();
		});

		add_task(async function test_expire_exotic_hidden() {
		let visits = [
		{
		// Should be expired cause it's the oldest visit
		uri: "http://old.mozilla.org/",
		visitDate: gExpirableTime++,
		expectedCount: 0,
		},
		{
		// Expirable typed hidden url.
		uri: "https://typedhidden.mozilla.org/",
		visitDate: gExpirableTime++,
		transition: PlacesUtils.history.TRANSITIONS.FRAMED_LINK,
		expectedCount: 2,
		},
		{
		// Mark as typed.
		uri: "https://typedhidden.mozilla.org/",
		visitDate: gExpirableTime++,
		transition: PlacesUtils.history.TRANSITIONS.TYPED,
		expectedCount: 2,
		},
		{
		// Expirable non-typed hidden url.
		uri: "https://hidden.mozilla.org/",
		visitDate: gExpirableTime++,
		transition: PlacesUtils.history.TRANSITIONS.FRAMED_LINK,
		expectedCount: 0,
		},
		];
		await PlacesTestUtils.addVisits(visits);
		for (let visit of visits) {
		Assert.greater(visits_in_database(visit.uri), 0);
		}

		await promiseForceExpirationStep(1);

		for (let visit of visits) {
		Assert.equal(
		visits_in_database(visit.uri),
		visit.expectedCount,
		`${visit.uri} should${
		visit.expectedCount == 0 ? " " : " not "
		}have been expired`
		);
		}
		// Clean up.
		await PlacesUtils.history.clear();
		});

		add_task(async function test_expire_unlimited() {
		let longurl = "http://long.mozilla.org/" + "a".repeat(232);
		await PlacesTestUtils.addVisits([
		{
		uri: "http://old.mozilla.org/",
		visitDate: gNow++,
		visitDate: gExpirableTime++,
		},
		{
		uri: "http://new.mozilla.org/",
		visitDate: gNow++,
		visitDate: gExpirableTime++,
		},
		// Add expirable visits.
		{
		uri: "http://download.mozilla.org/",
		visitDate: gNow++,
		visitDate: gExpirableTime++,
		transition: PlacesUtils.history.TRANSITION_DOWNLOAD,
		},
		{
		uri: longurl,
		visitDate: gNow++,
		visitDate: gExpirableTime++,
		},

		// Add non-expirable visits
		@@ -401,11 +461,9 @@ add_task(async function test_expire_icons() {
		await PlacesUtils.history.clear();
		});

		function run_test() {
		add_setup(async function() {
		// Set interval to a large value so we don't expire on it.
		setInterval(3600); // 1h
		// Set maxPages to a low value, so it's easy to go over it.
		setMaxPages(1);

		run_next_test();
		}
		});

toolkit/components/places/tests/expiration/test_notifications_pageRemoved_allVisits.js

+29 −26

Original line number	Diff line number	Diff line
		@@ -73,7 +73,7 @@ add_task(async () => {

		for (let testIndex = 1; testIndex <= tests.length; testIndex++) {
		let currentTest = tests[testIndex - 1];
		print("\nTEST " + testIndex + ": " + currentTest.desc);
		info("TEST " + testIndex + ": " + currentTest.desc);
		currentTest.receivedNotifications = 0;

		// Setup visits.
		@@ -107,6 +107,7 @@ add_task(async () => {
		}

		// Observe history.
		let notificationsHandled = new Promise(resolve => {
		const listener = async events => {
		for (const event of events) {
		Assert.equal(event.type, "page-removed");
		@@ -129,13 +130,15 @@ add_task(async () => {
		);
		}
		}
		PlacesObservers.removeListener(["page-removed"], listener);
		resolve();
		};
		PlacesObservers.addListener(["page-removed"], listener);
		});

		// Expire now.
		await promiseForceExpirationStep(currentTest.limitExpiration);

		PlacesObservers.removeListener(["page-removed"], listener);
		await notificationsHandled;

		Assert.equal(
		currentTest.receivedNotifications,