Commit fd0fb964 authored by reynaa's avatar reynaa
Browse files

Bug 1801057 - removed code as needed in Bookmarks.sys.mjs and...

Bug 1801057 - removed code as needed in Bookmarks.sys.mjs and PlacesDBUtils.sys.mjs and ran xpcshell tests. r=mak

All 250 tests were passed. Made changes to histograms.

Differential Revision: https://phabricator.services.mozilla.com/D169057
parent e5655328
Loading
Loading
Loading
Loading
+0 −71
Original line number Diff line number Diff line
@@ -2024,19 +2024,6 @@ async function updateBookmark(
        syncChangeDelta
      );
    }
    // Remove the Sync orphan annotation from reparented items. We don't
    // notify annotation observers about this because this is a temporary,
    // internal anno that's only used by Sync.
    await db.executeCached(
      `DELETE FROM moz_items_annos
       WHERE anno_attribute_id = (SELECT id FROM moz_anno_attributes
                                  WHERE name = :orphanAnno) AND
             item_id = :id`,
      {
        orphanAnno: lazy.PlacesSyncUtils.bookmarks.SYNC_PARENT_ANNO,
        id: item._id,
      }
    );
  }

  // If the parent changed, update related non-enumerable properties.
@@ -2652,11 +2639,6 @@ function removeBookmarks(items, options) {
          items,
          db.variableLimit
        )) {
          // We don't go through the annotations service for this cause otherwise
          // we'd get a pointless onItemChanged notification and it would also
          // set lastModified to an unexpected value.
          await removeAnnotationsForItems(db, chunk);

          // Remove the bookmarks.
          await db.executeCached(
            `DELETE FROM moz_bookmarks
@@ -2970,56 +2952,6 @@ function validateBookmarkObject(name, input, behavior) {
  );
}

/**
 * Removes any orphan annotation entries.
 *
 * @param db
 *        the Sqlite.sys.mjs connection handle.
 */
var removeOrphanAnnotations = async function(db) {
  await db.executeCached(
    `DELETE FROM moz_items_annos
     WHERE id IN (SELECT a.id from moz_items_annos a
                  LEFT JOIN moz_bookmarks b ON a.item_id = b.id
                  WHERE b.id ISNULL)
    `
  );
  await db.executeCached(
    `DELETE FROM moz_anno_attributes
     WHERE id IN (SELECT n.id from moz_anno_attributes n
                  LEFT JOIN moz_annos a1 ON a1.anno_attribute_id = n.id
                  LEFT JOIN moz_items_annos a2 ON a2.anno_attribute_id = n.id
                  WHERE a1.id ISNULL AND a2.id ISNULL)
    `
  );
};

/**
 * Removes annotations for a given item.
 *
 * @param db
 *        the Sqlite.sys.mjs connection handle.
 * @param items
 *        The items for which to remove annotations.
 */
var removeAnnotationsForItems = async function(db, items) {
  // Remove the annotations.
  let itemIds = items.map(item => item._id);
  await db.executeCached(
    `DELETE FROM moz_items_annos
     WHERE item_id IN (${lazy.PlacesUtils.sqlBindPlaceholders(itemIds)})`,
    itemIds
  );
  await db.executeCached(
    `DELETE FROM moz_anno_attributes
     WHERE id IN (SELECT n.id from moz_anno_attributes n
                  LEFT JOIN moz_annos a1 ON a1.anno_attribute_id = n.id
                  LEFT JOIN moz_items_annos a2 ON a2.anno_attribute_id = n.id
                  WHERE a1.id ISNULL AND a2.id ISNULL)
    `
  );
};

/**
 * Updates lastModified for all the ancestors of a given folder GUID.
 *
@@ -3134,9 +3066,6 @@ var removeFoldersContents = async function(db, folderGuids, options) {
  // folders.
  await addSyncChangesForRemovedTagFolders(db, itemsRemoved, syncChangeDelta);

  // Cleanup orphans.
  await removeOrphanAnnotations(db);

  // TODO (Bug 1087576): this may leave orphan tags behind.

  // Send onItemRemoved notifications to listeners.
+0 −38
Original line number Diff line number Diff line
@@ -376,25 +376,12 @@ export var PlacesDBUtils = {
        )`,
      },

      // A.2 remove obsolete annotations from moz_items_annos.
      {
        query: `DELETE FROM moz_items_annos
        WHERE type = 4 OR anno_attribute_id IN (
          SELECT id FROM moz_anno_attributes
          WHERE name = 'sync/children'
             OR name = 'placesInternal/GUID'
             OR name BETWEEN 'weave/' AND 'weave0'
        )`,
      },

      // A.3 remove unused attributes.
      {
        query: `DELETE FROM moz_anno_attributes WHERE id IN (
          SELECT id FROM moz_anno_attributes n
          WHERE NOT EXISTS
              (SELECT id FROM moz_annos WHERE anno_attribute_id = n.id LIMIT 1)
            AND NOT EXISTS
              (SELECT id FROM moz_items_annos WHERE anno_attribute_id = n.id LIMIT 1)
        )`,
      },

@@ -709,26 +696,6 @@ export var PlacesDBUtils = {
        )`,
      },

      // MOZ_ITEMS_ANNOS
      // H.1 remove item annos with an invalid attribute
      {
        query: `DELETE FROM moz_items_annos WHERE id IN (
          SELECT id FROM moz_items_annos t
          WHERE NOT EXISTS
            (SELECT id FROM moz_anno_attributes
              WHERE id = t.anno_attribute_id LIMIT 1)
        )`,
      },

      // H.2 remove orphan item annos
      {
        query: `DELETE FROM moz_items_annos WHERE id IN (
          SELECT id FROM moz_items_annos t
          WHERE NOT EXISTS
            (SELECT id FROM moz_bookmarks WHERE id = t.item_id LIMIT 1)
        )`,
      },

      // MOZ_KEYWORDS
      // I.1 remove unused keywords
      {
@@ -1197,11 +1164,6 @@ export var PlacesDBUtils = {
        },
      },

      {
        histogram: "PLACES_ANNOS_BOOKMARKS_COUNT",
        query: "SELECT count(*) FROM moz_items_annos",
      },

      {
        histogram: "PLACES_ANNOS_PAGES_COUNT",
        query: "SELECT count(*) FROM moz_annos",
+2 −207
Original line number Diff line number Diff line
@@ -25,7 +25,6 @@ async function cleanDatabase() {
      await db.executeCached("DELETE FROM moz_historyvisits");
      await db.executeCached("DELETE FROM moz_anno_attributes");
      await db.executeCached("DELETE FROM moz_annos");
      await db.executeCached("DELETE FROM moz_items_annos");
      await db.executeCached("DELETE FROM moz_inputhistory");
      await db.executeCached("DELETE FROM moz_keywords");
      await db.executeCached("DELETE FROM moz_icons");
@@ -178,74 +177,11 @@ tests.push({
  },
});

tests.push({
  name: "A.2",
  desc: "Remove obsolete annotations from moz_items_annos",

  _obsoleteSyncAttribute: "sync/children",
  _obsoleteGuidAttribute: "placesInternal/GUID",
  _obsoleteWeaveAttribute: "weave/test",
  _placeId: null,
  _bookmarkId: null,

  async setup() {
    // Add a place to ensure place_id = 1 is valid.
    this._placeId = await addPlace();
    // Add a bookmark.
    this._bookmarkId = await addBookmark(
      this._placeId,
      PlacesUtils.bookmarks.TYPE_BOOKMARK
    );
    await PlacesUtils.withConnectionWrapper("setup", async db => {
      await db.executeTransaction(async () => {
        // Add an obsolete attribute.
        await db.executeCached(
          `INSERT INTO moz_anno_attributes (name)
           VALUES (:anno1), (:anno2), (:anno3)`,
          {
            anno1: this._obsoleteSyncAttribute,
            anno2: this._obsoleteGuidAttribute,
            anno3: this._obsoleteWeaveAttribute,
          }
        );
        await db.executeCached(
          `INSERT INTO moz_items_annos (item_id, anno_attribute_id)
           SELECT :item_id, id
           FROM moz_anno_attributes
           WHERE name IN (:anno1, :anno2, :anno3)`,
          {
            item_id: this._bookmarkId,
            anno1: this._obsoleteSyncAttribute,
            anno2: this._obsoleteGuidAttribute,
            anno3: this._obsoleteWeaveAttribute,
          }
        );
      });
    });
  },

  async check() {
    // Check that the obsolete annotations have been removed.
    let db = await PlacesUtils.promiseDBConnection();
    let rows = await db.executeCached(
      `SELECT id FROM moz_anno_attributes
       WHERE name IN (:anno1, :anno2, :anno3)`,
      {
        anno1: this._obsoleteSyncAttribute,
        anno2: this._obsoleteGuidAttribute,
        anno3: this._obsoleteWeaveAttribute,
      }
    );
    Assert.equal(rows.length, 0);
  },
});

tests.push({
  name: "A.3",
  desc: "Remove unused attributes",

  _usedPageAttribute: "usedPage",
  _usedItemAttribute: "usedItem",
  _unusedAttribute: "unused",
  _placeId: null,
  _bookmarkId: null,
@@ -263,11 +199,10 @@ tests.push({
        // Add a used attribute and an unused one.
        await db.executeCached(
          `INSERT INTO moz_anno_attributes (name)
           VALUES (:anno1), (:anno2), (:anno3)`,
           VALUES (:anno1), (:anno2)`,
          {
            anno1: this._usedPageAttribute,
            anno2: this._usedItemAttribute,
            anno3: this._unusedAttribute,
            anno2: this._unusedAttribute,
          }
        );
        await db.executeCached(
@@ -278,14 +213,6 @@ tests.push({
            anno: this._usedPageAttribute,
          }
        );
        await db.executeCached(
          `INSERT INTO moz_items_annos (item_id, anno_attribute_id)
           VALUES(:item_id, (SELECT id FROM moz_anno_attributes WHERE name = :anno))`,
          {
            item_id: this._bookmarkId,
            anno: this._usedItemAttribute,
          }
        );
      });
    });
  },
