Bug 1302286 - Remove `_getNode` and `_getChildren` from the bookmarks engine. r?markh
MozReview-Commit-ID: 10HScO81AK3
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -426,21 +426,17 @@ BookmarksEngine.prototype = {
let mapped = this._mapDupe(item);
this._log.debug(item.id + " mapped to " + mapped);
// We must return a string, not an object, and the entries in the GUIDMap
// are created via "new String()" making them an object.
return mapped ? mapped.toString() : mapped;
},
pullAllChanges() {
- let changeset = new BookmarksChangeset();
- for (let id in this._store.getAllIDs()) {
- changeset.set(id, { modified: 0, deleted: false });
- }
- return changeset;
+ return new BookmarksChangeset(this._store.getAllIDs());
},
pullNewChanges() {
let modifiedGUIDs = this._getModifiedGUIDs();
if (!modifiedGUIDs.length) {
return new BookmarksChangeset(this._tracker.changedIDs);
}
@@ -456,25 +452,26 @@ BookmarksEngine.prototype = {
// of bound parameters per query.
for (let startIndex = 0;
startIndex < modifiedGUIDs.length;
startIndex += SQLITE_MAX_VARIABLE_NUMBER) {
let chunkLength = Math.min(startIndex + SQLITE_MAX_VARIABLE_NUMBER,
modifiedGUIDs.length);
+ // Change root IDs don't need to be escaped because they're integers and
+ // not vulnerable to injections.
+ let changeRootIds = PlacesSyncUtils.bookmarks.getChangeRootIds();
let query = `
WITH RECURSIVE
modifiedGuids(guid) AS (
VALUES ${new Array(chunkLength).fill("(?)").join(", ")}
),
syncedItems(id) AS (
- SELECT b.id
- FROM moz_bookmarks b
- WHERE b.id IN (${getChangeRootIds().join(", ")})
+ VALUES ${changeRootIds.map(id => `(${id})`).join(", ")}
UNION ALL
SELECT b.id
FROM moz_bookmarks b
JOIN syncedItems s ON b.parent = s.id
)
SELECT b.guid, b.id
FROM modifiedGuids m
JOIN moz_bookmarks b ON b.guid = m.guid
@@ -722,23 +719,16 @@ BookmarksStore.prototype = {
this._log.debug("Changing GUID " + oldID + " to " + newID);
Async.promiseSpinningly(PlacesSyncUtils.bookmarks.changeGuid(
BookmarkSpecialIds.syncIDToPlacesGUID(oldID),
BookmarkSpecialIds.syncIDToPlacesGUID(newID)
));
},
- _getNode: function BStore__getNode(folder) {
- let query = PlacesUtils.history.getNewQuery();
- query.setFolders([folder], 1);
- return PlacesUtils.history.executeQuery(
- query, PlacesUtils.history.getNewQueryOptions()).root;
- },
-
_getTags: function BStore__getTags(uri) {
try {
if (typeof(uri) == "string")
uri = Utils.makeURI(uri);
} catch(e) {
this._log.warn("Could not parse URI \"" + uri + "\": " + e);
}
return PlacesUtils.tagging.getTagsForURI(uri, {});
@@ -911,61 +901,41 @@ BookmarksStore.prototype = {
let result = Async.querySpinningly(this._frecencyStm, this._frecencyCols);
if (result.length)
index += result[0].frecency;
}
return index;
},
- _getChildren: function BStore_getChildren(guid, items) {
- let node = guid; // the recursion case
- if (typeof(node) == "string") { // callers will give us the guid as the first arg
- let nodeID = this.idForGUID(guid, true);
- if (!nodeID) {
- this._log.debug("No node for GUID " + guid + "; returning no children.");
- return items;
- }
- node = this._getNode(nodeID);
+ getAllIDs: function BStore_getAllIDs() {
+ let items = {};
+
+ let changeRootIds = PlacesSyncUtils.bookmarks.getChangeRootIds();
+ let query = `
+ WITH RECURSIVE
+ changeRootContents(id) AS (
+ VALUES ${changeRootIds.map(id => `(${id})`).join(", ")}
+ UNION ALL
+ SELECT b.id
+ FROM moz_bookmarks b
+ JOIN changeRootContents c ON b.parent = c.id
+ )
+ SELECT id, guid
+ FROM changeRootContents
+ JOIN moz_bookmarks USING (id)
+ `;
+
+ let statement = this._getStmt(query);
+ let results = Async.querySpinningly(statement, ["id", "guid"]);
+ for (let { id, guid } of results) {
+ let syncID = BookmarkSpecialIds.specialGUIDForId(id) || guid;
+ items[syncID] = { modified: 0, deleted: false };
}
- if (node.type == node.RESULT_TYPE_FOLDER) {
- node.QueryInterface(Ci.nsINavHistoryQueryResultNode);
- node.containerOpen = true;
- try {
- // Remember all the children GUIDs and recursively get more
- for (let i = 0; i < node.childCount; i++) {
- let child = node.getChild(i);
- items[this.GUIDForId(child.itemId)] = true;
- this._getChildren(child, items);
- }
- }
- finally {
- node.containerOpen = false;
- }
- }
-
- return items;
- },
-
- getAllIDs: function BStore_getAllIDs() {
- let items = {"menu": true,
- "toolbar": true,
- "unfiled": true,
- };
- // We also want "mobile" but only if a local mobile folder already exists
- // (otherwise we'll later end up creating it, which we want to avoid until
- // we actually need it.)
- if (BookmarkSpecialIds.findMobileRoot(false)) {
- items["mobile"] = true;
- }
- for (let guid of BookmarkSpecialIds.guids) {
- if (guid != "places" && guid != "tags")
- this._getChildren(guid, items);
- }
return items;
},
wipe: function BStore_wipe() {
let cb = Async.makeSpinningCallback();
Task.spawn(function* () {
// Save a backup before clearing out all bookmarks.
yield PlacesBackups.create(null, true);
@@ -1253,30 +1223,14 @@ BookmarksTracker.prototype = {
if (--this._batchDepth === 0 && this._batchSawScoreIncrement) {
this.score += SCORE_INCREMENT_XLARGE;
this._batchSawScoreIncrement = false;
}
},
onItemVisited: function () {}
};
-// Returns an array of root IDs to recursively query for synced bookmarks.
-// Items in other roots, including tags and organizer queries, will be
-// ignored.
-function getChangeRootIds() {
- let rootIds = [
- PlacesUtils.bookmarksMenuFolderId,
- PlacesUtils.toolbarFolderId,
- PlacesUtils.unfiledBookmarksFolderId,
- ];
- let mobileRootId = BookmarkSpecialIds.findMobileRoot(false);
- if (mobileRootId) {
- rootIds.push(mobileRootId);
- }
- return rootIds;
-}
-
class BookmarksChangeset extends Changeset {
getModifiedTimestamp(id) {
let change = this.changes[id];
return change ? change.modified : Number.NaN;
}
}
--- a/services/sync/tests/unit/test_bookmark_engine.js
+++ b/services/sync/tests/unit/test_bookmark_engine.js
@@ -127,19 +127,16 @@ add_task(function* bad_record_allIDs() {
Utils.makeURI("place:folder=1138"),
PlacesUtils.bookmarks.DEFAULT_INDEX,
null);
do_check_true(badRecordID > 0);
_("Record is " + badRecordID);
_("Type: " + PlacesUtils.bookmarks.getItemType(badRecordID));
- _("Fetching children.");
- store._getChildren("toolbar", {});
-
_("Fetching all IDs.");
let all = store.getAllIDs();
_("All IDs: " + JSON.stringify(all));
do_check_true("menu" in all);
do_check_true("toolbar" in all);
_("Clean up.");
--- a/services/sync/tests/unit/test_bookmark_places_query_rewriting.js
+++ b/services/sync/tests/unit/test_bookmark_places_query_rewriting.js
@@ -28,25 +28,28 @@ function run_test() {
let uri = "place:folder=499&type=7&queryType=1";
let tagRecord = makeTagRecord("abcdefabcdef", uri);
_("Type: " + tagRecord.type);
_("Folder name: " + tagRecord.folderName);
store.applyIncoming(tagRecord);
- let tags = store._getNode(PlacesUtils.tagsFolderId);
- tags.containerOpen = true;
+ let tags = PlacesUtils.getFolderContents(PlacesUtils.tagsFolderId).root;
let tagID;
- for (let i = 0; i < tags.childCount; ++i) {
- let child = tags.getChild(i);
- if (child.title == "bar")
- tagID = child.itemId;
+ try {
+ for (let i = 0; i < tags.childCount; ++i) {
+ let child = tags.getChild(i);
+ if (child.title == "bar") {
+ tagID = child.itemId;
+ }
+ }
+ } finally {
+ tags.containerOpen = false;
}
- tags.containerOpen = false;
_("Tag ID: " + tagID);
let insertedRecord = store.createRecord("abcdefabcdef", "bookmarks");
do_check_eq(insertedRecord.bmkUri, uri.replace("499", tagID));
_("... but not if the type is wrong.");
let wrongTypeURI = "place:folder=499&type=2&queryType=1";
let wrongTypeRecord = makeTagRecord("fedcbafedcba", wrongTypeURI);
--- a/toolkit/components/places/PlacesSyncUtils.jsm
+++ b/toolkit/components/places/PlacesSyncUtils.jsm
@@ -32,29 +32,50 @@ var PlacesSyncUtils = {};
const SMART_BOOKMARKS_ANNO = "Places/SmartBookmark";
const DESCRIPTION_ANNO = "bookmarkProperties/description";
const SIDEBAR_ANNO = "bookmarkProperties/loadInSidebar";
const PARENT_ANNO = "sync/parent";
const { SOURCE_SYNC } = Ci.nsINavBookmarksService;
const BookmarkSyncUtils = PlacesSyncUtils.bookmarks = Object.freeze({
+ SYNC_MOBILE_ROOT_ANNO: "mobile/bookmarksRoot",
+
KINDS: {
BOOKMARK: "bookmark",
// Microsummaries were removed from Places in bug 524091. For now, Sync
// treats them identically to bookmarks. Bug 745410 tracks removing them
// entirely.
MICROSUMMARY: "microsummary",
QUERY: "query",
FOLDER: "folder",
LIVEMARK: "livemark",
SEPARATOR: "separator",
},
/**
+ * Returns an array of Places root IDs to recursively query for synced
+ * bookmarks. This includes the menu, toolbar, unfiled bookmarks, and
+ * mobile bookmarks. Items in other roots, like tags and left pane queries,
+ * are not tracked by Sync.
+ */
+ getChangeRootIds() {
+ let rootIds = [
+ PlacesUtils.bookmarksMenuFolderId,
+ PlacesUtils.toolbarFolderId,
+ PlacesUtils.unfiledBookmarksFolderId,
+ ];
+ let mobileRootId = findMobileRoot();
+ if (mobileRootId) {
+ rootIds.push(mobileRootId);
+ }
+ return rootIds;
+ },
+
+ /**
* Fetches a folder's children, ordered by their position within the folder.
*/
fetchChildGuids: Task.async(function* (parentGuid) {
PlacesUtils.SYNC_BOOKMARK_VALIDATORS.guid(parentGuid);
let db = yield PlacesUtils.promiseDBConnection();
let children = yield fetchAllChildren(db, parentGuid);
return children.map(child => child.guid);
@@ -852,8 +873,16 @@ var getOrCreateTagFolder = Task.async(fu
// Create the tag if it doesn't exist.
let item = yield PlacesUtils.bookmarks.insert({
type: PlacesUtils.bookmarks.TYPE_FOLDER,
parentGuid: PlacesUtils.bookmarks.tagsGuid,
title: tag,
});
return PlacesUtils.promiseItemId(item.guid);
});
+
+// Returns the Sync mobile bookmarks root ID, or `null` if one doesn't exist.
+// This function will be removed in bug 647605.
+function findMobileRoot() {
+ let mobileRoot = PlacesUtils.annotations.getItemsWithAnnotation(
+ BookmarkSyncUtils.SYNC_MOBILE_ROOT_ANNO, {});
+ return mobileRoot.length ? mobileRoot[0] : null;
+}