Bug 1820790 - Generalize the 180 day history import limit, and apply …

…it to IE and Edge (EdgeHTML) migrators. r=Gijs

Differential Revision: https://phabricator.services.mozilla.com/D172718

browser/app/profile/firefox.js

@@ -2160,9 +2160,8 @@ pref("browser.migrate.brave.enabled", true);
 pref("browser.migrate.canary.enabled", true);
 
 pref("browser.migrate.chrome.enabled", true);
-// See comments in bug 1340115 on how we got to these numbers.
+// See comments in bug 1340115 on how we got to this number.
 pref("browser.migrate.chrome.history.limit", 2000);
-pref("browser.migrate.chrome.history.maxAgeInDays", 180);
 
 pref("browser.migrate.chrome-beta.enabled", true);
 pref("browser.migrate.chrome-dev.enabled", true);
@@ -2181,6 +2180,9 @@ pref("browser.migrate.vivaldi.enabled", true);
 pref("browser.migrate.content-modal.enabled", false);
 pref("browser.migrate.content-modal.import-all.enabled", false);
 
+// The maximum age of history entries we'll import, in days.
+pref("browser.migrate.history.maxAgeInDays", 180);
+
 pref("extensions.pocket.api", "api.getpocket.com");
 pref("extensions.pocket.bffApi", "firefox-api-proxy.readitlater.com");
 pref("extensions.pocket.bffRecentSaves", true);

browser/components/migration/ChromeProfileMigrator.sys.mjs

