Bug 1306445 - Remove the mobile bookmarks annotation from the mobile root as it is no longer required. r=mak

MozReview-Commit-ID: 4HhxEpY7NNs

--HG--
extra : rebase_source : da839516ea71b8ef38e9c1e2c30a11b823de07da

toolkit/components/places/Database.cpp

@@ -108,6 +108,8 @@
 #define LMANNO_SITEURI "livemark/siteURI"
 
 #define MOBILE_ROOT_GUID "mobile______"
+// This is no longer used & obsolete except for during migration.
+// Note: it may still be found in older places databases.
 #define MOBILE_ROOT_ANNO "mobile/bookmarksRoot"
 
 // We use a fixed title for the mobile root to avoid marking the database as
@@ -2218,55 +2220,6 @@ Database::CreateMobileRoot()
   rv = findIdStmt->GetInt64(0, &rootId);
   if (NS_FAILED(rv)) return -1;
 
-  // Set the mobile bookmarks anno on the new root, so that Sync code on an
-  // older channel can still find it in case of a downgrade. This can be
-  // removed in bug 1306445.
-  nsCOMPtr<mozIStorageStatement> addAnnoNameStmt;
-  rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
-    "INSERT OR IGNORE INTO moz_anno_attributes (name) VALUES (:anno_name)"
-  ), getter_AddRefs(addAnnoNameStmt));
-  if (NS_FAILED(rv)) return -1;
-  mozStorageStatementScoper addAnnoNameScoper(addAnnoNameStmt);
-
-  rv = addAnnoNameStmt->BindUTF8StringByName(
-    NS_LITERAL_CSTRING("anno_name"), NS_LITERAL_CSTRING(MOBILE_ROOT_ANNO));
-  if (NS_FAILED(rv)) return -1;
-  rv = addAnnoNameStmt->Execute();
-  if (NS_FAILED(rv)) return -1;
-
-  nsCOMPtr<mozIStorageStatement> addAnnoStmt;
-  rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
-    "INSERT OR IGNORE INTO moz_items_annos "
-      "(id, item_id, anno_attribute_id, content, flags, "
-       "expiration, type, dateAdded, lastModified) "
-    "SELECT "
-      "(SELECT a.id FROM moz_items_annos a "
-       "WHERE a.anno_attribute_id = n.id AND "
-             "a.item_id = :root_id), "
-      ":root_id, n.id, 1, 0, :expiration, :type, :timestamp, :timestamp "
-    "FROM moz_anno_attributes n WHERE name = :anno_name"
-  ), getter_AddRefs(addAnnoStmt));
-  if (NS_FAILED(rv)) return -1;
-  mozStorageStatementScoper addAnnoScoper(addAnnoStmt);
-
-  rv = addAnnoStmt->BindInt64ByName(NS_LITERAL_CSTRING("root_id"), rootId);
-  if (NS_FAILED(rv)) return -1;
-  rv = addAnnoStmt->BindUTF8StringByName(
-    NS_LITERAL_CSTRING("anno_name"), NS_LITERAL_CSTRING(MOBILE_ROOT_ANNO));
-  if (NS_FAILED(rv)) return -1;
-  rv = addAnnoStmt->BindInt32ByName(NS_LITERAL_CSTRING("expiration"),
-                                    nsIAnnotationService::EXPIRE_NEVER);
-  if (NS_FAILED(rv)) return -1;
-  rv = addAnnoStmt->BindInt32ByName(NS_LITERAL_CSTRING("type"),
-                                    nsIAnnotationService::TYPE_INT32);
-  if (NS_FAILED(rv)) return -1;
-  rv = addAnnoStmt->BindInt32ByName(NS_LITERAL_CSTRING("timestamp"),
-                                    RoundedPRNow());
-  if (NS_FAILED(rv)) return -1;
-
-  rv = addAnnoStmt->Execute();
-  if (NS_FAILED(rv)) return -1;
-
   return rootId;
 }
 

toolkit/components/places/PlacesSyncUtils.jsm

