Bug 1380835 - Don't check queries with `excludeQueries` for cycles in the bookmark validator. r=tcsc
authorKit Cambridge <kit@yakshaving.ninja>
Thu, 13 Jul 2017 19:52:57 -0700
changeset 368938 e9158194b48392803406de780d030fcd661d86f4
parent 368937 da8b5cbdecef07775d57150e9b6ac0edc3d3abea
child 368941 85c4d1a6ad0922bbc65dc6778e686da968728f2e
push id46493
push userkcambridge@mozilla.com
push dateFri, 14 Jul 2017 16:24:27 +0000
treeherderautoland@e9158194b483 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcsc
bugs1380835
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1380835 - Don't check queries with `excludeQueries` for cycles in the bookmark validator. r=tcsc MozReview-Commit-ID: DsPzrCxWWMG
services/sync/modules/bookmark_validator.js
services/sync/tests/unit/test_bookmark_validator.js
--- a/services/sync/modules/bookmark_validator.js
+++ b/services/sync/modules/bookmark_validator.js
@@ -249,16 +249,23 @@ class BookmarkValidator {
   }
 
   _followQueries(recordsByQueryId) {
     for (let entry of recordsByQueryId.values()) {
       if (entry.type !== "query" && (!entry.bmkUri || !entry.bmkUri.startsWith(QUERY_PROTOCOL))) {
         continue;
       }
       let params = new URLSearchParams(entry.bmkUri.slice(QUERY_PROTOCOL.length));
+      // Queries with `excludeQueries` won't form cycles because they'll
+      // exclude all queries, including themselves, from the result set.
+      let excludeQueries = params.get("excludeQueries");
+      if (excludeQueries === "1" || excludeQueries === "true") {
+        // `nsNavHistoryQuery::ParseQueryBooleanString` allows `1` and `true`.
+        continue;
+      }
       entry.concreteItems = [];
       let queryIds = params.getAll("folder");
       for (let queryId of queryIds) {
         let concreteItem = recordsByQueryId.get(queryId);
         if (concreteItem) {
           entry.concreteItems.push(concreteItem);
         }
       }
--- a/services/sync/tests/unit/test_bookmark_validator.js
+++ b/services/sync/tests/unit/test_bookmark_validator.js
@@ -337,16 +337,24 @@ add_task(async function test_cswc_client
   await PlacesUtils.bookmarks.insertTree({
     guid: PlacesUtils.bookmarks.menuGuid,
     children: [{
       // A query for the menu, referenced by its local ID instead of
       // `BOOKMARKS_MENU`. This should be reported as a cycle.
       guid: "dddddddddddd",
       url: `place:folder=${PlacesUtils.bookmarksMenuFolderId}`,
       title: "Bookmarks Menu",
+    }, {
+      // A query that references the menu, but excludes itself, so it can't
+      // form a cycle.
+      guid: "iiiiiiiiiiii",
+      url: `place:folder=BOOKMARKS_MENU&folder=UNFILED_BOOKMARKS&` +
+           `folder=TOOLBAR&queryType=1&sort=12&maxResults=10&` +
+           `excludeQueries=1`,
+      title: "Recently Bookmarked",
     }],
   });
 
   await PlacesUtils.bookmarks.insertTree({
     guid: PlacesUtils.bookmarks.toolbarGuid,
     children: [{
       guid: "eeeeeeeeeeee",
       type: PlacesUtils.bookmarks.TYPE_FOLDER,