Bug 1472127 - Update to Firefox 61.0 may wrongly remove incorrectly parented roots. r=standard8, a=RyanVM
authorMarco Bonardo <mbonardo@mozilla.com>
Fri, 29 Jun 2018 17:46:59 +0200
changeset 473796 ced0b5dc71eb
parent 473795 1b56c49591c5
child 473797 1d6943a03afb
push id1737
push userryanvm@gmail.com
push dateMon, 02 Jul 2018 16:05:08 +0000
treeherdermozilla-release@ced0b5dc71eb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersstandard8, RyanVM
bugs1472127
milestone61.0.1
Bug 1472127 - Update to Firefox 61.0 may wrongly remove incorrectly parented roots. r=standard8, a=RyanVM MozReview-Commit-ID: 6ZRBERi0PaW
toolkit/components/places/Database.cpp
toolkit/components/places/tests/migration/test_current_from_v43.js
--- a/toolkit/components/places/Database.cpp
+++ b/toolkit/components/places/Database.cpp
@@ -2116,16 +2116,17 @@ Database::MigrateV44Up() {
     "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 "
+      "WHERE b.guid NOT IN ('menu________', 'toolbar_____', 'tags________', 'unfiled_____', 'mobile______') "
     ") "
     "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;
--- a/toolkit/components/places/tests/migration/test_current_from_v43.js
+++ b/toolkit/components/places/tests/migration/test_current_from_v43.js
@@ -15,16 +15,17 @@ const EXPECTED_REMOVED_BOOKMARK_GUIDS = 
   "xGGhZK3b6GnW", // Downloads
   "EJG6I1nKkQFQ", // Tags
   "gSyHo5oNSUJV", // All Bookmarks
   // These are simulated add-on injections that we expect to be removed.
   "exaddon_____",
   "exaddon1____",
   "exaddon2____",
   "exaddon3____",
+  "test________",
 ];
 
 const EXPECTED_REMOVED_ANNOTATIONS = [
   "PlacesOrganizer/OrganizerFolder",
   "PlacesOrganizer/OrganizerQuery",
 ];
 
 const EXPECTED_REMOVED_PLACES_ENTRIES = ["exaddonh____", "exaddonh3___"];
@@ -48,18 +49,25 @@ async function assertItemIn(db, table, f
 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");
 
-  Assert.equal(rows.length, 0, "Should be nothing in moz_bookmarks_deleted");
+
+  // Break roots parenting, to test for Bug 1472127.
+  await db.execute(`INSERT INTO moz_bookmarks (title, parent, position, guid)
+                    VALUES ("test", 1, 0, "test________")`);
+  await db.execute(`UPDATE moz_bookmarks
+                    SET parent = (SELECT id FROM moz_bookmarks WHERE guid = "test________")
+                    WHERE guid = "menu________"`);
 
   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();
 });
@@ -80,25 +88,32 @@ add_task(async function test_roots_remov
     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,
+  // Note the -1 here is because we reparented the menu root wrongly above.
+  Assert.equal(rows.length, EXPECTED_REMAINING_ROOTS.length - 1,
     "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}`);
   }
+
+  // Check the reparented menu now.
+  rows = await db.execute(`
+    SELECT id FROM moz_bookmarks
+    WHERE guid = :guid
+  `, {guid: PlacesUtils.bookmarks.menuGuid});
+  Assert.equal(rows.length, 1, "Should have found the menu root.");
 });
 
 add_task(async function test_tombstones_added() {
   let db = await PlacesUtils.promiseDBConnection();
 
   let rows = await db.execute(`
     SELECT guid FROM moz_bookmarks_deleted
   `);