@@ -382,7 +382,6 @@ const BookmarkSyncUtils = PlacesSyncUtils.bookmarks = Object.freeze({
   DESCRIPTION_ANNO: "bookmarkProperties/description",
   SIDEBAR_ANNO: "bookmarkProperties/loadInSidebar",
   SYNC_PARENT_ANNO: "sync/parent",
-  SYNC_MOBILE_ROOT_ANNO: "mobile/bookmarksRoot",
 
   SYNC_ID_META_KEY: "sync/bookmarks/syncId",
   LAST_SYNC_META_KEY: "sync/bookmarks/lastSync",

toolkit/components/places/PlacesUtils.jsm

@@ -329,6 +329,7 @@ var PlacesUtils = {
   POST_DATA_ANNO: "bookmarkProperties/POSTData",
   READ_ONLY_ANNO: "placesInternal/READ_ONLY",
   CHARSET_ANNO: "URIProperties/characterSet",
+  // Deprecated: This is only used for supporting import from older datasets.
   MOBILE_ROOT_ANNO: "mobile/bookmarksRoot",
 
   TOPIC_SHUTDOWN: "places-shutdown",

toolkit/components/places/tests/bookmarks/test_bookmarks_eraseEverything.js

@@ -83,13 +83,9 @@ add_task(async function test_eraseEverything() {
   // Check there are no orphan annotations.
   let conn = await PlacesUtils.promiseDBConnection();
   let annoAttrs = await conn.execute(`SELECT id, name FROM moz_anno_attributes`);
-  // Bug 1306445 will eventually remove the mobile root anno.
-  Assert.equal(annoAttrs.length, 1);
-  Assert.equal(annoAttrs[0].getResultByName("name"), PlacesUtils.MOBILE_ROOT_ANNO);
+  Assert.equal(annoAttrs.length, 0);
   let annos = await conn.execute(`SELECT item_id, anno_attribute_id FROM moz_items_annos`);
-  Assert.equal(annos.length, 1);
-  Assert.equal(annos[0].getResultByName("item_id"), PlacesUtils.mobileFolderId);
-  Assert.equal(annos[0].getResultByName("anno_attribute_id"), annoAttrs[0].getResultByName("id"));
+  Assert.equal(annos.length, 0);
 });
 
 add_task(async function test_eraseEverything_roots() {

toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js

@@ -137,13 +137,9 @@ add_task(async function remove_bookmark_orphans() {
   // Check there are no orphan annotations.
   let conn = await PlacesUtils.promiseDBConnection();
   let annoAttrs = await conn.execute(`SELECT id, name FROM moz_anno_attributes`);
-  // Bug 1306445 will eventually remove the mobile root anno.
-  Assert.equal(annoAttrs.length, 1);
-  Assert.equal(annoAttrs[0].getResultByName("name"), PlacesUtils.MOBILE_ROOT_ANNO);
+  Assert.equal(annoAttrs.length, 0);
   let annos = await conn.execute(`SELECT item_id, anno_attribute_id FROM moz_items_annos`);
-  Assert.equal(annos.length, 1);
-  Assert.equal(annos[0].getResultByName("item_id"), PlacesUtils.mobileFolderId);
-  Assert.equal(annos[0].getResultByName("anno_attribute_id"), annoAttrs[0].getResultByName("id"));
+  Assert.equal(annos.length, 0);
 });
 
 add_task(async function remove_bookmark_empty_title() {

toolkit/components/places/tests/migration/test_current_from_v34.js

@@ -131,11 +131,4 @@ add_task(async function test_mobile_root() {
     "Thunderbird bookmark should be moved to new mobile root");
   equal(tbBmk.index, 1,
     "Thunderbird bookmark should be second child of new root");
-
-  let mobileRootId = await PlacesUtils.promiseItemId(
-    PlacesUtils.bookmarks.mobileGuid);
-  let annoItemIds = PlacesUtils.annotations.getItemsWithAnnotation(
-    PlacesUtils.MOBILE_ROOT_ANNO, {});
-  deepEqual(annoItemIds, [mobileRootId],
-    "Only mobile root should have mobile anno");
 });

toolkit/components/places/tests/migration/test_current_from_v43.js

@@ -185,18 +185,6 @@ add_task(async function test_no_orphan_annotations() {
     `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();
 

toolkit/components/places/tests/unit/test_import_mobile_bookmarks.js

@@ -48,12 +48,6 @@ add_task(async function test_restore_mobile_bookmarks_root() {
     }, {
       guid: PlacesUtils.bookmarks.mobileGuid,
       index: 4,
-      annos: [{
-        name: "mobile/bookmarksRoot",
-        flags: 0,
-        expires: 4,
-        value: 1,
-      }],
       children: [
         { guid: "_o8e1_zxTJFg", index: 0 },
         { guid: "QCtSqkVYUbXB", index: 1 },
@@ -89,12 +83,6 @@ add_task(async function test_import_mobile_bookmarks_root() {
     }, {
       guid: PlacesUtils.bookmarks.mobileGuid,
       index: 4,
-      annos: [{
-        name: "mobile/bookmarksRoot",
-        flags: 0,
-        expires: 4,
-        value: 1,
-      }],
       children: [
         // The first two are in ..._import.json, the second two are in
         // ..._merge.json
@@ -110,6 +98,8 @@ add_task(async function test_import_mobile_bookmarks_root() {
 });
 
 add_task(async function test_restore_mobile_bookmarks_folder() {
+  // This tests importing a mobile bookmarks folder with the annotation,
+  // and the old, random guid.
   await importFromFixture("mobile_bookmarks_folder_import.json",
                            /* replace */ true);
 
@@ -134,12 +124,6 @@ add_task(async function test_restore_mobile_bookmarks_folder() {
     }, {
       guid: PlacesUtils.bookmarks.mobileGuid,
       index: 4,
-      annos: [{
-        name: "mobile/bookmarksRoot",
-        flags: 0,
-        expires: 4,
-        value: 1,
-      }],
       children: [
         { guid: "_o8e1_zxTJFg", index: 0 },
         { guid: "QCtSqkVYUbXB", index: 1 },
@@ -186,12 +170,6 @@ add_task(async function test_import_mobile_bookmarks_folder() {
     }, {
       guid: PlacesUtils.bookmarks.mobileGuid,
       index: 4,
-      annos: [{
-        name: "mobile/bookmarksRoot",
-        flags: 0,
-        expires: 4,
-        value: 1,
-      }],
       children: [
         { guid: "_o8e1_zxTJFg", index: 0 },
         { guid: "QCtSqkVYUbXB", index: 1 },
@@ -230,12 +208,6 @@ add_task(async function test_restore_multiple_bookmarks_folders() {
     }, {
       guid: PlacesUtils.bookmarks.mobileGuid,
       index: 4,
-      annos: [{
-        name: "mobile/bookmarksRoot",
-        flags: 0,
-        expires: 4,
-        value: 1,
-      }],
       children: [
         { guid: "a17yW6-nTxEJ", index: 0 },
         { guid: "sSZ86WT9WbN3", index: 1 },
@@ -275,12 +247,6 @@ add_task(async function test_import_multiple_bookmarks_folders() {
     }, {
       guid: PlacesUtils.bookmarks.mobileGuid,
       index: 4,
-      annos: [{
-        name: "mobile/bookmarksRoot",
-        flags: 0,
-        expires: 4,
-        value: 1,
-      }],
       children: [
         { guid: "_o8e1_zxTJFg", index: 0 },
         { guid: "QCtSqkVYUbXB", index: 1 },

toolkit/components/places/tests/unit/test_telemetry.js

@@ -20,9 +20,7 @@ var histograms = {
   // PLACES_AUTOCOMPLETE_1ST_RESULT_TIME_MS:  val => do_check_true(val > 1),
   PLACES_IDLE_FRECENCY_DECAY_TIME_MS: val => Assert.ok(val >= 0),
   PLACES_IDLE_MAINTENANCE_TIME_MS: val => Assert.ok(val > 0),
-  // One from the `setItemAnnotation` call; the other from the mobile root.
-  // This can be removed along with the anno in bug 1306445.
-  PLACES_ANNOS_BOOKMARKS_COUNT: val => Assert.equal(val, 2),
+  PLACES_ANNOS_BOOKMARKS_COUNT: val => Assert.equal(val, 1),
   PLACES_ANNOS_PAGES_COUNT: val => Assert.equal(val, 1),
   PLACES_MAINTENANCE_DAYSFROMLAST: val => Assert.ok(val >= 0),
 };