author Mark Banner <standard8@mozilla.com>

Fri, 02 Mar 2018 11:09:12 +0000

changeset 767467 ccb4e30c799479d3a0efabd63db1ab9085e124ad

parent 767466 4e29f78ae9b3345e86bdc4ec6251b6927aae0039

child 767569 058db4a662dd95aea859292cf08e4f05554f6c0c

push id 102609

push user bmo:standard8@mozilla.com

push date Wed, 14 Mar 2018 17:33:35 +0000

reviewers mak

bugs 1310295

milestone 61.0a1

toolkit/components/places/Database.cpp file | annotate | diff | comparison | revisions

toolkit/components/places/Database.h file | annotate | diff | comparison | revisions

toolkit/components/places/tests/head_common.js file | annotate | diff | comparison | revisions

toolkit/components/places/tests/migration/places_v43.sqlite file | annotate | diff | comparison | revisions

toolkit/components/places/tests/migration/test_current_from_v43.js file | annotate | diff | comparison | revisions

toolkit/components/places/tests/migration/xpcshell.ini file | annotate | diff | comparison | revisions
--- a/toolkit/components/places/Database.cpp
+++ b/toolkit/components/places/Database.cpp
@@ -1223,16 +1223,23 @@ Database::InitSchema(bool* aDatabaseMigr
 
       if (currentSchemaVersion < 43) {
         rv = MigrateV43Up();
         NS_ENSURE_SUCCESS(rv, rv);
       }
 
       // Firefox 60 uses schema version 43.
 
+      if (currentSchemaVersion < 44) {
+        rv = MigrateV44Up();
+        NS_ENSURE_SUCCESS(rv, rv);
+      }
+
+      // Firefox 61 uses schema version 44.
+
       // Schema Upgrades must add migration code here.
       // >>> IMPORTANT! <<<
       // NEVER MIX UP SYNC AND ASYNC EXECUTION IN MIGRATORS, YOU MAY LOCK THE
       // CONNECTION AND CAUSE FURTHER STEPS TO FAIL.
       // In case, set a bool and do the async work in the ScopeExit guard just
       // before the migration steps.
     }
   }
@@ -1938,16 +1945,140 @@ Database::MigrateV43Up() {
     "WHERE post_data ISNULL "
   ));
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
 nsresult