@@ -536,21 +536,17 @@ async function GetHistoryResource(aProfileFolder) {
 
     migrate(aCallback) {
       (async function() {
-        const MAX_AGE_IN_DAYS = Services.prefs.getIntPref(
-          "browser.migrate.chrome.history.maxAgeInDays"
-        );
         const LIMIT = Services.prefs.getIntPref(
           "browser.migrate.chrome.history.limit"
         );
 
         let query =
           "SELECT url, title, last_visit_time, typed_count FROM urls WHERE hidden = 0";
-        if (MAX_AGE_IN_DAYS) {
-          let maxAge = lazy.ChromeMigrationUtils.dateToChromeTime(
-            Date.now() - MAX_AGE_IN_DAYS * 24 * 60 * 60 * 1000
-          );
-          query += " AND last_visit_time > " + maxAge;
-        }
+        let maxAge = lazy.ChromeMigrationUtils.dateToChromeTime(
+          Date.now() - MigrationUtils.HISTORY_MAX_AGE_IN_MILLISECONDS
+        );
+        query += " AND last_visit_time > " + maxAge;
+
         if (LIMIT) {
           query += " ORDER BY last_visit_time DESC LIMIT " + LIMIT;
         }

browser/components/migration/EdgeProfileMigrator.sys.mjs

@@ -129,7 +129,17 @@ EdgeTypedURLMigrator.prototype = {
   migrate(aCallback) {
     let typedURLs = this._typedURLs;
     let pageInfos = [];
+    let now = new Date();
+    let maxDate = new Date(
+      Date.now() - MigrationUtils.HISTORY_MAX_AGE_IN_MILLISECONDS
+    );
+
     for (let [urlString, time] of typedURLs) {
+      let visitDate = time ? lazy.PlacesUtils.toDate(time) : now;
+      if (time && visitDate < maxDate) {
+        continue;
+      }
+
       let url;
       try {
         url = new URL(urlString);
@@ -164,13 +174,15 @@ EdgeTypedURLMigrator.prototype = {
   },
 };
 
-function EdgeTypedURLDBMigrator() {}
+function EdgeTypedURLDBMigrator(dbOverride) {
+  this.dbOverride = dbOverride;
+}
 
 EdgeTypedURLDBMigrator.prototype = {
   type: MigrationUtils.resourceTypes.HISTORY,
 
   get db() {
-    return lazy.gEdgeDatabase;
+    return this.dbOverride || lazy.gEdgeDatabase;
   },
 
   get exists() {
@@ -210,26 +222,24 @@ EdgeTypedURLDBMigrator.prototype = {
     }
 
     let pageInfos = [];
-    // Sometimes the values are bogus (e.g. 0 becomes some date in 1600),
-    // and places will throw *everything* away, not just the bogus ones,
-    // so deal with that by having a cutoff date. Also, there's not much
-    // point importing really old entries. The cut-off date is related to
-    // Edge's launch date.
-    const kDateCutOff = new Date("2016", 0, 1);
+
+    const kDateCutOff = new Date(
+      Date.now() - MigrationUtils.HISTORY_MAX_AGE_IN_MILLISECONDS
+    );
     for (let typedUrlInfo of typedUrls) {
       try {
-        let url = new URL(typedUrlInfo.URL);
-        if (!["http:", "https:", "ftp:"].includes(url.protocol)) {
-          continue;
-        }
-
         let date = typedUrlInfo.AccessDateTimeUTC;
         if (!date) {
           date = kDateCutOff;
         } else if (date < kDateCutOff) {
           continue;
         }
 
+        let url = new URL(typedUrlInfo.URL);
+        if (!["http:", "https:", "ftp:"].includes(url.protocol)) {
+          continue;
+        }
+
         pageInfos.push({
           url,
           visits: [
@@ -508,6 +518,14 @@ export class EdgeProfileMigrator extends MigratorBase {
     return new EdgeReadingListMigrator(dbOverride);
   }
 
+  getHistoryDBMigratorForTesting(dbOverride) {
+    return new EdgeTypedURLDBMigrator(dbOverride);
+  }
+
+  getHistoryRegistryMigratorForTesting() {
+    return new EdgeTypedURLMigrator();
+  }
+
   getResources() {
     let resources = [
       new EdgeBookmarksMigrator(),

browser/components/migration/IEProfileMigrator.sys.mjs

@@ -43,6 +43,11 @@ History.prototype = {
     let typedURLs = MSMigrationUtils.getTypedURLs(
       "Software\\Microsoft\\Internet Explorer"
     );
+    let now = new Date();
+    let maxDate = new Date(
+      Date.now() - MigrationUtils.HISTORY_MAX_AGE_IN_MILLISECONDS
+    );
+
     for (let entry of Cc[
       "@mozilla.org/profile/migrator/iehistoryenumerator;1"
     ].createInstance(Ci.nsISimpleEnumerator)) {
@@ -64,18 +69,22 @@ History.prototype = {
       let transition = typedURLs.has(url.spec)
         ? lazy.PlacesUtils.history.TRANSITIONS.LINK
         : lazy.PlacesUtils.history.TRANSITIONS.TYPED;
-      // use the current date if we have no visits for this entry.
+
       let time = entry.get("time");
 
+      let visitDate = time ? lazy.PlacesUtils.toDate(time) : null;
+      if (visitDate && visitDate < maxDate) {
+        continue;
+      }
+
       pageInfos.push({
         url,
         title,
         visits: [
           {
             transition,
-            date: time
-              ? lazy.PlacesUtils.toDate(entry.get("time"))
-              : new Date(),
+            // use the current date if we have no visits for this entry.
+            date: visitDate ?? now,
           },
         ],
       });

browser/components/migration/MigrationUtils.sys.mjs

@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import { AppConstants } from "resource://gre/modules/AppConstants.sys.mjs";
+import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs";
 
 const lazy = {};
 
@@ -116,6 +117,15 @@ const MIGRATOR_MODULES = Object.freeze({
  * instance of this class is exported from this module as `MigrationUtils`.
  */
 class MigrationUtils {
+  constructor() {
+    XPCOMUtils.defineLazyPreferenceGetter(
+      this,
+      "HISTORY_MAX_AGE_IN_DAYS",
+      "browser.migrate.history.maxAgeInDays",
+      180
+    );
+  }
+
   resourceTypes = Object.freeze({
     ALL: 0x0000,
     /* 0x01 used to be used for settings, but was removed. */
@@ -1047,6 +1057,10 @@ class MigrationUtils {
   getSourceIdForTelemetry(sourceName) {
     return this.#SOURCE_NAME_TO_ID_MAPPING_ENUM[sourceName] || 0;
   }
+
+  get HISTORY_MAX_AGE_IN_MILLISECONDS() {
+    return this.HISTORY_MAX_AGE_IN_DAYS * 24 * 60 * 60 * 1000;
+  }
 }
 
 const MigrationUtilsSingleton = new MigrationUtils();

browser/components/migration/tests/unit/head_migration.js

@@ -165,3 +165,27 @@ function getRootPath() {
   }
   return Services.dirsvc.get(dirKey, Ci.nsIFile).path;
 }
+
+/**
+ * Returns a PRTime value for the current date minus daysAgo number
+ * of days.
+ *
+ * @param {number} daysAgo
+ *   How many days in the past from now the returned date should be.
+ * @returns {number}
+ */
+function PRTimeDaysAgo(daysAgo) {
+  return PlacesUtils.toPRTime(Date.now() - daysAgo * 24 * 60 * 60 * 1000);
+}
+
+/**
+ * Returns a Date value for the current date minus daysAgo number
+ * of days.
+ *
+ * @param {number} daysAgo
+ *   How many days in the past from now the returned date should be.
+ * @returns {Date}
+ */
+function dateDaysAgo(daysAgo) {
+  return new Date(Date.now() - daysAgo * 24 * 60 * 60 * 1000);
+}

browser/components/migration/tests/unit/test_Edge_db_migration.js

@@ -496,6 +496,53 @@ add_task(async function() {
       IsDeleted: false,
     },
   ];
+
+  // The following entries are expected to be skipped as being too old to
+  // migrate.
+  let expiredTypedURLsReferenceItems = [
+    {
+      URL: "https://expired1.invalid/",
+      AccessDateTimeUTC: dateDaysAgo(500),
+    },
+    {
+      URL: "https://expired2.invalid/",
+      AccessDateTimeUTC: dateDaysAgo(300),
+    },
+    {
+      URL: "https://expired3.invalid/",
+      AccessDateTimeUTC: dateDaysAgo(190),
+    },
+  ];
+
+  // The following entries should be new enough to migrate.
+  let unexpiredTypedURLsReferenceItems = [
+    {
+      URL: "https://unexpired1.invalid/",
+      AccessDateTimeUTC: dateDaysAgo(179),
+    },
+    {
+      URL: "https://unexpired2.invalid/",
+      AccessDateTimeUTC: dateDaysAgo(50),
+    },
+    {
+      URL: "https://unexpired3.invalid/",
+    },
+  ];
+
+  let typedURLsReferenceItems = [
+    ...expiredTypedURLsReferenceItems,
+    ...unexpiredTypedURLsReferenceItems,
+  ];
+
+  Assert.ok(
+    MigrationUtils.HISTORY_MAX_AGE_IN_DAYS < 300,
+    "This test expects the current pref to be less than the youngest expired visit."
+  );
+  Assert.ok(
+    MigrationUtils.HISTORY_MAX_AGE_IN_DAYS > 160,
+    "This test expects the current pref to be greater than the oldest unexpired visit."
+  );
+
   eseDBWritingHelpers.setupDB(
     db,
     new Map([
@@ -535,6 +582,19 @@ add_task(async function() {
           rows: readingListReferenceItems,
         },
       ],
+      [
+        "TypedURLs",
+        {
+          columns: [
+            { type: COLUMN_TYPES.JET_coltypLongText, name: "URL", cbMax: 4096 },
+            {
+              type: COLUMN_TYPES.JET_coltypLongLong,
+              name: "AccessDateTimeUTC",
+            },
+          ],
+          rows: typedURLsReferenceItems,
+        },
+      ],
     ])
   );
 
@@ -770,4 +830,24 @@ add_task(async function() {
     !readingListReferenceItems.length,
     "Should have seen all expected items."
   );
+
+  let historyDBMigrator = migrator.getHistoryDBMigratorForTesting(db);
+  await new Promise(resolve => {
+    historyDBMigrator.migrate(resolve);
+  });
+  Assert.ok(true, "History DB migration done!");
+  for (let expiredEntry of expiredTypedURLsReferenceItems) {
+    let entry = await PlacesUtils.history.fetch(expiredEntry.URL, {
+      includeVisits: true,
+    });
+    Assert.equal(entry, null, "Should not have found an entry.");
+  }
+
+  for (let unexpiredEntry of unexpiredTypedURLsReferenceItems) {
+    let entry = await PlacesUtils.history.fetch(unexpiredEntry.URL, {
+      includeVisits: true,
+    });
+    Assert.equal(entry.url, unexpiredEntry.URL, "Should have the correct URL");
+    Assert.ok(!!entry.visits.length, "Should have some visits");
+  }
 });