Bug 1450789 - Use more accurate names for structure conflict event telemetry keys. r=tcsc
authorKit Cambridge <kit@yakshaving.ninja>
Mon, 02 Apr 2018 14:41:16 -0700
changeset 411402 720ffa73f567809363331c3331bf82b2f1af3c68
parent 411401 70400746eb530da029289760b9bab836558f9824
child 411403 d75d996016dcf325c2db2ed8a47af512d07ffacd
child 411413 d2734042605a92b6822e566a277ab430f1f7ada6
push id101643
push userdluca@mozilla.com
push dateTue, 03 Apr 2018 04:25:27 +0000
treeherdermozilla-inbound@ac667545d8aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcsc
bugs1450789
milestone61.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 1450789 - Use more accurate names for structure conflict event telemetry keys. r=tcsc MozReview-Commit-ID: FCLy34YU3qK
toolkit/components/places/SyncedBookmarksMirror.jsm
toolkit/components/places/tests/sync/test_bookmark_deletion.js
--- a/toolkit/components/places/SyncedBookmarksMirror.jsm
+++ b/toolkit/components/places/SyncedBookmarksMirror.jsm
@@ -3402,20 +3402,20 @@ class BookmarkMerger {
     this.remoteTree = remoteTree;
     this.newRemoteContents = newRemoteContents;
     this.matchingDupesByLocalParentNode = new Map();
     this.mergedGuids = new Set();
     this.deleteLocally = new Set();
     this.deleteRemotely = new Set();
     this.structureCounts = {
       new: 0,
-      remoteItemDel: 0,
-      localFolderDel: 0,
-      localItemDel: 0,
-      remoteFolderDel: 0,
+      remoteRevives: 0, // Remote non-folder change wins over local deletion.
+      localDeletes: 0, // Local folder deletion wins over remote change.
+      localRevives: 0, // Local non-folder change wins over remote deletion.
+      remoteDeletes: 0, // Remote folder deletion wins over local change.
     };
     this.dupeCount = 0;
     this.extraTelemetryEvents = [];
   }
 
   summarizeTelemetryEvents() {
     let events = [...this.extraTelemetryEvents];
     if (this.dupeCount > 0) {
@@ -4103,27 +4103,27 @@ class BookmarkMerger {
 
     if (remoteNode.needsMerge) {
       if (!remoteNode.isFolder()) {
         // If a non-folder child is deleted locally and changed remotely, we
         // ignore the local deletion and take the remote child.
         MirrorLog.trace("Remote non-folder ${remoteNode} deleted locally " +
                         "and changed remotely; taking remote change",
                         { remoteNode });
-        this.structureCounts.remoteItemDel++;
+        this.structureCounts.remoteRevives++;
         return BookmarkMerger.STRUCTURE.UNCHANGED;
       }
       // For folders, we always take the local deletion and relocate remotely
       // changed grandchildren to the merged node. We could use the mirror to
       // revive the child folder, but it's easier to relocate orphaned
       // grandchildren than to partially revive the child folder.
       MirrorLog.trace("Remote folder ${remoteNode} deleted locally " +
                       "and changed remotely; taking local deletion",
                       { remoteNode });
-      this.structureCounts.localFolderDel++;
+      this.structureCounts.localDeletes++;
     } else {
       MirrorLog.trace("Remote node ${remoteNode} deleted locally and not " +
                        "changed remotely; taking local deletion",
                        { remoteNode });
     }
 
     this.deleteRemotely.add(remoteNode.guid);
 
@@ -4172,22 +4172,22 @@ class BookmarkMerger {
       }
       return BookmarkMerger.STRUCTURE.UNCHANGED;
     }
 
     if (localNode.needsMerge) {
       if (!localNode.isFolder()) {
         MirrorLog.trace("Local non-folder ${localNode} deleted remotely and " +
                         "changed locally; taking local change", { localNode });
-        this.structureCounts.localItemDel++;
+        this.structureCounts.localRevives++;
         return BookmarkMerger.STRUCTURE.UNCHANGED;
       }
       MirrorLog.trace("Local folder ${localNode} deleted remotely and " +
                       "changed locally; taking remote deletion", { localNode });
-      this.structureCounts.remoteFolderDel++;
+      this.structureCounts.remoteDeletes++;
     } else {
       MirrorLog.trace("Local node ${localNode} deleted remotely and not " +
                       "changed locally; taking remote deletion", { localNode });
     }
 
     MirrorLog.trace("Local node ${localNode} deleted remotely; taking remote " +
                     "deletion", { localNode });
 
--- a/toolkit/components/places/tests/sync/test_bookmark_deletion.js
+++ b/toolkit/components/places/tests/sync/test_bookmark_deletion.js
@@ -109,17 +109,17 @@ add_task(async function test_complex_orp
     bmkUri: "http://example.com/f",
   }]));
 
   info("Apply remote");
   let changesToUpload = await buf.apply();
   deepEqual(await buf.fetchUnmergedGuids(), [], "Should merge all items");
   deepEqual(mergeTelemetryEvents, [{
     value: "structure",
-    extra: { new: "2", localFolderDel: "1", remoteFolderDel: "1" },
+    extra: { new: "2", localDeletes: "1", remoteDeletes: "1" },
   }], "Should record telemetry with structure change counts");
 
   let idsToUpload = inspectChangeRecords(changesToUpload);
   deepEqual(idsToUpload, {
     updated: ["bookmarkEEEE", "bookmarkFFFF", "folderAAAAAA", "folderCCCCCC"],
     deleted: ["folderDDDDDD"],
   }, "Should upload new records for (A > E), (C > F); tombstone for D");
 
@@ -301,17 +301,17 @@ add_task(async function test_locally_mod
     deleted: true,
   }]);
 
   info("Apply remote");
   let changesToUpload = await buf.apply();
   deepEqual(await buf.fetchUnmergedGuids(), [], "Should merge all items");
   deepEqual(mergeTelemetryEvents, [{
     value: "structure",
-    extra: { new: "1", localItemDel: "1", remoteFolderDel: "2" },
+    extra: { new: "1", localRevives: "1", remoteDeletes: "2" },
   }], "Should record telemetry for local item and remote folder deletions");
 
   let idsToUpload = inspectChangeRecords(changesToUpload);
   deepEqual(idsToUpload, {
     updated: ["bookmarkAAAA", "bookmarkFFFF", "bookmarkGGGG", "menu"],
     deleted: [],
   }, "Should upload A, relocated local orphans, and menu");
 
@@ -448,17 +448,17 @@ add_task(async function test_locally_del
     bmkUri: "http://example.com/g-remote",
   }]);
 
   info("Apply remote");
   let changesToUpload = await buf.apply();
   deepEqual(await buf.fetchUnmergedGuids(), [], "Should merge all items");
   deepEqual(mergeTelemetryEvents, [{
     value: "structure",
-    extra: { new: "1", remoteItemDel: "1", localFolderDel: "2" },
+    extra: { new: "1", remoteRevives: "1", localDeletes: "2" },
   }], "Should record telemetry for remote item and local folder deletions");
 
   let idsToUpload = inspectChangeRecords(changesToUpload);
   deepEqual(idsToUpload, {
     updated: ["bookmarkFFFF", "bookmarkGGGG", "menu"],
     deleted: ["bookmarkCCCC", "bookmarkEEEE", "folderBBBBBB", "folderDDDDDD"],
   }, "Should upload relocated remote orphans and menu");