From f5d28a8ed6d21c9c6cc36445dc455adcb957ffd8 Mon Sep 17 00:00:00 2001
From: Marco Bonardo <mbonardo@mozilla.com>
Date: Mon, 27 Nov 2017 18:15:21 +0100
Subject: [PATCH] Bug 1420571 - Don't write unchanged page metadata to
 places.sqlite. r=Mardak

MozReview-Commit-ID: Gdv1qVSsfnO

--HG--
extra : rebase_source : 3aa365b44dbd148f1f835e12a08100add8e5e4a2
---
 toolkit/components/places/History.jsm         | 30 +++++++++++--------
 toolkit/components/places/PlacesUtils.jsm     |  2 +-
 .../places/tests/history/test_update.js       | 12 ++++----
 3 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/toolkit/components/places/History.jsm b/toolkit/components/places/History.jsm
index 0fff2ede2fa5..e91ce939e551 100644
--- a/toolkit/components/places/History.jsm
+++ b/toolkit/components/places/History.jsm
@@ -1388,27 +1388,31 @@ var insertMany = function(db, pageInfos, onResult, onError) {
 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);
 };
diff --git a/toolkit/components/places/PlacesUtils.jsm b/toolkit/components/places/PlacesUtils.jsm
index dee08a11d4f6..03ef7b498a06 100644
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -206,7 +206,7 @@ function serializeNode(aNode, aIsLivemark) {
 // 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.
diff --git a/toolkit/components/places/tests/history/test_update.js b/toolkit/components/places/tests/history/test_update.js
index ce5cabeaab81..2bf10faa0356 100644
--- a/toolkit/components/places/tests/history/test_update.js
+++ b/toolkit/components/places/tests/history/test_update.js
@@ -70,9 +70,9 @@ add_task(async function test_error_cases() {
 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 });
@@ -104,10 +104,10 @@ add_task(async function test_description_change_saved() {
 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 });
@@ -129,9 +129,9 @@ add_task(async function test_previewImageURL_change_saved() {
 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";