Bug 1383621 - Use the async bookmarks API to insert the mobile query. r=markh
MozReview-Commit-ID: KaeXwFHEg7K
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -518,17 +518,17 @@ BookmarksEngine.prototype = {
async _orderChildren() {
await this._store._orderChildren();
this._store._childrenToOrder = {};
},
async _syncFinish() {
await SyncEngine.prototype._syncFinish.call(this);
- this._tracker._ensureMobileQuery();
+ await PlacesSyncUtils.bookmarks.ensureMobileQuery();
},
async _syncCleanup() {
await SyncEngine.prototype._syncCleanup.call(this);
delete this._guidMap;
},
async _createRecord(id) {
@@ -1018,62 +1018,16 @@ BookmarksTracker.prototype = {
if (IGNORED_SOURCES.includes(source)) {
return;
}
this._log.trace("onItemRemoved: " + itemId);
this._upScore();
},
- _ensureMobileQuery: function _ensureMobileQuery() {
- Services.prefs.setBoolPref("browser.bookmarks.showMobileBookmarks", true);
- let find = val =>
- PlacesUtils.annotations.getItemsWithAnnotation(ORGANIZERQUERY_ANNO, {}).filter(
- id => PlacesUtils.annotations.getItemAnnotation(id, ORGANIZERQUERY_ANNO) == val
- );
-
- // Don't continue if the Library isn't ready
- let all = find(ALLBOOKMARKS_ANNO);
- if (all.length == 0)
- return;
-
- let mobile = find(MOBILE_ANNO);
- let queryURI = Utils.makeURI("place:folder=" + PlacesUtils.mobileFolderId);
- let title = PlacesBundle.GetStringFromName("MobileBookmarksFolderTitle");
-
- // Don't add OR remove the mobile bookmarks if there's nothing.
- if (PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.mobileFolderId, 0) == -1) {
- if (mobile.length != 0)
- PlacesUtils.bookmarks.removeItem(mobile[0], SOURCE_SYNC);
- } else if (mobile.length == 0) {
- // Add the mobile bookmarks query if it doesn't exist
- let query = PlacesUtils.bookmarks.insertBookmark(all[0], queryURI, -1, title, /* guid */ null, SOURCE_SYNC);
- PlacesUtils.annotations.setItemAnnotation(query, ORGANIZERQUERY_ANNO, MOBILE_ANNO, 0,
- PlacesUtils.annotations.EXPIRE_NEVER, SOURCE_SYNC);
- PlacesUtils.annotations.setItemAnnotation(query, PlacesUtils.EXCLUDE_FROM_BACKUP_ANNO, 1, 0,
- PlacesUtils.annotations.EXPIRE_NEVER, SOURCE_SYNC);
- } else {
- // Make sure the existing query URL and title are correct
- if (!PlacesUtils.bookmarks.getBookmarkURI(mobile[0]).equals(queryURI)) {
- PlacesUtils.bookmarks.changeBookmarkURI(mobile[0], queryURI,
- SOURCE_SYNC);
- }
- let queryTitle = PlacesUtils.bookmarks.getItemTitle(mobile[0]);
- if (queryTitle != title) {
- PlacesUtils.bookmarks.setItemTitle(mobile[0], title, SOURCE_SYNC);
- }
- let rootTitle =
- PlacesUtils.bookmarks.getItemTitle(PlacesUtils.mobileFolderId);
- if (rootTitle != title) {
- PlacesUtils.bookmarks.setItemTitle(PlacesUtils.mobileFolderId, title,
- SOURCE_SYNC);
- }
- }
- },
-
// This method is oddly structured, but the idea is to return as quickly as
// possible -- this handler gets called *every time* a bookmark changes, for
// *each change*.
onItemChanged: function BMT_onItemChanged(itemId, property, isAnno, value,
lastModified, itemType, parentId,
guid, parentGuid, oldValue,
source) {
if (IGNORED_SOURCES.includes(source)) {
--- a/services/sync/tests/unit/test_bookmark_tracker.js
+++ b/services/sync/tests/unit/test_bookmark_tracker.js
@@ -1688,87 +1688,16 @@ add_task(async function test_onItemDelet
await verifyTrackedItems([fx_guid, tb_guid, folder1_guid, folder2_guid]);
do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 3);
} finally {
_("Clean up.");
await cleanup();
}
});
-add_task(async function test_mobile_query() {
- _("Ensure we correctly create the mobile query");
-
- try {
- await startTracking();
-
- // Creates the organizer queries as a side effect.
- let leftPaneId = PlacesUIUtils.leftPaneFolderId;
- _(`Left pane root ID: ${leftPaneId}`);
-
- let allBookmarksGuids = await PlacesSyncUtils.bookmarks.fetchGuidsWithAnno(
- "PlacesOrganizer/OrganizerQuery", "AllBookmarks");
- equal(allBookmarksGuids.length, 1, "Should create folder with all bookmarks queries");
- let allBookmarkGuid = allBookmarksGuids[0];
-
- _("Try creating query after organizer is ready");
- tracker._ensureMobileQuery();
- let queryGuids = await PlacesSyncUtils.bookmarks.fetchGuidsWithAnno(
- "PlacesOrganizer/OrganizerQuery", "MobileBookmarks");
- equal(queryGuids.length, 0, "Should not create query without any mobile bookmarks");
-
- _("Insert mobile bookmark, then create query");
- let mozBmk = await PlacesUtils.bookmarks.insert({
- parentGuid: PlacesUtils.bookmarks.mobileGuid,
- url: "https://mozilla.org",
- });
- tracker._ensureMobileQuery();
- queryGuids = await PlacesSyncUtils.bookmarks.fetchGuidsWithAnno(
- "PlacesOrganizer/OrganizerQuery", "MobileBookmarks");
- equal(queryGuids.length, 1, "Should create query once mobile bookmarks exist");
-
- let queryGuid = queryGuids[0];
-
- let queryInfo = await PlacesUtils.bookmarks.fetch(queryGuid);
- equal(queryInfo.url, `place:folder=${PlacesUtils.mobileFolderId}`, "Query should point to mobile root");
- equal(queryInfo.title, "Mobile Bookmarks", "Query title should be localized");
- equal(queryInfo.parentGuid, allBookmarkGuid, "Should append mobile query to all bookmarks queries");
-
- _("Rename root and query, then recreate");
- await PlacesUtils.bookmarks.update({
- guid: PlacesUtils.bookmarks.mobileGuid,
- title: "renamed root",
- });
- await PlacesUtils.bookmarks.update({
- guid: queryGuid,
- title: "renamed query",
- });
- tracker._ensureMobileQuery();
- let rootInfo = await PlacesUtils.bookmarks.fetch(PlacesUtils.bookmarks.mobileGuid);
- equal(rootInfo.title, "Mobile Bookmarks", "Should fix root title");
- queryInfo = await PlacesUtils.bookmarks.fetch(queryGuid);
- equal(queryInfo.title, "Mobile Bookmarks", "Should fix query title");
-
- _("Point query to different folder");
- await PlacesUtils.bookmarks.update({
- guid: queryGuid,
- url: "place:folder=BOOKMARKS_MENU",
- });
- tracker._ensureMobileQuery();
- queryInfo = await PlacesUtils.bookmarks.fetch(queryGuid);
- equal(queryInfo.url.href, `place:folder=${PlacesUtils.mobileFolderId}`,
- "Should fix query URL to point to mobile root");
-
- _("We shouldn't track the query or the left pane root");
- await verifyTrackedItems([mozBmk.guid, "mobile"]);
- } finally {
- _("Clean up.");
- await cleanup();
- }
-});
-
add_task(async function test_skip_migration() {
await insertBookmarksToMigrate();
let originalTombstones = await PlacesTestUtils.fetchSyncTombstones();
let originalFields = await PlacesTestUtils.fetchBookmarkSyncFields(
"0gtWTOgYcoJD", "0dbpnMdxKxfg", "r5ouWdPB3l28", "YK5Bdq5MIqL6");
let filePath = OS.Path.join(OS.Constants.Path.profileDir, "weave", "changes",
--- a/toolkit/components/places/PlacesSyncUtils.jsm
+++ b/toolkit/components/places/PlacesSyncUtils.jsm
@@ -5,16 +5,17 @@
"use strict";
this.EXPORTED_SYMBOLS = ["PlacesSyncUtils"];
const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
Cu.importGlobalProperties(["URL", "URLSearchParams"]);
+Cu.import("resource://gre/modules/Services.jsm");
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "Log",
"resource://gre/modules/Log.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
"resource://gre/modules/PlacesUtils.jsm");
/**
@@ -24,16 +25,21 @@ XPCOMUtils.defineLazyModuleGetter(this,
* tags, keywords, synced annotations, and missing parents.
*/
var PlacesSyncUtils = {};
const { SOURCE_SYNC } = Ci.nsINavBookmarksService;
const MICROSECONDS_PER_SECOND = 1000000;
+const ORGANIZER_QUERY_ANNO = "PlacesOrganizer/OrganizerQuery";
+const ORGANIZER_ALL_BOOKMARKS_ANNO_VALUE = "AllBookmarks";
+const ORGANIZER_MOBILE_QUERY_ANNO_VALUE = "MobileBookmarks";
+const MOBILE_BOOKMARKS_PREF = "browser.bookmarks.showMobileBookmarks";
+
// These are defined as lazy getters to defer initializing the bookmarks
// service until it's needed.
XPCOMUtils.defineLazyGetter(this, "ROOT_SYNC_ID_TO_GUID", () => ({
menu: PlacesUtils.bookmarks.menuGuid,
places: PlacesUtils.bookmarks.rootGuid,
tags: PlacesUtils.bookmarks.tagsGuid,
toolbar: PlacesUtils.bookmarks.toolbarGuid,
unfiled: PlacesUtils.bookmarks.unfiledGuid,
@@ -831,16 +837,96 @@ const BookmarkSyncUtils = PlacesSyncUtil
const possible = [+existingMillis, +serverMillis].filter(n => !isNaN(n) && n > lowerBound);
if (!possible.length) {
return undefined;
}
return Math.min(...possible);
},
/**
+ * Rebuilds the left pane query for the mobile root under "All Bookmarks" if
+ * necessary. Sync calls this method at the end of each bookmark sync. This
+ * code should eventually move to `PlacesUIUtils#maybeRebuildLeftPane`; see
+ * bug 647605.
+ *
+ * - If there are no mobile bookmarks, the query will not be created, or
+ * will be removed if it already exists.
+ * - If there are mobile bookmarks, the query will be created if it doesn't
+ * exist, or will be updated with the correct title and URL otherwise.
+ */
+ async ensureMobileQuery() {
+ Services.prefs.setBoolPref(MOBILE_BOOKMARKS_PREF, true);
+
+ let db = await PlacesUtils.promiseDBConnection();
+
+ let maybeAllBookmarksGuids = await fetchGuidsWithAnno(db,
+ ORGANIZER_QUERY_ANNO, ORGANIZER_ALL_BOOKMARKS_ANNO_VALUE);
+ if (!maybeAllBookmarksGuids.length) {
+ return;
+ }
+
+ let hasMobileBookmarks = (await db.executeCached(`SELECT EXISTS(
+ SELECT 1 FROM moz_bookmarks b
+ JOIN moz_bookmarks p ON p.id = b.parent
+ WHERE p.guid = :mobileGuid
+ ) AS hasMobile`, {
+ mobileGuid: PlacesUtils.bookmarks.mobileGuid,
+ }))[0].getResultByName("hasMobile");
+
+ let allBookmarksGuid = maybeAllBookmarksGuids[0];
+ let mobileTitle = PlacesUtils.getString("MobileBookmarksFolderTitle");
+
+ let maybeMobileQueryGuids = await fetchGuidsWithAnno(db,
+ ORGANIZER_QUERY_ANNO, ORGANIZER_MOBILE_QUERY_ANNO_VALUE);
+ if (maybeMobileQueryGuids.length) {
+ let mobileQueryGuid = maybeMobileQueryGuids[0];
+ if (hasMobileBookmarks) {
+ // We have a left pane query for mobile bookmarks, and at least one
+ // mobile bookmark. Make sure the query title is correct.
+ await PlacesUtils.bookmarks.update({
+ guid: mobileQueryGuid,
+ url: "place:folder=MOBILE_BOOKMARKS",
+ title: mobileTitle,
+ source: SOURCE_SYNC,
+ });
+ } else {
+ // We have a left pane query for mobile bookmarks, but no mobile
+ // bookmarks. Remove the query.
+ await PlacesUtils.bookmarks.remove(mobileQueryGuid, {
+ source: SOURCE_SYNC,
+ });
+ }
+ } else if (hasMobileBookmarks) {
+ // We have mobile bookmarks, but no left pane query. Create the query.
+ let mobileQuery = await PlacesUtils.bookmarks.insert({
+ parentGuid: allBookmarksGuid,
+ url: "place:folder=MOBILE_BOOKMARKS",
+ title: mobileTitle,
+ source: SOURCE_SYNC,
+ });
+
+ let mobileQueryId = await PlacesUtils.promiseItemId(mobileQuery.guid);
+
+ PlacesUtils.annotations.setItemAnnotation(mobileQueryId,
+ ORGANIZER_QUERY_ANNO, ORGANIZER_MOBILE_QUERY_ANNO_VALUE, 0,
+ PlacesUtils.annotations.EXPIRE_NEVER, SOURCE_SYNC);
+ PlacesUtils.annotations.setItemAnnotation(mobileQueryId,
+ PlacesUtils.EXCLUDE_FROM_BACKUP_ANNO, 1, 0,
+ PlacesUtils.annotations.EXPIRE_NEVER, SOURCE_SYNC);
+ }
+
+ // Make sure the mobile root title matches the query.
+ await PlacesUtils.bookmarks.update({
+ guid: PlacesUtils.bookmarks.mobileGuid,
+ title: mobileTitle,
+ source: SOURCE_SYNC,
+ });
+ },
+
+ /**
* Fetches an array of GUIDs for items that have an annotation set with the
* given value.
*/
async fetchGuidsWithAnno(anno, val) {
let db = await PlacesUtils.promiseDBConnection();
return fetchGuidsWithAnno(db, anno, val);
},
});
--- a/toolkit/components/places/tests/unit/test_sync_utils.js
+++ b/toolkit/components/places/tests/unit/test_sync_utils.js
@@ -2663,8 +2663,86 @@ add_task(async function test_migrateOldT
deepEqual(tombstones, [{
guid: tombstoneSyncId,
dateRemoved: new Date(1479162463976),
}], "Should write tombstone for nonexistent migrated item");
await PlacesUtils.bookmarks.eraseEverything();
await PlacesSyncUtils.bookmarks.reset();
});
+
+add_task(async function test_ensureMobileQuery() {
+ do_print("Ensure we correctly create the mobile query");
+
+ const PlacesUIUtils = {};
+ try {
+ Cu.import("resource://gre/modules/PlacesUIUtils.jsm", PlacesUIUtils);
+ } catch (ex) {
+ do_print("Can't build left pane roots; skipping test");
+ return;
+ }
+
+ // Creates the organizer queries as a side effect.
+ let leftPaneId = PlacesUIUtils.leftPaneFolderId;
+ do_print(`Left pane root ID: ${leftPaneId}`);
+
+ let allBookmarksGuids = await PlacesSyncUtils.bookmarks.fetchGuidsWithAnno(
+ "PlacesOrganizer/OrganizerQuery", "AllBookmarks");
+ equal(allBookmarksGuids.length, 1, "Should create folder with all bookmarks queries");
+ let allBookmarkGuid = allBookmarksGuids[0];
+
+ do_print("Try creating query after organizer is ready");
+ await PlacesSyncUtils.bookmarks.ensureMobileQuery()
+ let queryGuids = await PlacesSyncUtils.bookmarks.fetchGuidsWithAnno(
+ "PlacesOrganizer/OrganizerQuery", "MobileBookmarks");
+ equal(queryGuids.length, 0, "Should not create query without any mobile bookmarks");
+
+ do_print("Insert mobile bookmark, then create query");
+ let mozBmk = await PlacesUtils.bookmarks.insert({
+ parentGuid: PlacesUtils.bookmarks.mobileGuid,
+ url: "https://mozilla.org",
+ });
+ await PlacesSyncUtils.bookmarks.ensureMobileQuery()
+ queryGuids = await PlacesSyncUtils.bookmarks.fetchGuidsWithAnno(
+ "PlacesOrganizer/OrganizerQuery", "MobileBookmarks");
+ equal(queryGuids.length, 1, "Should create query once mobile bookmarks exist");
+
+ let queryGuid = queryGuids[0];
+
+ let queryInfo = await PlacesUtils.bookmarks.fetch(queryGuid);
+ equal(queryInfo.url, `place:folder=MOBILE_BOOKMARKS`, "Query should point to mobile root");
+ equal(queryInfo.title, "Mobile Bookmarks", "Query title should be localized");
+ equal(queryInfo.parentGuid, allBookmarkGuid, "Should append mobile query to all bookmarks queries");
+
+ do_print("Rename root and query, then recreate");
+ await PlacesUtils.bookmarks.update({
+ guid: PlacesUtils.bookmarks.mobileGuid,
+ title: "renamed root",
+ });
+ await PlacesUtils.bookmarks.update({
+ guid: queryGuid,
+ title: "renamed query",
+ });
+ await PlacesSyncUtils.bookmarks.ensureMobileQuery()
+ let rootInfo = await PlacesUtils.bookmarks.fetch(PlacesUtils.bookmarks.mobileGuid);
+ equal(rootInfo.title, "Mobile Bookmarks", "Should fix root title");
+ queryInfo = await PlacesUtils.bookmarks.fetch(queryGuid);
+ equal(queryInfo.title, "Mobile Bookmarks", "Should fix query title");
+
+ do_print("Point query to different folder");
+ await PlacesUtils.bookmarks.update({
+ guid: queryGuid,
+ url: "place:folder=BOOKMARKS_MENU",
+ });
+ await PlacesSyncUtils.bookmarks.ensureMobileQuery()
+ queryInfo = await PlacesUtils.bookmarks.fetch(queryGuid);
+ equal(queryInfo.url.href, `place:folder=MOBILE_BOOKMARKS`,
+ "Should fix query URL to point to mobile root");
+
+ do_print("We shouldn't track the query or the left pane root");
+
+ let changes = await PlacesSyncUtils.bookmarks.pullChanges();
+ deepEqual(Object.keys(changes).sort(), [mozBmk.guid, "mobile"],
+ "Should not track mobile query");
+
+ await PlacesUtils.bookmarks.eraseEverything();
+ await PlacesSyncUtils.bookmarks.reset();
+});