Bug 1446886 - SyncedBookmarksMirror now uses PlacesUtils.bookmarks.userContentRoots instead of duplicating the guids. r?kitcambridge draft
authorMark Hammond <mhammond@skippinet.com.au>
Mon, 19 Mar 2018 17:49:20 +1100
changeset 769233 4c9d1e388106c107de7b4c5d4a7574c180122324
parent 767726 0d81c80876dd09536f78d1158e1e6ff78f9ad226
push id103078
push userbmo:markh@mozilla.com
push dateMon, 19 Mar 2018 06:50:34 +0000
reviewerskitcambridge
bugs1446886
milestone61.0a1
Bug 1446886 - SyncedBookmarksMirror now uses PlacesUtils.bookmarks.userContentRoots instead of duplicating the guids. r?kitcambridge MozReview-Commit-ID: GhGvIdM6Tg0
toolkit/components/places/SyncedBookmarksMirror.jsm
--- a/toolkit/components/places/SyncedBookmarksMirror.jsm
+++ b/toolkit/components/places/SyncedBookmarksMirror.jsm
@@ -65,16 +65,20 @@ XPCOMUtils.defineLazyModuleGetters(this,
   PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
   Sqlite: "resource://gre/modules/Sqlite.jsm",
 });
 
 XPCOMUtils.defineLazyGetter(this, "MirrorLog", () =>
   Log.repository.getLogger("Sync.Engine.Bookmarks.Mirror")
 );
 
+XPCOMUtils.defineLazyGetter(this, "UserContentRootsAsSqlList", () =>
+  PlacesUtils.bookmarks.userContentRoots.map(v => `'${v}'`).join(",")
+);
+
 // These can be removed once they're exposed in a central location (bug
 // 1375896).
 const DB_URL_LENGTH_MAX = 65536;
 const DB_TITLE_LENGTH_MAX = 4096;
 const DB_DESCRIPTION_LENGTH_MAX = 256;
 
 const SQLITE_MAX_VARIABLE_NUMBER = 999;
 