@@ -300,13 +227,6 @@ tests.push({
      }
    );
    Assert.equal(rows.length, 1);
    rows = await db.executeCached(
      "SELECT id FROM moz_anno_attributes WHERE name = :anno",
      {
        anno: this._usedItemAttribute,
      }
    );
    Assert.equal(rows.length, 1);
    // Check that unused attribute has been removed
    rows = await db.executeCached(
      "SELECT id FROM moz_anno_attributes WHERE name = :anno",
@@ -1544,131 +1464,6 @@ tests.push({

// ------------------------------------------------------------------------------

tests.push({
  name: "H.1",
  desc: "Remove item annos with an invalid attribute",

  _usedItemAttribute: "usedItem",
  _bookmarkId: null,
  _placeId: null,

  async setup() {
    // Add a place to ensure place_id = 1 is valid
    this._placeId = await addPlace();
    // Insert a bookmark
    this._bookmarkId = await addBookmark(
      this._placeId,
      PlacesUtils.bookmarks.TYPE_BOOKMARK
    );
    await PlacesUtils.withConnectionWrapper("setup", async db => {
      await db.executeTransaction(async () => {
        // Add a used attribute.
        await db.executeCached(
          "INSERT INTO moz_anno_attributes (name) VALUES (:anno)",
          { anno: this._usedItemAttribute }
        );
        await db.executeCached(
          `INSERT INTO moz_items_annos (item_id, anno_attribute_id)
           VALUES(:item_id, (SELECT id FROM moz_anno_attributes WHERE name = :anno))`,
          { item_id: this._bookmarkId, anno: this._usedItemAttribute }
        );
        // Add an annotation with a nonexistent attribute
        await db.executeCached(
          "INSERT INTO moz_items_annos (item_id, anno_attribute_id) VALUES(:item_id, 1337)",
          { item_id: this._bookmarkId }
        );
      });
    });
  },

  async check() {
    let db = await PlacesUtils.promiseDBConnection();
    // Check that used attribute is still there
    let rows = await db.executeCached(
      "SELECT id FROM moz_anno_attributes WHERE name = :anno",
      { anno: this._usedItemAttribute }
    );
    Assert.equal(rows.length, 1);
    // check that annotation with valid attribute is still there
    rows = await db.executeCached(
      `SELECT id FROM moz_items_annos WHERE anno_attribute_id = (SELECT id FROM moz_anno_attributes WHERE name = :anno)`,
      { anno: this._usedItemAttribute }
    );
    Assert.equal(rows.length, 1);
    // Check that annotation with bogus attribute has been removed
    rows = await db.executeCached(
      "SELECT id FROM moz_items_annos WHERE anno_attribute_id = 1337"
    );
    Assert.equal(rows.length, 0);
  },
});

// ------------------------------------------------------------------------------

tests.push({
  name: "H.2",
  desc: "Remove orphan item annotations",

  _usedItemAttribute: "usedItem",
  _bookmarkId: null,
  _invalidBookmarkId: 8888,
  _placeId: null,

  async setup() {
    // Add a place to ensure place_id = 1 is valid
    this._placeId = await addPlace();
    // Insert a bookmark
    this._bookmarkId = await addBookmark(
      this._placeId,
      PlacesUtils.bookmarks.TYPE_BOOKMARK
    );
    await PlacesUtils.withConnectionWrapper("setup", async db => {
      await db.executeTransaction(async () => {
        // Add a used attribute.
        await db.executeCached(
          "INSERT INTO moz_anno_attributes (name) VALUES (:anno)",
          { anno: this._usedItemAttribute }
        );
        await db.executeCached(
          `INSERT INTO moz_items_annos (item_id, anno_attribute_id)
           VALUES (:item_id, (SELECT id FROM moz_anno_attributes WHERE name = :anno))`,
          { item_id: this._bookmarkId, anno: this._usedItemAttribute }
        );
        // Add an annotation to a nonexistent item
        await db.executeCached(
          `INSERT INTO moz_items_annos (item_id, anno_attribute_id)
           VALUES (:item_id, (SELECT id FROM moz_anno_attributes WHERE name = :anno))`,
          { item_id: this._invalidBookmarkId, anno: this._usedItemAttribute }
        );
      });
    });
  },

  async check() {
    let db = await PlacesUtils.promiseDBConnection();
    // Check that used attribute is still there
    let rows = await db.executeCached(
      "SELECT id FROM moz_anno_attributes WHERE name = :anno",
      { anno: this._usedItemAttribute }
    );
    Assert.equal(rows.length, 1);
    // check that annotation with valid attribute is still there
    rows = await db.executeCached(
      `SELECT id FROM moz_items_annos
       WHERE anno_attribute_id = (SELECT id FROM moz_anno_attributes WHERE name = :anno)`,
      { anno: this._usedItemAttribute }
    );
    Assert.equal(rows.length, 1);
    // Check that an annotation to a nonexistent page has been removed
    rows = await db.executeCached(
      "SELECT id FROM moz_items_annos WHERE item_id = 8888"
    );
    Assert.equal(rows.length, 0);
  },
});

// ------------------------------------------------------------------------------

tests.push({
  name: "I.1",
  desc: "Remove unused keywords",
+0 −1
Original line number Diff line number Diff line
@@ -27,7 +27,6 @@ const histograms = {
  PLACES_DATABASE_SIZE_PER_PAGE_B: val => Assert.ok(val > 0),
  PLACES_EXPIRATION_STEPS_TO_CLEAN2: val => Assert.ok(val > 1),
  PLACES_IDLE_MAINTENANCE_TIME_MS: val => Assert.ok(val > 0),
  PLACES_ANNOS_BOOKMARKS_COUNT: val => Assert.equal(val, 0),
  PLACES_ANNOS_PAGES_COUNT: val => Assert.equal(val, 1),
  PLACES_MAINTENANCE_DAYSFROMLAST: val => Assert.ok(val >= 0),
};
+0 −10
Original line number Diff line number Diff line
@@ -7231,16 +7231,6 @@
    "bug_numbers": [1811209],
    "description": "PLACES: Time to recalculate frecency for a chunk of pages (ms)"
  },
  "PLACES_ANNOS_BOOKMARKS_COUNT": {
    "record_in_processes": ["main", "content"],
    "products": ["firefox", "fennec", "thunderbird"],
    "expires_in_version": "never",
    "kind": "exponential",
    "low": 50,
    "high": 5000,
    "n_buckets": 10,
    "description": "PLACES: Number of bookmarks annotations"
  },
  "PLACES_ANNOS_PAGES_COUNT": {
    "record_in_processes": ["main", "content"],
    "products": ["firefox", "fennec", "thunderbird"],
Loading