Backed out changeset aa40fbe24dd6 (bug 1707204) for landing with the wrong bug number

2021-04-27 19:34:23 +03:00 · 2021-04-27 19:34:23 +03:00 · 8d3f41a505
--- a/browser/components/migration/360seProfileMigrator.jsm
+++ b/browser/components/migration/360seProfileMigrator.jsm
@ -144,6 +144,7 @@ Bookmarks.prototype = {
        path: this._file.path,
      });

+      let histogramBookmarkRoots = 0;
      try {
        let rows = await connection.execute(
          `WITH RECURSIVE
@ -201,6 +202,8 @@ Bookmarks.prototype = {
      }

      if (toolbarBMs.length) {
+        histogramBookmarkRoots |=
+          MigrationUtils.SOURCE_BOOKMARK_ROOTS_BOOKMARKS_TOOLBAR;
        let parentGuid = PlacesUtils.bookmarks.toolbarGuid;
        if (
          !Services.prefs.getBoolPref("browser.toolbars.bookmarks.2h2020") &&
@ -217,6 +220,9 @@ Bookmarks.prototype = {
        await MigrationUtils.insertManyBookmarksWrapper(toolbarBMs, parentGuid);
        PlacesUIUtils.maybeToggleBookmarkToolbarVisibilityAfterMigration();
      }
+      Services.telemetry
+        .getKeyedHistogramById("FX_MIGRATION_BOOKMARKS_ROOTS")
+        .add("360se", histogramBookmarkRoots);
    })().then(
      () => aCallback(true),
      e => {
--- a/browser/components/migration/ChromeProfileMigrator.jsm
+++ b/browser/components/migration/ChromeProfileMigrator.jsm
@ -112,7 +112,11 @@ ChromeProfileMigrator.prototype.getResources = async function Chrome_getResource
        this.getBrowserKey()
      ).replace(/^source-name-/, "");
      let possibleResourcePromises = [
-        GetBookmarksResource(profileFolder, localePropertySuffix),
+        GetBookmarksResource(
+          profileFolder,
+          localePropertySuffix,
+          this.getBrowserKey()
+        ),
        GetHistoryResource(profileFolder),
        GetCookiesResource(profileFolder),
      ];
@ -214,7 +218,11 @@ Object.defineProperty(ChromeProfileMigrator.prototype, "sourceLocked", {
  },
 });

-async function GetBookmarksResource(aProfileFolder, aLocalePropertySuffix) {
+async function GetBookmarksResource(
+  aProfileFolder,
+  aLocalePropertySuffix,
+  aBrowserKey
+) {
  let bookmarksPath = OS.Path.join(aProfileFolder, "Bookmarks");
  if (!(await OS.File.exists(bookmarksPath))) {
    return null;
@ -234,10 +242,13 @@ async function GetBookmarksResource(aProfileFolder, aLocalePropertySuffix) {
          encoding: "UTF-8",
        });
        let roots = JSON.parse(bookmarkJSON).roots;
+        let histogramBookmarkRoots = 0;

        // Importing bookmark bar items
        if (roots.bookmark_bar.children && roots.bookmark_bar.children.length) {
          // Toolbar
+          histogramBookmarkRoots |=
+            MigrationUtils.SOURCE_BOOKMARK_ROOTS_BOOKMARKS_TOOLBAR;
          let parentGuid = PlacesUtils.bookmarks.toolbarGuid;
          let bookmarks = convertBookmarks(
            roots.bookmark_bar.children,
@ -265,6 +276,8 @@ async function GetBookmarksResource(aProfileFolder, aLocalePropertySuffix) {
        // Importing bookmark menu items
        if (roots.other.children && roots.other.children.length) {
          // Bookmark menu
+          histogramBookmarkRoots |=
+            MigrationUtils.SOURCE_BOOKMARK_ROOTS_BOOKMARKS_MENU;
          let parentGuid = PlacesUtils.bookmarks.menuGuid;
          let bookmarks = convertBookmarks(roots.other.children, errorGatherer);
          if (
@ -286,6 +299,9 @@ async function GetBookmarksResource(aProfileFolder, aLocalePropertySuffix) {
        if (gotErrors) {
          throw new Error("The migration included errors.");
        }
+        Services.telemetry
+          .getKeyedHistogramById("FX_MIGRATION_BOOKMARKS_ROOTS")
+          .add(aBrowserKey, histogramBookmarkRoots);
      })().then(
        () => aCallback(true),
        () => aCallback(false)
--- a/browser/components/migration/EdgeProfileMigrator.jsm
+++ b/browser/components/migration/EdgeProfileMigrator.jsm
@ -395,7 +395,10 @@ EdgeBookmarksMigrator.prototype = {
      throw new Error("Edge seems to be running - its database is locked.");
    }
    let { toplevelBMs, toolbarBMs } = this._fetchBookmarksFromDB();
+    let histogramBookmarkRoots = 0;
    if (toplevelBMs.length) {
+      histogramBookmarkRoots |=
+        MigrationUtils.SOURCE_BOOKMARK_ROOTS_BOOKMARKS_MENU;
      let parentGuid = PlacesUtils.bookmarks.menuGuid;
      if (
        !Services.prefs.getBoolPref("browser.toolbars.bookmarks.2h2020") &&
@ -411,6 +414,8 @@ EdgeBookmarksMigrator.prototype = {
      await MigrationUtils.insertManyBookmarksWrapper(toplevelBMs, parentGuid);
    }
    if (toolbarBMs.length) {
+      histogramBookmarkRoots |=
+        MigrationUtils.SOURCE_BOOKMARK_ROOTS_BOOKMARKS_TOOLBAR;
      let parentGuid = PlacesUtils.bookmarks.toolbarGuid;
      if (
        !Services.prefs.getBoolPref("browser.toolbars.bookmarks.2h2020") &&
@ -426,6 +431,9 @@ EdgeBookmarksMigrator.prototype = {
      await MigrationUtils.insertManyBookmarksWrapper(toolbarBMs, parentGuid);
      PlacesUIUtils.maybeToggleBookmarkToolbarVisibilityAfterMigration();
    }
+    Services.telemetry
+      .getKeyedHistogramById("FX_MIGRATION_BOOKMARKS_ROOTS")
+      .add("edge", histogramBookmarkRoots);
  },

  _fetchBookmarksFromDB() {
--- a/browser/components/migration/MSMigrationUtils.jsm
+++ b/browser/components/migration/MSMigrationUtils.jsm
@ -411,11 +411,20 @@ Bookmarks.prototype = {
    return this.__toolbarFolderName;
  },

+  _histogramBookmarkRoots: 0,
  migrate: function B_migrate(aCallback) {
    return (async () => {
      // Import to the bookmarks menu.
      let folderGuid = PlacesUtils.bookmarks.menuGuid;
      await this._migrateFolder(this._favoritesFolder, folderGuid);
+      Services.telemetry
+        .getKeyedHistogramById("FX_MIGRATION_BOOKMARKS_ROOTS")
+        .add(
+          this._migrationType == MSMigrationUtils.MIGRATION_TYPE_IE
+            ? "ie"
+            : "edge",
+          this._histogramBookmarkRoots
+        );
    })().then(
      () => aCallback(true),
      e => {
@ -431,6 +440,14 @@ Bookmarks.prototype = {
      return;
    }

+    if (aDestFolderGuid == PlacesUtils.bookmarks.menuGuid) {
+      this._histogramBookmarkRoots |=
+        MigrationUtils.SOURCE_BOOKMARK_ROOTS_BOOKMARKS_MENU;
+    } else if (aDestFolderGuid == PlacesUtils.bookmarks.toolbarGuid) {
+      this._histogramBookmarkRoots |=
+        MigrationUtils.SOURCE_BOOKMARK_ROOTS_BOOKMARKS_TOOLBAR;
+    }
+
    if (
      !Services.prefs.getBoolPref("browser.toolbars.bookmarks.2h2020") &&
      !MigrationUtils.isStartupMigration &&
--- a/browser/components/migration/SafariProfileMigrator.jsm
+++ b/browser/components/migration/SafariProfileMigrator.jsm
@ -70,6 +70,9 @@ Bookmarks.prototype = {
      ) {
        PlacesUIUtils.maybeToggleBookmarkToolbarVisibilityAfterMigration();
      }
+      Services.telemetry
+        .getKeyedHistogramById("FX_MIGRATION_BOOKMARKS_ROOTS")
+        .add("safari", this._histogramBookmarkRoots);
    })().then(
      () => aCallback(true),
      e => {
--- a/browser/components/migration/tests/unit/test_IE_bookmarks.js
+++ b/browser/components/migration/tests/unit/test_IE_bookmarks.js
@ -13,10 +13,17 @@ add_task(async function() {
  // on the actual favorites stored on the local machine's IE favorites database.
  // As such, we can't assert that bookmarks were migrated to both the bookmarks
  // menu and the bookmarks toolbar.
+  let bookmarkRoots = 0;
  let itemCount = 0;
  let listener = events => {
    for (let event of events) {
      if (event.itemType == PlacesUtils.bookmarks.TYPE_BOOKMARK) {
+        if (event.parentGuid == PlacesUtils.bookmarks.toolbarGuid) {
+          bookmarkRoots |=
+            MigrationUtils.SOURCE_BOOKMARK_ROOTS_BOOKMARKS_TOOLBAR;
+        } else if (event.parentGuid == PlacesUtils.bookmarks.menuGuid) {
+          bookmarkRoots |= MigrationUtils.SOURCE_BOOKMARK_ROOTS_BOOKMARKS_MENU;
+        }
        info("bookmark added: " + event.parentGuid);
        itemCount++;
      }
@ -46,6 +53,17 @@ add_task(async function() {
    itemCount,
    "Ensure telemetry matches actual number of imported items."
  );
+  await TestUtils.waitForCondition(() => {
+    let snapshot = Services.telemetry.getSnapshotForKeyedHistograms(
+      "main",
+      false
+    ).parent.FX_MIGRATION_BOOKMARKS_ROOTS;
+    if (!snapshot || !snapshot.ie) {
+      return false;
+    }
+    info(`Expected ${bookmarkRoots}, got ${snapshot.ie.sum}`);
+    return snapshot.ie.sum == bookmarkRoots;
+  }, "Wait until telemetry is updated");

  // Check the bookmarks have been imported to all the expected parents.
  Assert.ok(observerNotified, "The observer should be notified upon migration");
--- a/browser/components/migration/tests/unit/test_Safari_bookmarks.js
+++ b/browser/components/migration/tests/unit/test_Safari_bookmarks.js
@ -16,12 +16,28 @@ add_task(async function() {
  // we're importing to has less than 3 bookmarks in the destination
  // so a "From Safari" folder isn't created.
  let expectedParents = [PlacesUtils.toolbarFolderId];
+  let bookmarkRoots = 0;
+  let bookmarkRootMap = {
+    [PlacesUtils.bookmarks.toolbarGuid]:
+      MigrationUtils.SOURCE_BOOKMARK_ROOTS_BOOKMARKS_TOOLBAR,
+    [PlacesUtils.bookmarks.menuGuid]:
+      MigrationUtils.SOURCE_BOOKMARK_ROOTS_BOOKMARKS_MENU,
+    [PlacesUtils.bookmarks.unfiledGuid]:
+      MigrationUtils.SOURCE_BOOKMARK_ROOTS_UNFILED,
+  };
  let itemCount = 0;

  let gotFolder = false;
  let listener = events => {
    for (let event of events) {
      itemCount++;
+      if (
+        event.itemType == PlacesUtils.bookmarks.TYPE_BOOKMARK &&
+        bookmarkRootMap[event.parentGuid]
+      ) {
+        bookmarkRoots |= bookmarkRootMap[event.parentGuid];
+      }
+
      if (
        event.itemType == PlacesUtils.bookmarks.TYPE_FOLDER &&
        event.title == "Stuff"
@ -65,5 +81,25 @@ add_task(async function() {
    itemCount,
    "Telemetry reporting correct."
  );
+  let telemetryRootsMatchesExpectations = await TestUtils.waitForCondition(
+    () => {
+      let snapshot = Services.telemetry.getSnapshotForKeyedHistograms(
+        "main",
+        false
+      ).parent.FX_MIGRATION_BOOKMARKS_ROOTS;
+      if (!snapshot || !snapshot.safari) {
+        return false;
+      }
+      let sum = arr => Object.values(arr).reduce((a, b) => a + b, 0);
+      let sumOfValues = sum(snapshot.safari.values);
+      info(`Expected ${bookmarkRoots}, got ${sumOfValues}`);
+      return sumOfValues == bookmarkRoots;
+    },
+    "Wait until telemetry is updated"
+  );
+  ok(
+    telemetryRootsMatchesExpectations,
+    "The value in the roots histogram should match expectations"
+  );
  Assert.ok(observerNotified, "The observer should be notified upon migration");
 });
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@ -8007,6 +8007,18 @@
    "releaseChannelCollection": "opt-out",
    "description": "Accumulated timer delay (variance between when the timer was expected to fire and when it actually fired) in milliseconds as an indicator for decreased main-thread responsiveness while importing logins / passwords from a CSV/TSV file. The import is happening on a background thread and should ideally not affect the UI noticeably. The time with the blocking Keychain dialog on macOS can skew this data."
  },
+  "FX_MIGRATION_BOOKMARKS_ROOTS": {
+    "record_in_processes": ["main"],
+    "products": ["firefox"],
+    "bug_numbers": [1656277],
+    "alert_emails": ["jaws@mozilla.com", "rtestard@mozilla.com"],
+    "expires_in_version": "91",
+    "kind": "enumerated",
+    "n_values": 32,
+    "releaseChannelCollection": "opt-out",
+    "keyed": true,
+    "description": "Where bookmarks were stored in the other browser (1=bookmarks toolbar, 2=bookmarks menu, 4=reading list, 8=unfiled), keyed by the name of the browser. Note that migrating the reading list from classic Edge is supported but its root is not reported here."
+  },
  "FX_MIGRATION_BOOKMARKS_QUANTITY": {
    "record_in_processes": ["main"],
    "products": ["firefox"],