@@ -866,18 +870,17 @@ class SyncedBookmarksMirror {
        FROM items v
        LEFT JOIN moz_bookmarks b ON v.guid = b.guid
        WHERE v.needsMerge AND
        (NOT v.isDeleted OR b.guid NOT NULL)
       ) OR EXISTS (
        WITH RECURSIVE
        syncedItems(id, syncChangeCounter) AS (
          SELECT b.id, b.syncChangeCounter FROM moz_bookmarks b
-         WHERE b.guid IN ('menu________', 'toolbar_____', 'unfiled_____',
-                          'mobile______')
+         WHERE b.guid IN (${UserContentRootsAsSqlList})
          UNION ALL
          SELECT b.id, b.syncChangeCounter FROM moz_bookmarks b
          JOIN syncedItems s ON b.parent = s.id
        )
        SELECT 1
        FROM syncedItems
        WHERE syncChangeCounter > 0
       ) OR EXISTS (
@@ -1014,17 +1017,17 @@ class SyncedBookmarksMirror {
     // This unsightly query collects all descendants and maps their Places types
     // to the Sync record kinds. We start with the roots, and work our way down.
     // The list of roots in `syncedItems` should be updated if we add new
     // syncable roots to Places.
     let itemRows = await this.db.execute(`
       WITH RECURSIVE
       syncedItems(id, level) AS (
         SELECT b.id, 0 AS level FROM moz_bookmarks b
-        WHERE b.guid IN (:menuGuid, :toolbarGuid, :unfiledGuid, :mobileGuid)
+        WHERE b.guid IN (${UserContentRootsAsSqlList})
         UNION ALL
         SELECT b.id, s.level + 1 AS level FROM moz_bookmarks b
         JOIN syncedItems s ON s.id = b.parent
       )
       SELECT b.id, b.guid, p.guid AS parentGuid,
              /* Map Places item types to Sync record kinds. */
              (CASE b.type
                 WHEN :bookmarkType THEN (
@@ -1043,21 +1046,17 @@ class SyncedBookmarksMirror {
                   ) THEN :livemarkKind
                   ELSE :folderKind END)
                 ELSE :separatorKind END) AS kind,
              b.lastModified / 1000 AS localModified, b.syncChangeCounter
       FROM moz_bookmarks b
       JOIN moz_bookmarks p ON p.id = b.parent
       JOIN syncedItems s ON s.id = b.id
       ORDER BY s.level, b.parent, b.position`,
-      { menuGuid: PlacesUtils.bookmarks.menuGuid,
-        toolbarGuid: PlacesUtils.bookmarks.toolbarGuid,
-        unfiledGuid: PlacesUtils.bookmarks.unfiledGuid,
-        mobileGuid: PlacesUtils.bookmarks.mobileGuid,
-        bookmarkType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+      { bookmarkType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         queryKind: SyncedBookmarksMirror.KIND.QUERY,
         bookmarkKind: SyncedBookmarksMirror.KIND.BOOKMARK,
         folderType: PlacesUtils.bookmarks.TYPE_FOLDER,
         feedURLAnno: PlacesUtils.LMANNO_FEEDURI,
         livemarkKind: SyncedBookmarksMirror.KIND.LIVEMARK,
         folderKind: SyncedBookmarksMirror.KIND.FOLDER,
         separatorKind: SyncedBookmarksMirror.KIND.SEPARATOR });
 
@@ -1540,17 +1539,17 @@ class SyncedBookmarksMirror {
             b.dateAdded / 1000 < v.dateAdded`,
       { valueState: BookmarkMergeState.TYPE.REMOTE });
 
     // Stage remaining locally changed items for upload.
     await this.db.execute(`
       WITH RECURSIVE
       syncedItems(id, level) AS (
         SELECT b.id, 0 AS level FROM moz_bookmarks b
-        WHERE b.guid IN (:menuGuid, :toolbarGuid, :unfiledGuid, :mobileGuid)
+        WHERE b.guid IN (${UserContentRootsAsSqlList})
         UNION ALL
         SELECT b.id, s.level + 1 AS level FROM moz_bookmarks b
         JOIN syncedItems s ON s.id = b.parent
       )
       INSERT INTO itemsToUpload(id, guid, syncChangeCounter, parentGuid,
                                 parentTitle, dateAdded, type, title, isQuery,
                                 url, tags, description, loadInSidebar,
                                 smartBookmarkName, keyword, feedURL, siteURL,
@@ -1591,21 +1590,17 @@ class SyncedBookmarksMirror {
              b.position
       FROM moz_bookmarks b
       JOIN moz_bookmarks p ON p.id = b.parent
       JOIN syncedItems s ON s.id = b.id
       LEFT JOIN moz_places h ON h.id = b.fk
       LEFT JOIN itemsToWeaklyReupload w ON w.id = b.id
       WHERE b.syncChangeCounter >= 1 OR
             w.id NOT NULL`,
-      { menuGuid: PlacesUtils.bookmarks.menuGuid,
-        toolbarGuid: PlacesUtils.bookmarks.toolbarGuid,
-        unfiledGuid: PlacesUtils.bookmarks.unfiledGuid,
-        mobileGuid: PlacesUtils.bookmarks.mobileGuid,
-        bookmarkType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+      { bookmarkType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         tagsGuid: PlacesUtils.bookmarks.tagsGuid,
         descriptionAnno: PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO,
         sidebarAnno: PlacesSyncUtils.bookmarks.SIDEBAR_ANNO,
         smartBookmarkAnno: PlacesSyncUtils.bookmarks.SMART_BOOKMARKS_ANNO,
         folderType: PlacesUtils.bookmarks.TYPE_FOLDER,
         feedURLAnno: PlacesUtils.LMANNO_FEEDURI,
         siteURLAnno: PlacesUtils.LMANNO_SITEURI });
 
@@ -1982,51 +1977,38 @@ async function initializeMirrorDatabase(
 
   await db.execute(`CREATE INDEX mirror.urlHashes ON urls(hash)`);
 
   await createMirrorRoots(db);
 }
 
 /**
  * Sets up the syncable roots. All items in the mirror should descend from these
- * roots. If we ever add new syncable roots to Places, this function should also
- * be updated to create them in the mirror.
+ * roots.
  *
  * @param {Sqlite.OpenedConnection} db
  *        The mirror database connection.
  */
 async function createMirrorRoots(db) {
   const syncableRoots = [{
     guid: PlacesUtils.bookmarks.rootGuid,
     // The Places root is its own parent, to satisfy the foreign key and
     // `NOT NULL` constraints on `structure`.
     parentGuid: PlacesUtils.bookmarks.rootGuid,
     position: -1,
     needsMerge: false,
-  }, {
-    guid: PlacesUtils.bookmarks.menuGuid,
-    parentGuid: PlacesUtils.bookmarks.rootGuid,
-    position: 0,
-    needsMerge: true,
-  }, {
-    guid: PlacesUtils.bookmarks.toolbarGuid,
-    parentGuid: PlacesUtils.bookmarks.rootGuid,
-    position: 1,
-    needsMerge: true,
-  }, {
-    guid: PlacesUtils.bookmarks.unfiledGuid,
-    parentGuid: PlacesUtils.bookmarks.rootGuid,
-    position: 2,
-    needsMerge: true,
-  }, {
-    guid: PlacesUtils.bookmarks.mobileGuid,
-    parentGuid: PlacesUtils.bookmarks.rootGuid,
-    position: 3,
-    needsMerge: true,
-  }];
+  }, ...PlacesUtils.bookmarks.userContentRoots.map((guid, position) => {
+    return {
+      guid,
+      parentGuid: PlacesUtils.bookmarks.rootGuid,
+      position,
+      needsMerge: true,
+    };
+  })];
+
   for (let { guid, parentGuid, position, needsMerge } of syncableRoots) {
     await db.executeCached(`
       INSERT INTO items(guid, kind, needsMerge)
       VALUES(:guid, :kind, :needsMerge)`,
       { guid, kind: SyncedBookmarksMirror.KIND.FOLDER, needsMerge });
 
     await db.executeCached(`
       INSERT INTO structure(guid, parentGuid, position)