Bug 1450789 - Use more accurate names for structure conflict event telemetry keys. r?tcsc
MozReview-Commit-ID: FCLy34YU3qK
--- 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");