Bug 1349630 - Optimize `PlacesSyncUtils.bookmarks.havePendingChanges`. r=tcsc draft
authorKit Cambridge <kit@yakshaving.ninja>
Wed, 22 Mar 2017 11:42:44 -0700
changeset 503412 0c2e75b11de955c7b74bcd1a7c24022b32129236
parent 502997 e03e0c60462c775c7558a1dc9d5cf2076c3cd1f9
child 550418 baab8c0fa6ec1aead898be79d3f9fddd62d928d6
push id50568
push userbmo:kit@mozilla.com
push dateThu, 23 Mar 2017 06:03:26 +0000
reviewerstcsc
bugs1349630
milestone55.0a1
Bug 1349630 - Optimize `PlacesSyncUtils.bookmarks.havePendingChanges`. r=tcsc MozReview-Commit-ID: Iw8Q2ZB7mvL
toolkit/components/places/PlacesSyncUtils.jsm
--- a/toolkit/components/places/PlacesSyncUtils.jsm
+++ b/toolkit/components/places/PlacesSyncUtils.jsm
@@ -296,22 +296,39 @@ const BookmarkSyncUtils = PlacesSyncUtil
     let orderedChildrenGuids = childSyncIds.map(BookmarkSyncUtils.syncIdToGuid);
     return PlacesUtils.bookmarks.reorder(parentGuid, orderedChildrenGuids,
                                          { source: SOURCE_SYNC });
   }),
 
   /**
    * Resolves to true if there are known sync changes.
    */
-  havePendingChanges() {
-    // This could be optimized to use a more efficient query -- We don't need
-    // grab all the records if all we care about is whether or not any exist.
-    return PlacesUtils.withConnectionWrapper("BookmarkSyncUtils: havePendingChanges",
-      db => pullSyncChanges(db, true).then(changes => Object.keys(changes).length > 0));
-  },
+  havePendingChanges: Task.async(function* () {
+    let db = yield PlacesUtils.promiseDBConnection();
+    let rows = yield db.executeCached(`
+      WITH RECURSIVE
+      syncedItems(id, guid, syncChangeCounter) AS (
+        SELECT b.id, b.guid, b.syncChangeCounter
+         FROM moz_bookmarks b
+         WHERE b.guid IN ('menu________', 'toolbar_____', 'unfiled_____',
+                          'mobile______')
+        UNION ALL
+        SELECT b.id, b.guid, b.syncChangeCounter
+        FROM moz_bookmarks b
+        JOIN syncedItems s ON b.parent = s.id
+      ),
+      changedItems(guid) AS (
+        SELECT guid FROM syncedItems
+        WHERE syncChangeCounter >= 1
+        UNION ALL
+        SELECT guid FROM moz_bookmarks_deleted
+      )
+      SELECT EXISTS(SELECT guid FROM changedItems) AS haveChanges`);
+    return !!rows[0].getResultByName("haveChanges");
+  }),
 
   /**
    * Returns a changeset containing local bookmark changes since the last sync.
    * Updates the sync status of all "NEW" bookmarks to "NORMAL", so that Sync
    * can recover correctly after an interrupted sync.
    *
    * @return {Promise} resolved once all items have been fetched.
    * @resolves to an object containing records for changed bookmarks, keyed by
@@ -1692,23 +1709,21 @@ function addRowToChangeRecords(row, chan
 
 /**
  * Queries the database for synced bookmarks and tombstones, updates the sync
  * status of all "NEW" bookmarks to "NORMAL", and returns a changeset for the
  * Sync bookmarks engine.
  *
  * @param db
  *        The Sqlite.jsm connection handle.
- * @param preventUpdate {boolean}
- *        Should we skip updating the records we pull.
  * @return {Promise} resolved once all items have been fetched.
  * @resolves to an object containing records for changed bookmarks, keyed by
  *           the sync ID.
  */
-var pullSyncChanges = Task.async(function* (db, preventUpdate = false) {
+var pullSyncChanges = Task.async(function* (db) {
   let changeRecords = {};
 
   yield db.executeCached(`
     WITH RECURSIVE
     syncedItems(id, guid, modified, syncChangeCounter, syncStatus) AS (
       SELECT b.id, b.guid, b.lastModified, b.syncChangeCounter, b.syncStatus
        FROM moz_bookmarks b
        WHERE b.guid IN ('menu________', 'toolbar_____', 'unfiled_____',
@@ -1723,19 +1738,17 @@ var pullSyncChanges = Task.async(functio
     WHERE syncChangeCounter >= 1
     UNION ALL
     SELECT guid, dateRemoved AS modified, 1 AS syncChangeCounter,
            :deletedSyncStatus, 1 AS tombstone
     FROM moz_bookmarks_deleted`,
     { deletedSyncStatus: PlacesUtils.bookmarks.SYNC_STATUS.NORMAL },
     row => addRowToChangeRecords(row, changeRecords));
 
-  if (!preventUpdate) {
-    yield markChangesAsSyncing(db, changeRecords);
-  }
+  yield markChangesAsSyncing(db, changeRecords);
 
   return changeRecords;
 });
 
 var touchSyncBookmark = Task.async(function* (db, bookmarkItem) {
   if (BookmarkSyncLog.level <= Log.Level.Trace) {
     BookmarkSyncLog.trace(
       `touch: Reviving item "${bookmarkItem.guid}" and marking parent ` +