+Database::MigrateV44Up() {
+  // We need to remove any non-builtin roots and their descendants.
+
+  // Install a temp trigger to clean up linked tables when the main
+  // bookmarks are deleted.
+  nsresult rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "CREATE TEMP TRIGGER moz_migrate_bookmarks_trigger "
+    "AFTER DELETE ON moz_bookmarks FOR EACH ROW "
+    "BEGIN "
+      // Insert tombstones.
+      "INSERT OR IGNORE INTO moz_bookmarks_deleted (guid, dateRemoved) "
+        "VALUES (OLD.guid, strftime('%s', 'now', 'localtime', 'utc') * 1000); "
+      // Remove old annotations for the bookmarks.
+      "DELETE FROM moz_items_annos "
+        "WHERE item_id = OLD.id; "
+      // Decrease the foreign_count in moz_places.
+      "UPDATE moz_places "
+        "SET foreign_count = foreign_count - 1 "
+        "WHERE id = OLD.fk; "
+    "END "
+  ));
+  if (NS_FAILED(rv)) return rv;
+
+  // This trigger listens for moz_places deletes, and updates moz_annos and
+  // moz_keywords accordingly.
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "CREATE TEMP TRIGGER moz_migrate_annos_trigger "
+    "AFTER UPDATE ON moz_places FOR EACH ROW "
+    // Only remove from moz_places if we don't have any remaining keywords pointing to
+    // this place, and it hasn't been visited. Note: orphan keywords are tidied up below.
+    "WHEN NEW.visit_count = 0 AND "
+      " NEW.foreign_count = (SELECT COUNT(*) FROM moz_keywords WHERE place_id = NEW.id) "
+    "BEGIN "
+      // No more references to the place, so we can delete the place itself.
+      "DELETE FROM moz_places "
+        "WHERE id = NEW.id; "
+      // Delete annotations relating to the place.
+      "DELETE FROM moz_annos "
+        "WHERE place_id = NEW.id; "
+      // Delete keywords relating to the place.
+      "DELETE FROM moz_keywords "
+        "WHERE place_id = NEW.id; "
+    "END "
+  ));
+  if (NS_FAILED(rv)) return rv;
+
+  // Listens to moz_keyword deletions, to ensure moz_places gets the
+  // foreign_count updated corrrectly.
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "CREATE TEMP TRIGGER moz_migrate_keyword_trigger "
+    "AFTER DELETE ON moz_keywords FOR EACH ROW "
+    "BEGIN "
+      // If we remove a keyword, then reduce the foreign_count.
+      "UPDATE moz_places "
+        "SET foreign_count = foreign_count - 1 "
+          "WHERE id = OLD.place_id; "
+    "END "
+  ));
+  if (NS_FAILED(rv)) return rv;
+
+  // First of all, find the non-builtin roots.
+  nsCOMPtr<mozIStorageStatement> deleteStmt;
+  rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
+    "WITH RECURSIVE "
+    "itemsToRemove(id, guid) AS ( "
+      "SELECT b.id, b.guid FROM moz_bookmarks b "
+      "JOIN moz_bookmarks p ON b.parent = p.id "
+      "WHERE p.guid = 'root________' AND "
+        "b.guid NOT IN ('menu________', 'toolbar_____', 'tags________', 'unfiled_____', 'mobile______') "
+      "UNION ALL "
+      "SELECT b.id, b.guid FROM moz_bookmarks b "
+      "JOIN itemsToRemove d ON d.id = b.parent "
+    ") "
+    "DELETE FROM moz_bookmarks "
+      "WHERE id IN (SELECT id FROM itemsToRemove) "
+  ), getter_AddRefs(deleteStmt));
+  if (NS_FAILED(rv)) return rv;
+
+  rv = deleteStmt->Execute();
+  if (NS_FAILED(rv)) return rv;
+
+  // Before we remove the triggers, check for keywords attached to places which
+  // no longer have a bookmark to them. We do this before removing the triggers,
+  // so that we can make use of the keyword trigger to update the counts in
+  // moz_places.
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "DELETE FROM moz_keywords WHERE place_id IN ( "
+      "SELECT h.id FROM moz_keywords k "
+      "JOIN moz_places h ON h.id = k.place_id "
+      "GROUP BY place_id HAVING h.foreign_count = count(*) "
+    ")"
+  ));
+  if (NS_FAILED(rv)) return rv;
+
+  // Now remove the temp triggers.
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "DROP TRIGGER moz_migrate_bookmarks_trigger "
+  ));
+  if (NS_FAILED(rv)) return rv;
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "DROP TRIGGER moz_migrate_annos_trigger "
+  ));
+  if (NS_FAILED(rv)) return rv;
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "DROP TRIGGER moz_migrate_keyword_trigger "
+  ));
+  if (NS_FAILED(rv)) return rv;
+
+  // Cleanup any orphan annotation attributes.
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "DELETE FROM moz_anno_attributes WHERE id IN ( "
+      "SELECT id FROM moz_anno_attributes n "
+      "EXCEPT "
+      "SELECT DISTINCT anno_attribute_id FROM moz_annos "
+      "EXCEPT "
+      "SELECT DISTINCT anno_attribute_id FROM moz_items_annos "
+    ")"
+  ));
+  if (NS_FAILED(rv)) return rv;
+
+  return rv;
+}
+
+nsresult
 Database::GetItemsWithAnno(const nsACString& aAnnoName, int32_t aItemType,
                            nsTArray<int64_t>& aItemIds)
 {
   nsCOMPtr<mozIStorageStatement> stmt;
   nsresult rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
     "SELECT b.id FROM moz_items_annos a "
     "JOIN moz_anno_attributes n ON n.id = a.anno_attribute_id "
     "JOIN moz_bookmarks b ON b.id = a.item_id "
--- a/toolkit/components/places/Database.h
+++ b/toolkit/components/places/Database.h
@@ -14,17 +14,17 @@
 #include "mozilla/storage/StatementCache.h"
 #include "mozilla/Attributes.h"
 #include "nsIEventTarget.h"
 #include "Shutdown.h"
 #include "nsCategoryCache.h"
 
 // This is the schema version. Update it at any schema change and add a
 // corresponding migrateVxx method below.
-#define DATABASE_SCHEMA_VERSION 43
+#define DATABASE_SCHEMA_VERSION 44
 
 // Fired after Places inited.
 #define TOPIC_PLACES_INIT_COMPLETE "places-init-complete"
 // This topic is received when the profile is about to be lost.  Places does
 // initial shutdown work and notifies TOPIC_PLACES_SHUTDOWN to all listeners.
 // Any shutdown work that requires the Places APIs should happen here.
 #define TOPIC_PROFILE_CHANGE_TEARDOWN "profile-change-teardown"
 // Fired when Places is shutting down.  Any code should stop accessing Places
@@ -299,16 +299,17 @@ protected:
   nsresult MigrateV36Up();
   nsresult MigrateV37Up();
   nsresult MigrateV38Up();
   nsresult MigrateV39Up();
   nsresult MigrateV40Up();
   nsresult MigrateV41Up();
   nsresult MigrateV42Up();
   nsresult MigrateV43Up();
+  nsresult MigrateV44Up();
 
   nsresult UpdateBookmarkRootTitles();
 
   friend class ConnectionShutdownBlocker;
 
   int64_t CreateMobileRoot();
   nsresult GetItemsWithAnno(const nsACString& aAnnoName, int32_t aItemType,
                             nsTArray<int64_t>& aItemIds);
--- a/toolkit/components/places/tests/head_common.js
+++ b/toolkit/components/places/tests/head_common.js
@@ -1,17 +1,14 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*-
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-// It is expected that the test files importing this file define Cu etc.
-/* global Cu, Ci, Cc, Cr */
-
-const CURRENT_SCHEMA_VERSION = 43;
+const CURRENT_SCHEMA_VERSION = 44;
 const FIRST_UPGRADABLE_SCHEMA_VERSION = 30;
 
 const NS_APP_USER_PROFILE_50_DIR = "ProfD";
 
 // Shortcuts to transitions type.
 const TRANSITION_LINK = Ci.nsINavHistoryService.TRANSITION_LINK;
 const TRANSITION_TYPED = Ci.nsINavHistoryService.TRANSITION_TYPED;
 const TRANSITION_BOOKMARK = Ci.nsINavHistoryService.TRANSITION_BOOKMARK;
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/migration/test_current_from_v43.js
@@ -0,0 +1,210 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const EXPECTED_REMAINING_ROOTS = [
+  ...PlacesUtils.bookmarks.userContentRoots,
+  PlacesUtils.bookmarks.tagsGuid,
+];
+
+const EXPECTED_REMOVED_BOOKMARK_GUIDS = [
+  // These first ones are the old left-pane folder queries
+  "SNLmwJH6GtW9", // Root Query
+  "r0dY_2_y4mlx", // History
+  "xGGhZK3b6GnW", // Downloads
+  "EJG6I1nKkQFQ", // Tags
+  "gSyHo5oNSUJV", // All Bookmarks
+  // These are simulated add-on injections that we expect to be removed.
+  "exaddon_____",
+  "exaddon1____",
+  "exaddon2____",
+  "exaddon3____",
+];
+
+const EXPECTED_REMOVED_ANNOTATIONS = [
+  "PlacesOrganizer/OrganizerFolder",
+  "PlacesOrganizer/OrganizerQuery",
+];
+
+const EXPECTED_REMOVED_PLACES_ENTRIES = ["exaddonh____", "exaddonh3___"];
+const EXPECTED_KEPT_PLACES_ENTRY = "exaddonh2___";
+const EXPECTED_REMOVED_KEYWORDS = ["exaddon", "exaddon2"];
+
+async function assertItemIn(db, table, field, expectedItems) {
+  let rows = await db.execute(`SELECT ${field} from ${table}`);
+
+  Assert.ok(rows.length >= expectedItems.length,
+    "Should be at least the number of annotations we expect to be removed.");
+
+  let fieldValues = rows.map(row => row.getResultByName(field));
+
+  for (let item of expectedItems) {
+    Assert.ok(fieldValues.includes(item),
+      `${table} should have ${expectedItems}`);
+  }
+}
+
+add_task(async function setup() {
+  await setupPlacesDatabase("places_v43.sqlite");
+
+  // Setup database contents to be migrated.
+  let path = OS.Path.join(OS.Constants.Path.profileDir, DB_FILENAME);
+  let db = await Sqlite.openConnection({ path });
+
+  let rows = await db.execute(`SELECT * FROM moz_bookmarks_deleted`);
+
+  Assert.equal(rows.length, 0, "Should be nothing in moz_bookmarks_deleted");
+
+  await assertItemIn(db, "moz_anno_attributes", "name", EXPECTED_REMOVED_ANNOTATIONS);
+  await assertItemIn(db, "moz_bookmarks", "guid", EXPECTED_REMOVED_BOOKMARK_GUIDS);
+  await assertItemIn(db, "moz_keywords", "keyword", EXPECTED_REMOVED_KEYWORDS);
+  await assertItemIn(db, "moz_places", "guid", EXPECTED_REMOVED_PLACES_ENTRIES);
+
+  await db.close();
+});
+
+add_task(async function database_is_valid() {
+  // Accessing the database for the first time triggers migration.
+  Assert.equal(PlacesUtils.history.databaseStatus,
+               PlacesUtils.history.DATABASE_STATUS_UPGRADED);
+
+  let db = await PlacesUtils.promiseDBConnection();
+  Assert.equal((await db.getSchemaVersion()), CURRENT_SCHEMA_VERSION);
+});
+
+add_task(async function test_roots_removed() {
+  let db = await PlacesUtils.promiseDBConnection();
+  let rows = await db.execute(`
+    SELECT id FROM moz_bookmarks
+    WHERE guid = :guid
+  `, {guid: PlacesUtils.bookmarks.rootGuid});
+  Assert.equal(rows.length, 1, "Should have exactly one root row.");
+  let rootId = rows[0].getResultByName("id");
+
+  rows = await db.execute(`
+    SELECT guid FROM moz_bookmarks
+    WHERE parent = :rootId`, {rootId});
+
+  Assert.equal(rows.length, EXPECTED_REMAINING_ROOTS.length,
+    "Should only have the built-in folder roots.");
+
+  for (let row of rows) {
+    let guid = row.getResultByName("guid");
+    Assert.ok(EXPECTED_REMAINING_ROOTS.includes(guid),
+      `Should have only the expected guids remaining, unexpected guid: ${guid}`);
+  }
+});
+
+add_task(async function test_tombstones_added() {
+  let db = await PlacesUtils.promiseDBConnection();
+
+  let rows = await db.execute(`
+    SELECT guid FROM moz_bookmarks_deleted
+  `);
+
+  for (let row of rows) {
+    let guid = row.getResultByName("guid");
+    Assert.ok(EXPECTED_REMOVED_BOOKMARK_GUIDS.includes(guid),
+      `Should have tombstoned the expected guids, unexpected guid: ${guid}`);
+  }
+
+  Assert.equal(rows.length, EXPECTED_REMOVED_BOOKMARK_GUIDS.length,
+    "Should have removed all the expected bookmarks.");
+});
+
+add_task(async function test_annotations_removed() {
+  let db = await PlacesUtils.promiseDBConnection();
+
+  for (let anno of EXPECTED_REMOVED_ANNOTATIONS) {
+    let rows = await db.execute(`
+      SELECT id FROM moz_anno_attributes
+      WHERE name = :anno
+    `, {anno});
+
+    Assert.equal(rows.length, 0, `${anno} should not exist in the database`);
+  }
+});
+
+add_task(async function test_check_history_entries() {
+  let db = await PlacesUtils.promiseDBConnection();
+
+  for (let entry of EXPECTED_REMOVED_PLACES_ENTRIES) {
+    let rows = await db.execute(`
+      SELECT id FROM moz_places
+      WHERE guid = '${entry}'`);
+
+    Assert.equal(rows.length, 0,
+      `Should have removed an orphaned history entry ${EXPECTED_REMOVED_PLACES_ENTRIES}.`);
+  }
+
+  let rows = await db.execute(`
+    SELECT foreign_count FROM moz_places
+    WHERE guid = :guid
+  `, {guid: EXPECTED_KEPT_PLACES_ENTRY});
+
+  Assert.equal(rows.length, 1,
+    `Should have kept visited history entry ${EXPECTED_KEPT_PLACES_ENTRY}`);
+
+  let foreignCount = rows[0].getResultByName("foreign_count");
+  Assert.equal(foreignCount, 0,
+    `Should have updated the foreign_count for ${EXPECTED_KEPT_PLACES_ENTRY}`);
+});
+
+add_task(async function test_check_keyword_removed() {
+  let db = await PlacesUtils.promiseDBConnection();
+
+  for (let keyword of EXPECTED_REMOVED_KEYWORDS) {
+    let rows = await db.execute(`
+      SELECT keyword FROM moz_keywords
+      WHERE keyword = :keyword
+    `, {keyword});
+
+    Assert.equal(rows.length, 0,
+      `Should have removed the expected keyword: ${keyword}.`);
+  }
+});
+
+add_task(async function test_no_orphan_annotations() {
+  let db = await PlacesUtils.promiseDBConnection();
+
+  let rows = await db.execute(`
+    SELECT item_id FROM moz_items_annos
+    WHERE item_id NOT IN (SELECT id from moz_bookmarks)
+  `);
+
+  Assert.equal(rows.length, 0,
+    `Should have no orphan annotations.`);
+
+  rows = await db.execute(`
+    SELECT id FROM moz_anno_attributes
+    WHERE id NOT IN (SELECT id from moz_items_annos)
+  `);
+
+  Assert.equal(rows.length, 0,
+    `Should have no orphan annotation attributes.`);
+});
+
+add_task(async function test_mobile_bookmarks_root_still_exists() {
+  let db = await PlacesUtils.promiseDBConnection();
+
+  let rows = await db.execute(`
+    SELECT id FROM moz_anno_attributes
+    WHERE name = 'mobile/bookmarksRoot'
+  `);
+
+  Assert.equal(rows.length, 1,
+    "Mobile bookmarks root annotation should still exist");
+});
+
+add_task(async function test_no_orphan_keywords() {
+  let db = await PlacesUtils.promiseDBConnection();
+
+  let rows = await db.execute(`
+    SELECT place_id FROM moz_keywords
+    WHERE place_id NOT IN (SELECT id from moz_places)
+  `);
+
+  Assert.equal(rows.length, 0,
+    `Should have no orphan keywords.`);
+});
--- a/toolkit/components/places/tests/migration/xpcshell.ini
+++ b/toolkit/components/places/tests/migration/xpcshell.ini
@@ -5,19 +5,21 @@ support-files =
   favicons_v41.sqlite
   places_outdated.sqlite
   places_v31.sqlite
   places_v34.sqlite
   places_v35.sqlite
   places_v36.sqlite
   places_v38.sqlite
   places_v42.sqlite
+  places_v43.sqlite
 
 [test_current_from_downgraded.js]
 [test_current_from_outdated.js]
 [test_current_from_v31.js]
 [test_current_from_v34.js]
 [test_current_from_v34_no_roots.js]
 [test_current_from_v35.js]
 [test_current_from_v36.js]
 [test_current_from_v38.js]
 [test_current_from_v41.js]
 [test_current_from_v42.js]
+[test_current_from_v43.js]
author	Mark Banner <standard8@mozilla.com>
	Fri, 02 Mar 2018 11:09:12 +0000
changeset 767467	ccb4e30c799479d3a0efabd63db1ab9085e124ad
parent 767466	4e29f78ae9b3345e86bdc4ec6251b6927aae0039
child 767569	058db4a662dd95aea859292cf08e4f05554f6c0c
push id	102609
push user	bmo:standard8@mozilla.com
push date	Wed, 14 Mar 2018 17:33:35 +0000
reviewers	mak
bugs	1310295
milestone	61.0a1
toolkit/components/places/Database.cpp		file \| annotate \| diff \| comparison \| revisions
toolkit/components/places/Database.h		file \| annotate \| diff \| comparison \| revisions
toolkit/components/places/tests/head_common.js		file \| annotate \| diff \| comparison \| revisions
toolkit/components/places/tests/migration/places_v43.sqlite		file \| annotate \| diff \| comparison \| revisions
toolkit/components/places/tests/migration/test_current_from_v43.js		file \| annotate \| diff \| comparison \| revisions
toolkit/components/places/tests/migration/xpcshell.ini		file \| annotate \| diff \| comparison \| revisions