Bug 1720721: Decide whether to create a snapshot for new interactions when the user is idle. r=mak

Differential Revision: https://phabricator.services.mozilla.com/D119876
This commit is contained in:
Dave Townsend
2021-07-20 09:14:11 +00:00
parent a5bb634eb3
commit 731baf72ad
6 changed files with 637 additions and 30 deletions

View File

@@ -17,6 +17,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm",
Services: "resource://gre/modules/Services.jsm",
Snapshots: "resource:///modules/Snapshots.jsm",
setTimeout: "resource://gre/modules/Timer.jsm",
});
@@ -43,6 +44,13 @@ XPCOMUtils.defineLazyPreferenceGetter(
60
);
XPCOMUtils.defineLazyPreferenceGetter(
this,
"snapshotIdleTime",
"browser.places.interactions.snapshotIdleTime",
2
);
XPCOMUtils.defineLazyPreferenceGetter(
this,
"saveInterval",
@@ -534,6 +542,17 @@ class InteractionsStore {
* Used to unblock the queue of promises when the timer is cleared.
*/
#timerResolve = undefined;
/*
* A list of URLs that have had interactions updated since we last checked for
* new snapshots.
* @type {Set<string>}
*/
#potentialSnapshots = new Set();
/*
* Whether the user has been idle for more than the value of
* `browser.places.interactions.snapshotIdleTime`.
*/
#userIsIdle = false;
constructor() {
// Block async shutdown to ensure the last write goes through.
@@ -546,6 +565,19 @@ class InteractionsStore {
// Can be used to wait for the last pending write to have happened.
this.pendingPromise = Promise.resolve();
idleService.addIdleObserver(this, snapshotIdleTime);
}
/**
* Tells the snapshot service to check all of the potential snapshots.
*
* @returns {Promise}
*/
updateSnapshots() {
let urls = [...this.#potentialSnapshots];
this.#potentialSnapshots.clear();
return Snapshots.updateSnapshots(urls);
}
/**
@@ -557,6 +589,7 @@ class InteractionsStore {
clearTimeout(this.#timer);
this.#timerResolve();
await this.#updateDatabase();
await this.updateSnapshots();
}
}
@@ -608,6 +641,24 @@ class InteractionsStore {
}
}
/**
* Handles notifications from the observer service.
*
* @param {nsISupports} subject
* @param {string} topic
* @param {string} data
*/
observe(subject, topic, data) {
switch (topic) {
case "idle":
this.#userIsIdle = true;
this.updateSnapshots();
break;
case "active":
this.#userIsIdle = false;
}
}
async #updateDatabase() {
this.#timer = undefined;
@@ -619,7 +670,9 @@ class InteractionsStore {
let params = {};
let SQLInsertFragments = [];
let i = 0;
for (let interactionsForUrl of interactions.values()) {
for (let [url, interactionsForUrl] of interactions) {
this.#potentialSnapshots.add(url);
for (let interaction of interactionsForUrl.values()) {
params[`url${i}`] = interaction.url;
params[`created_at${i}`] = interaction.created_at;
@@ -670,5 +723,9 @@ class InteractionsStore {
}
);
this.progress.pendingUpdates = 0;
if (this.#userIsIdle) {
this.updateSnapshots();
}
}
}

View File

@@ -15,6 +15,64 @@ XPCOMUtils.defineLazyModuleGetters(this, {
Services: "resource://gre/modules/Services.jsm",
});
/**
* @typedef {object} SnapshotCriteria
* A set of tests to check if a set of interactions are a snapshot.
* @property {string} property
* The property to check.
* @property {string} aggregator
* An aggregator to apply ("sum", "avg", "max", "min").
* @property {number} cutoff
* The value to compare against.
* @property {?number} interactionCount
* The maximum number of interactions to consider.
* @property {?number} interactionRecency
* The maximum age of interactions to consider, in milliseconds.
*/
XPCOMUtils.defineLazyGetter(this, "logConsole", function() {
return console.createInstance({
prefix: "SnapshotsManager",
maxLogLevel: Services.prefs.getBoolPref(
"browser.places.interactions.log",
false
)
? "Debug"
: "Warn",
});
});
const DEFAULT_CRITERIA = [
{
property: "total_view_time",
aggregator: "max",
cutoff: 30000,
},
{
property: "total_view_time",
aggregator: "sum",
cutoff: 120000,
interactionCount: 5,
},
{
property: "key_presses",
aggregator: "sum",
cutoff: 250,
interactionCount: 10,
},
];
/**
* This is an array of criteria that would make a page a snapshot. Each element is a SnapshotCriteria.
* A page is a snapshot if any of the criteria apply.
*/
XPCOMUtils.defineLazyPreferenceGetter(
this,
"snapshotCriteria",
"browser.places.interactions.snapshotCriteria",
JSON.stringify(DEFAULT_CRITERIA)
);
/**
* @typedef {object} Snapshot
* A snapshot summarises a collection of interactions.
@@ -38,13 +96,15 @@ XPCOMUtils.defineLazyModuleGetters(this, {
* Handles storing and retrieving of Snapshots in the Places database.
*
* Notifications of updates are sent via the observer service:
* - places-snapshot-added, data: url
* - places-snapshots-added, data: JSON encoded array of urls
* Sent when a new snapshot is added
* - places-snapshot-deleted, data: url
* - places-snapshots-deleted, data: JSON encoded array of urls
* Sent when a snapshot is removed.
*/
const Snapshots = new (class Snapshots {
#snapshots = new Map();
#notify(topic, urls) {
Services.obs.notifyObservers(null, topic, JSON.stringify(urls));
}
/**
* Adds a new snapshot.
@@ -64,9 +124,14 @@ const Snapshots = new (class Snapshots {
if (!url) {
throw new Error("Missing url parameter to Snapshots.add()");
}
await PlacesUtils.withConnectionWrapper("Snapshots: add", async db => {
await db.executeCached(
`
let added = await PlacesUtils.withConnectionWrapper(
"Snapshots: add",
async db => {
let now = Date.now();
let rows = await db.executeCached(
`
INSERT INTO moz_places_metadata_snapshots
(place_id, first_interaction_at, last_interaction_at, document_type, created_at, user_persisted)
SELECT place_id, min(created_at), max(created_at),
@@ -75,12 +140,19 @@ const Snapshots = new (class Snapshots {
FROM moz_places_metadata
WHERE place_id = (SELECT id FROM moz_places WHERE url_hash = hash(:url) AND url = :url)
ON CONFLICT DO UPDATE SET user_persisted = :userPersisted, removed_at = NULL WHERE :userPersisted = 1
RETURNING created_at
`,
{ createdAt: Date.now(), url, userPersisted }
);
});
{ createdAt: now, url, userPersisted }
);
Services.obs.notifyObservers(null, "places-snapshot-added", url);
// If the url did not exist in moz_places then rows will be empty.
return !!rows.length;
}
);
if (added) {
this.#notify("places-snapshots-added", [url]);
}
}
/**
@@ -102,7 +174,7 @@ const Snapshots = new (class Snapshots {
);
});
Services.obs.notifyObservers(null, "places-snapshot-deleted", url);
this.#notify("places-snapshots-deleted", [url]);
}
/**
@@ -213,4 +285,140 @@ const Snapshots = new (class Snapshots {
}
return null;
}
/**
* Adds snapshots for any pages where the gathered metadata meets a set of
* criteria.
*
* @param {string[]} urls
* The list of pages to check, if undefined all pages are checked.
*/
async updateSnapshots(urls = undefined) {
if (urls !== undefined && !urls.length) {
return;
}
logConsole.debug(
`Updating ${urls ? urls.length : "all"} potential snapshots`
);
let model;
try {
model = JSON.parse(snapshotCriteria);
if (!model.length) {
logConsole.debug(`No model provided, falling back to default`);
model = DEFAULT_CRITERIA;
}
} catch (e) {
logConsole.error(
"Invalid snapshot criteria, falling back to default.",
e
);
model = DEFAULT_CRITERIA;
}
let insertedUrls = await PlacesUtils.withConnectionWrapper(
"Snapshots.jsm::updateSnapshots",
async db => {
let bindings = {};
let urlFilter = "";
if (urls !== undefined) {
let urlMatches = [];
urls.forEach((url, idx) => {
bindings[`url${idx}`] = url;
urlMatches.push(
`(url_hash = hash(:url${idx}) AND url = :url${idx})`
);
});
urlFilter = `WHERE ${urlMatches.join(" OR ")}`;
}
let modelQueries = [];
model.forEach((criteria, idx) => {
let wheres = [];
if (criteria.interactionCount) {
wheres.push(`row <= :count${idx}`);
bindings[`count${idx}`] = criteria.interactionCount;
}
if (criteria.interactionRecency) {
wheres.push(`created_at >= :recency${idx}`);
bindings[`recency${idx}`] = Date.now() - criteria.interactionCount;
}
let where = wheres.length ? `WHERE ${wheres.join(" AND ")}` : "";
modelQueries.push(
`
SELECT
place_id,
min(created_at) AS first_interaction_at,
max(created_at) AS last_interaction_at,
doc_type,
:createdAt
FROM metadata
${where}
GROUP BY place_id
HAVING ${criteria.aggregator}(${criteria.property}) >= :cutoff${idx}
`
);
bindings[`cutoff${idx}`] = criteria.cutoff;
});
let query = `
WITH metadata AS (
SELECT
moz_places_metadata.*,
row_number() OVER (PARTITION BY place_id ORDER BY created_at DESC) AS row,
first_value(document_type) OVER (PARTITION BY place_id ORDER BY created_at DESC) AS doc_type
FROM moz_places_metadata JOIN moz_places ON moz_places_metadata.place_id = moz_places.id
${urlFilter}
)
INSERT OR IGNORE INTO moz_places_metadata_snapshots
(place_id, first_interaction_at, last_interaction_at, document_type, created_at)
${modelQueries.join(" UNION ")}
RETURNING (SELECT url FROM moz_places WHERE id=place_id) AS url, created_at
`;
let now = Date.now();
let results = await db.execute(query, {
...bindings,
createdAt: now,
});
let newUrls = [];
for (let row of results) {
// If created_at differs from the passed value then this snapshot already existed.
if (row.getResultByName("created_at") == now) {
newUrls.push(row.getResultByName("url"));
}
}
return newUrls;
}
);
if (insertedUrls.length) {
logConsole.debug(`Inserted ${insertedUrls.length} snapshots.`);
this.#notify("places-snapshots-added", insertedUrls);
}
}
/**
* Completely clears the store. This exists for testing purposes.
*/
async reset() {
await PlacesUtils.withConnectionWrapper(
"Snapshots.jsm::reset",
async db => {
await db.executeCached(`DELETE FROM moz_places_metadata_snapshots`);
}
);
}
})();

View File

@@ -11,6 +11,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
PlacesTestUtils: "resource://testing-common/PlacesTestUtils.jsm",
PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
setTimeout: "resource://gre/modules/Timer.jsm",
Services: "resource://gre/modules/Services.jsm",
Snapshots: "resource:///modules/Snapshots.jsm",
TestUtils: "resource://testing-common/TestUtils.jsm",
});
@@ -18,6 +19,10 @@ XPCOMUtils.defineLazyModuleGetters(this, {
// Initialize profile.
var gProfD = do_get_profile(true);
// Observer notifications.
const TOPIC_ADDED = "places-snapshots-added";
const TOPIC_DELETED = "places-snapshots-deleted";
/**
* Adds a test interaction to the database.
*
@@ -43,6 +48,60 @@ async function addInteractions(interactions) {
await Interactions.store.flush();
}
/**
* Executes an async task and verifies that the given notification was sent with
* the given list of urls.
*
* @param {string} topic
* @param {string[]} expected
* @param {function} task
*/
async function assertUrlNotification(topic, expected, task) {
let seen = false;
let listener = (subject, _, data) => {
try {
let arr = JSON.parse(data);
if (arr.length != expected.length) {
return;
}
if (expected.every(url => arr.includes(url))) {
seen = true;
}
} catch (e) {
Assert.ok(false, e);
}
};
Services.obs.addObserver(listener, topic);
await task();
Services.obs.removeObserver(listener, topic);
Assert.ok(seen, `Should have seen ${topic} notification.`);
}
/**
* Executes an async task and verifies that the given observer notification was
* not sent.
*
* @param {string} topic
* @param {function} task
*/
async function assertTopicNotObserved(topic, task) {
let seen = false;
let listener = () => {
seen = true;
};
Services.obs.addObserver(listener, topic);
await task();
Services.obs.removeObserver(listener, topic);
Assert.ok(!seen, `Should not have seen ${topic} notification.`);
}
/**
* Asserts that a date looks reasonably valid, i.e. created no earlier than
* 24 hours prior to the current date.
@@ -125,3 +184,11 @@ async function assertSnapshots(expected, options) {
assertSnapshot(snapshots[i], expected[i]);
}
}
/**
* Clears all data from the snapshots and metadata tables.
*/
async function reset() {
await Snapshots.reset();
await Interactions.reset();
}

View File

@@ -8,8 +8,6 @@
const TEST_URL1 = "https://example.com/";
const TEST_URL2 = "https://example.com/12345";
const TEST_URL3 = "https://example.com/14235";
const TOPIC_ADDED = "places-snapshot-added";
const TOPIC_DELETED = "places-snapshot-deleted";
add_task(async function setup() {
let now = Date.now();
@@ -28,19 +26,13 @@ add_task(async function setup() {
});
add_task(async function test_add_simple_snapshot() {
let promise = TestUtils.topicObserved(
TOPIC_ADDED,
(subject, data) => !subject && data == TEST_URL1
await assertUrlNotification(TOPIC_ADDED, [TEST_URL1], () =>
Snapshots.add({ url: TEST_URL1 })
);
await Snapshots.add({ url: TEST_URL1 });
await promise;
promise = TestUtils.topicObserved(
TOPIC_ADDED,
(subject, data) => !subject && data == TEST_URL2
await assertUrlNotification(TOPIC_ADDED, [TEST_URL2], () =>
Snapshots.add({ url: TEST_URL2, userPersisted: true })
);
await Snapshots.add({ url: TEST_URL2, userPersisted: true });
await promise;
await assertSnapshots([
{ url: TEST_URL2, userPersisted: true },
@@ -64,7 +56,9 @@ add_task(async function test_add_duplicate_snapshot() {
let initialSnapshot = await Snapshots.get(TEST_URL3);
await Snapshots.add({ url: TEST_URL3 });
await assertTopicNotObserved(TOPIC_ADDED, () =>
Snapshots.add({ url: TEST_URL3 })
);
let newSnapshot = await Snapshots.get(TEST_URL3);
Assert.deepEqual(
@@ -109,12 +103,9 @@ add_task(async function test_get_snapshot_not_found() {
add_task(async function test_delete_snapshot() {
let removedAt = new Date();
let promise = TestUtils.topicObserved(
TOPIC_DELETED,
(subject, data) => !subject && data == TEST_URL1
await assertUrlNotification(TOPIC_DELETED, [TEST_URL1], () =>
Snapshots.delete(TEST_URL1)
);
await Snapshots.delete(TEST_URL1);
await promise;
let snapshot = await Snapshots.get(TEST_URL1);
Assert.ok(!snapshot, "Tombstone snapshots should not be returned by default");

View File

@@ -0,0 +1,283 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */
/**
* Tests for automatic snapshot creation.
*/
const TEST_URL1 = "https://example.com/";
const TEST_URL2 = "https://example.com/12345";
const TEST_URL3 = "https://example.com/14235";
add_task(async function maxViewTime() {
let now = Date.now();
// Make snapshots for any page with a typing time greater than 30 seconds in any one visit.
Services.prefs.setCharPref(
"browser.places.interactions.snapshotCriteria",
JSON.stringify([
{
property: "total_view_time",
aggregator: "max",
cutoff: 30000,
},
])
);
await assertUrlNotification(TOPIC_ADDED, [TEST_URL1, TEST_URL3], () =>
addInteractions([
{
url: TEST_URL1,
totalViewTime: 40000,
created_at: now - 1000,
},
{
url: TEST_URL2,
totalViewTime: 20000,
created_at: now - 2000,
},
{
url: TEST_URL2,
totalViewTime: 20000,
created_at: now - 3000,
},
{
url: TEST_URL3,
totalViewTime: 20000,
documentType: Interactions.DOCUMENT_TYPE.MEDIA,
created_at: now - 3000,
},
{
url: TEST_URL3,
totalViewTime: 20000,
documentType: Interactions.DOCUMENT_TYPE.GENERIC,
created_at: now - 4000,
},
{
url: TEST_URL3,
totalViewTime: 30000,
documentType: Interactions.DOCUMENT_TYPE.MEDIA,
created_at: now - 5000,
},
])
);
await assertSnapshots([
{
url: TEST_URL1,
userPersisted: false,
documentType: Interactions.DOCUMENT_TYPE.GENERIC,
},
{
url: TEST_URL3,
userPersisted: false,
documentType: Interactions.DOCUMENT_TYPE.MEDIA,
},
]);
await reset();
});
add_task(async function cumulative() {
// Snapshots should be created as new interactions occur
Services.prefs.setCharPref(
"browser.places.interactions.snapshotCriteria",
JSON.stringify([
{
property: "total_view_time",
aggregator: "sum",
cutoff: 30000,
},
])
);
await assertTopicNotObserved(TOPIC_ADDED, () =>
addInteractions([
{
url: TEST_URL1,
totalViewTime: 20000,
},
])
);
Assert.strictEqual(
await Snapshots.get(TEST_URL1, true),
null,
"Should not have created this snapshot yet."
);
await assertUrlNotification(TOPIC_ADDED, [TEST_URL1], () =>
addInteractions([
{
url: TEST_URL1,
totalViewTime: 20000,
},
])
);
await assertSnapshots([
{
url: TEST_URL1,
userPersisted: false,
},
]);
// Shouldn't see the notification when a url is already a snapshot.
await assertTopicNotObserved(TOPIC_ADDED, () =>
addInteractions([
{
url: TEST_URL1,
totalViewTime: 20000,
},
])
);
await reset();
});
add_task(async function tombstoned() {
let removedAt = new Date();
// Tombstoned snapshots should not be re-created.
Services.prefs.setCharPref(
"browser.places.interactions.snapshotCriteria",
JSON.stringify([
{
property: "total_view_time",
aggregator: "sum",
cutoff: 30000,
},
])
);
await addInteractions([
{
url: TEST_URL1,
totalViewTime: 1000,
},
]);
Assert.strictEqual(
await Snapshots.get(TEST_URL1, true),
null,
"Should not have created this snapshot yet."
);
await Snapshots.add({
url: TEST_URL1,
userPersisted: true,
});
assertSnapshot(await Snapshots.get(TEST_URL1, true), {
url: TEST_URL1,
userPersisted: true,
});
await assertTopicNotObserved(TOPIC_ADDED, () =>
addInteractions([
{
url: TEST_URL1,
totalViewTime: 40000,
},
])
);
// Shouldn't have overwritten the userPersisted flag.
assertSnapshot(await Snapshots.get(TEST_URL1, true), {
url: TEST_URL1,
userPersisted: true,
});
await assertUrlNotification(TOPIC_DELETED, [TEST_URL1], () =>
Snapshots.delete(TEST_URL1)
);
assertSnapshot(await Snapshots.get(TEST_URL1, true), {
url: TEST_URL1,
userPersisted: true,
removedAt,
});
await assertTopicNotObserved(TOPIC_ADDED, () =>
addInteractions([
{
url: TEST_URL1,
totalViewTime: 40000,
},
])
);
// Shouldn't have overwritten the tombstone flag.
assertSnapshot(await Snapshots.get(TEST_URL1, true), {
url: TEST_URL1,
userPersisted: true,
removedAt,
});
await reset();
});
add_task(async function multipleCriteria() {
let now = Date.now();
// Different criteria can apply to create snapshots.
Services.prefs.setCharPref(
"browser.places.interactions.snapshotCriteria",
JSON.stringify([
{
property: "total_view_time",
aggregator: "min",
cutoff: 10000,
},
{
property: "key_presses",
aggregator: "avg",
cutoff: 20,
},
])
);
await addInteractions([
{
url: TEST_URL1,
totalViewTime: 10000,
created_at: now - 1000,
},
{
url: TEST_URL1,
totalViewTime: 20000,
created_at: now - 1000,
},
{
url: TEST_URL2,
keypresses: 10,
created_at: now - 2000,
},
{
url: TEST_URL2,
keypresses: 50,
created_at: now - 2000,
},
{
url: TEST_URL2,
keypresses: 20,
created_at: now - 2000,
},
{
url: TEST_URL3,
totalViewTime: 30000,
keypresses: 10,
created_at: now - 3000,
},
{
url: TEST_URL3,
totalViewTime: 5000,
keypresses: 10,
created_at: now - 3000,
},
]);
await assertSnapshots([{ url: TEST_URL1 }, { url: TEST_URL2 }]);
await reset();
});

View File

@@ -4,4 +4,5 @@ firefox-appdir = browser
skip-if = toolkit == 'android'
[test_snapshots_basics.js]
[test_snapshots_create_criteria.js]
[test_snapshots_queries.js]