Bug 1420571 - Don't write unchanged page metadata to places.sqlite. r=Mardak
MozReview-Commit-ID: Gdv1qVSsfnO
--- a/toolkit/components/places/History.jsm
+++ b/toolkit/components/places/History.jsm
@@ -1383,32 +1383,36 @@ var insertMany = function(db, pageInfos,
}, true);
});
};
// Inner implementation of History.update.
var update = async function(db, pageInfo) {
let updateFragments = [];
let whereClauseFragment = "";
- let info = {};
+ let params = {};
// Prefer GUID over url if it's present
if (typeof pageInfo.guid === "string") {
- whereClauseFragment = "WHERE guid = :guid";
- info.guid = pageInfo.guid;
+ whereClauseFragment = "guid = :guid";
+ params.guid = pageInfo.guid;
} else {
- whereClauseFragment = "WHERE url_hash = hash(:url) AND url = :url";
- info.url = pageInfo.url.href;
+ whereClauseFragment = "url_hash = hash(:url) AND url = :url";
+ params.url = pageInfo.url.href;
}
if (pageInfo.description || pageInfo.description === null) {
- updateFragments.push("description = :description");
- info.description = pageInfo.description;
+ updateFragments.push("description");
+ params.description = pageInfo.description;
}
if (pageInfo.previewImageURL || pageInfo.previewImageURL === null) {
- updateFragments.push("preview_image_url = :previewImageURL");
- info.previewImageURL = pageInfo.previewImageURL ? pageInfo.previewImageURL.href : null;
+ updateFragments.push("preview_image_url");
+ params.preview_image_url = pageInfo.previewImageURL ? pageInfo.previewImageURL.href : null;
}
- let query = `UPDATE moz_places
- SET ${updateFragments.join(", ")}
- ${whereClauseFragment}`;
- await db.execute(query, info);
+ // Since this data may be written at every visit and is textual, avoid
+ // overwriting the existing record if it didn't change.
+ await db.execute(`
+ UPDATE moz_places
+ SET ${updateFragments.map(v => `${v} = :${v}`).join(", ")}
+ WHERE ${whereClauseFragment}
+ AND (${updateFragments.map(v => `IFNULL(${v}, "") <> IFNULL(:${v}, "")`).join(" OR ")})
+ `, params);
};
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -201,17 +201,17 @@ function serializeNode(aNode, aIsLivemar
}
return JSON.stringify(data);
}
// Imposed to limit database size.
const DB_URL_LENGTH_MAX = 65536;
const DB_TITLE_LENGTH_MAX = 4096;
-const DB_DESCRIPTION_LENGTH_MAX = 1024;
+const DB_DESCRIPTION_LENGTH_MAX = 256;
/**
* List of bookmark object validators, one per each known property.
* Validators must throw if the property value is invalid and return a fixed up
* version of the value, if needed.
*/
const BOOKMARK_VALIDATORS = Object.freeze({
guid: simpleValidateFunc(v => PlacesUtils.isValidGuid(v)),
--- a/toolkit/components/places/tests/history/test_update.js
+++ b/toolkit/components/places/tests/history/test_update.js
@@ -65,19 +65,19 @@ add_task(async function test_error_cases
/TypeError: pageInfo object must at least/,
"passing a pageInfo with neither description nor previewImageURL should throw a TypeError"
);
});
add_task(async function test_description_change_saved() {
await PlacesTestUtils.clearHistory();
- let TEST_URL = NetUtil.newURI("http://mozilla.org/test_description_change_saved");
+ let TEST_URL = "http://mozilla.org/test_description_change_saved";
await PlacesTestUtils.addVisits(TEST_URL);
- Assert.ok(page_in_database(TEST_URL));
+ Assert.ok(await PlacesTestUtils.isPageInDB(TEST_URL));
let description = "Test description";
await PlacesUtils.history.update({ url: TEST_URL, description });
let descriptionInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "description");
Assert.equal(description, descriptionInDB, "description should be updated via URL as expected");
description = "";
await PlacesUtils.history.update({ url: TEST_URL, description });
@@ -99,20 +99,20 @@ add_task(async function test_description
await PlacesUtils.history.update({ url: TEST_URL, description});
descriptionInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "description");
Assert.strictEqual(description, descriptionInDB, "a null description should set it to null in the database");
});
add_task(async function test_previewImageURL_change_saved() {
await PlacesTestUtils.clearHistory();
- let TEST_URL = NetUtil.newURI("http://mozilla.org/test_previewImageURL_change_saved");
+ let TEST_URL = "http://mozilla.org/test_previewImageURL_change_saved";
let IMAGE_URL = "http://mozilla.org/test_preview_image.png";
await PlacesTestUtils.addVisits(TEST_URL);
- Assert.ok(page_in_database(TEST_URL));
+ Assert.ok(await PlacesTestUtils.isPageInDB(TEST_URL));
let previewImageURL = IMAGE_URL;
await PlacesUtils.history.update({ url: TEST_URL, previewImageURL });
let previewImageURLInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "preview_image_url");
Assert.equal(previewImageURL, previewImageURLInDB, "previewImageURL should be updated via URL as expected");
previewImageURL = null;
await PlacesUtils.history.update({ url: TEST_URL, previewImageURL});
@@ -124,19 +124,19 @@ add_task(async function test_previewImag
await PlacesUtils.history.update({ url: TEST_URL, guid, previewImageURL });
previewImageURLInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "preview_image_url");
Assert.equal(previewImageURL, previewImageURLInDB, "previewImageURL should be updated via GUID as expected");
});
add_task(async function test_change_both_saved() {
await PlacesTestUtils.clearHistory();
- let TEST_URL = NetUtil.newURI("http://mozilla.org/test_change_both_saved");
+ let TEST_URL = "http://mozilla.org/test_change_both_saved";
await PlacesTestUtils.addVisits(TEST_URL);
- Assert.ok(page_in_database(TEST_URL));
+ Assert.ok(await PlacesTestUtils.isPageInDB(TEST_URL));
let description = "Test description";
let previewImageURL = "http://mozilla.org/test_preview_image.png";
await PlacesUtils.history.update({ url: TEST_URL, description, previewImageURL });
let descriptionInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "description");
let previewImageURLInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "preview_image_url");
Assert.equal(description, descriptionInDB, "description should be updated via URL as expected");