зеркало из https://github.com/mozilla/gecko-dev.git
Bug 1333233 - part 1: add telemetry for error counts from undo operations, r=bsmedberg,Dolske
MozReview-Commit-ID: EdelbiibVWi --HG-- extra : rebase_source : 21d376c84c66e7dc6ff3ae05f1c0a8826ad68d82
This commit is contained in:
Gijs Kruitbosch
Родитель
391b744644
Коммит
569e10b1e0
|
|
@ -210,13 +210,33 @@ const AutoMigrate = {
|
|||
compression: "lz4",
|
||||
});
|
||||
let stateData = this._dejsonifyUndoState(yield readPromise);
|
||||
yield this._removeUnchangedBookmarks(stateData.get("bookmarks"));
|
||||
histogram.add(12);
|
||||
|
||||
this._errorMap = {bookmarks: 0, visits: 0, logins: 0};
|
||||
let browserId = Preferences.get(kAutoMigrateBrowserPref, "unknown");
|
||||
let reportErrorTelemetry = (type) => {
|
||||
let histogramId = `FX_STARTUP_MIGRATION_UNDO_${type.toUpperCase()}_ERRORCOUNT`;
|
||||
Services.telemetry.getKeyedHistogramById(histogramId).add(browserId, this._errorMap[type]);
|
||||
};
|
||||
yield this._removeUnchangedBookmarks(stateData.get("bookmarks")).catch(ex => {
|
||||
Cu.reportError("Uncaught exception when removing unchanged bookmarks!");
|
||||
Cu.reportError(ex);
|
||||
});
|
||||
reportErrorTelemetry("bookmarks");
|
||||
histogram.add(15);
|
||||
|
||||
yield this._removeSomeVisits(stateData.get("visits"));
|
||||
yield this._removeSomeVisits(stateData.get("visits")).catch(ex => {
|
||||
Cu.reportError("Uncaught exception when removing history visits!");
|
||||
Cu.reportError(ex);
|
||||
});
|
||||
reportErrorTelemetry("visits");
|
||||
histogram.add(20);
|
||||
|
||||
yield this._removeUnchangedLogins(stateData.get("logins"));
|
||||
yield this._removeUnchangedLogins(stateData.get("logins")).catch(ex => {
|
||||
Cu.reportError("Uncaught exception when removing unchanged logins!");
|
||||
Cu.reportError(ex);
|
||||
});
|
||||
reportErrorTelemetry("logins");
|
||||
histogram.add(25);
|
||||
|
||||
// This is async, but no need to wait for it.
|
||||
|
|
@ -419,7 +439,12 @@ const AutoMigrate = {
|
|||
let bmPromises = Array.from(guidToLMMap.keys()).map(guid => {
|
||||
// Ignore bookmarks where the promise doesn't resolve (ie that are missing)
|
||||
// Also check that the bookmark fetch returns isn't null before adding it.
|
||||
return PlacesUtils.bookmarks.fetch(guid).then(bm => bm && bookmarksFromDB.push(bm), () => {});
|
||||
try {
|
||||
return PlacesUtils.bookmarks.fetch(guid).then(bm => bm && bookmarksFromDB.push(bm), () => {});
|
||||
} catch (ex) {
|
||||
// Ignore immediate exceptions, too.
|
||||
}
|
||||
return Promise.resolve();
|
||||
});
|
||||
// We can't use the result of Promise.all because that would include nulls
|
||||
// for bookmarks that no longer exist (which we're catching above).
|
||||
|
|
@ -428,7 +453,7 @@ const AutoMigrate = {
|
|||
return bm.lastModified.getTime() == guidToLMMap.get(bm.guid).getTime();
|
||||
});
|
||||
|
||||
// We need to remove items with no ancestors first, followed by their
|
||||
// We need to remove items without children first, followed by their
|
||||
// parents, etc. In order to do this, find out how many ancestors each item
|
||||
// has that also appear in our list of things to remove, and sort the items
|
||||
// by those numbers. This ensures that children are always removed before
|
||||
|
|
@ -448,11 +473,16 @@ const AutoMigrate = {
|
|||
unchangedBookmarks.forEach(determineAncestorCount);
|
||||
unchangedBookmarks.sort((a, b) => b._ancestorCount - a._ancestorCount);
|
||||
for (let {guid} of unchangedBookmarks) {
|
||||
yield PlacesUtils.bookmarks.remove(guid, {preventRemovalOfNonEmptyFolders: true}).catch(err => {
|
||||
// Can't just use a .catch() because Bookmarks.remove() can throw (rather
|
||||
// than returning rejected promises).
|
||||
try {
|
||||
yield PlacesUtils.bookmarks.remove(guid, {preventRemovalOfNonEmptyFolders: true});
|
||||
} catch (err) {
|
||||
if (err && err.message != "Cannot remove a non-empty folder.") {
|
||||
this._errorMap.bookmarks++;
|
||||
Cu.reportError(err);
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
}),
|
||||
|
||||
|
|
@ -468,6 +498,7 @@ const AutoMigrate = {
|
|||
} catch (ex) {
|
||||
Cu.reportError("Failed to remove a login for " + foundLogins.hostname);
|
||||
Cu.reportError(ex);
|
||||
this._errorMap.logins++;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -490,10 +521,11 @@ const AutoMigrate = {
|
|||
};
|
||||
try {
|
||||
yield PlacesUtils.history.removeVisitsByFilter(visitData);
|
||||
} catch(ex) {
|
||||
} catch (ex) {
|
||||
this._errorMap.visits++;
|
||||
try {
|
||||
visitData.url = visitData.url.href;
|
||||
} catch (ex) {}
|
||||
} catch (ignoredEx) {}
|
||||
Cu.reportError("Failed to remove a visit: " + JSON.stringify(visitData));
|
||||
Cu.reportError(ex);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -188,6 +188,7 @@ add_task(function* checkUndoPreconditions() {
|
|||
*/
|
||||
add_task(function* checkUndoRemoval() {
|
||||
MigrationUtils.initializeUndoData();
|
||||
Preferences.set("browser.migrate.automigrate.browser", "automationbrowser");
|
||||
// Insert a login and check that that worked.
|
||||
MigrationUtils.insertLoginWrapper({
|
||||
hostname: "www.mozilla.org",
|
||||
|
|
@ -257,6 +258,18 @@ add_task(function* checkUndoRemoval() {
|
|||
Assert.ok(AutoMigrate.canUndo(), "Should be possible to undo migration");
|
||||
yield AutoMigrate.undo();
|
||||
|
||||
let histograms = [
|
||||
"FX_STARTUP_MIGRATION_UNDO_BOOKMARKS_ERRORCOUNT",
|
||||
"FX_STARTUP_MIGRATION_UNDO_LOGINS_ERRORCOUNT",
|
||||
"FX_STARTUP_MIGRATION_UNDO_VISITS_ERRORCOUNT",
|
||||
];
|
||||
for (let histogramId of histograms) {
|
||||
let keyedHistogram = Services.telemetry.getKeyedHistogramById(histogramId);
|
||||
let histogramData = keyedHistogram.snapshot().automationbrowser;
|
||||
Assert.equal(histogramData.sum, 0, `Should have reported 0 errors to ${histogramId}.`);
|
||||
Assert.greaterOrEqual(histogramData.counts[0], 1, `Should have reported value of 0 one time to ${histogramId}.`);
|
||||
}
|
||||
|
||||
// Check that the undo removed the history visits:
|
||||
visits = PlacesUtils.history.executeQuery(query, opts);
|
||||
visits.root.containerOpen = true;
|
||||
|
|
@ -612,6 +625,27 @@ add_task(function* checkUndoVisitsState() {
|
|||
});
|
||||
|
||||
add_task(function* checkHistoryRemovalCompletion() {
|
||||
AutoMigrate._errorMap = {bookmarks: 0, visits: 0, logins: 0};
|
||||
yield AutoMigrate._removeSomeVisits([{url: "http://www.example.com/", limit: -1}]);
|
||||
ok(true, "Removing visits should complete even if removing some visits failed.");
|
||||
Assert.equal(AutoMigrate._errorMap.visits, 1, "Should have logged the error for visits.");
|
||||
|
||||
// Unfortunately there's not a reliable way to make removing bookmarks be
|
||||
// unhappy unless the DB is messed up (e.g. contains children but has
|
||||
// parents removed already).
|
||||
yield AutoMigrate._removeUnchangedBookmarks([
|
||||
{guid: PlacesUtils.bookmarks, lastModified: new Date(0), parentGuid: 0},
|
||||
{guid: "gobbledygook", lastModified: new Date(0), parentGuid: 0},
|
||||
]);
|
||||
ok(true, "Removing bookmarks should complete even if some items are gone or bogus.");
|
||||
Assert.equal(AutoMigrate._errorMap.bookmarks, 0,
|
||||
"Should have ignored removing non-existing (or builtin) bookmark.");
|
||||
|
||||
|
||||
yield AutoMigrate._removeUnchangedLogins([
|
||||
{guid: "gobbledygook", timePasswordChanged: new Date(0)},
|
||||
]);
|
||||
ok(true, "Removing logins should complete even if logins don't exist.");
|
||||
Assert.equal(AutoMigrate._errorMap.logins, 0,
|
||||
"Should have ignored removing non-existing logins.");
|
||||
});
|
||||
|
|
|
|||
|
|
@ -5173,6 +5173,39 @@
|
|||
"releaseChannelCollection": "opt-out",
|
||||
"description": "Indicates we showed a 'would you like to undo this automatic migration?' notification bar. The bucket indicates which nth day we're on (1st/2nd/3rd, by default - 0 would be indicative the pref didn't get set which shouldn't happen). After 3 days on which the notification gets shown, it will get disabled and never shown again."
|
||||
},
|
||||
"FX_STARTUP_MIGRATION_UNDO_BOOKMARKS_ERRORCOUNT": {
|
||||
"bug_numbers": [1333233],
|
||||
"alert_emails": ["gijs@mozilla.com"],
|
||||
"expires_in_version": "58",
|
||||
"keyed": true,
|
||||
"kind": "exponential",
|
||||
"n_buckets": 20,
|
||||
"high": 100,
|
||||
"releaseChannelCollection": "opt-out",
|
||||
"description": "Indicates how many errors we find when trying to 'undo' bookmarks import. Keys are internal ids of browsers we import from, e.g. 'chrome' or 'ie', etc."
|
||||
},
|
||||
"FX_STARTUP_MIGRATION_UNDO_LOGINS_ERRORCOUNT": {
|
||||
"bug_numbers": [1333233],
|
||||
"alert_emails": ["gijs@mozilla.com"],
|
||||
"expires_in_version": "58",
|
||||
"keyed": true,
|
||||
"kind": "exponential",
|
||||
"n_buckets": 20,
|
||||
"high": 100,
|
||||
"releaseChannelCollection": "opt-out",
|
||||
"description": "Indicates how many errors we find when trying to 'undo' login (password) import. Keys are internal ids of browsers we import from, e.g. 'chrome' or 'ie', etc."
|
||||
},
|
||||
"FX_STARTUP_MIGRATION_UNDO_VISITS_ERRORCOUNT": {
|
||||
"bug_numbers": [1333233],
|
||||
"alert_emails": ["gijs@mozilla.com"],
|
||||
"expires_in_version": "58",
|
||||
"keyed": true,
|
||||
"kind": "exponential",
|
||||
"n_buckets": 20,
|
||||
"high": 100,
|
||||
"releaseChannelCollection": "opt-out",
|
||||
"description": "Indicates how many errors we find when trying to 'undo' history import. Keys are internal ids of browsers we import from, e.g. 'chrome' or 'ie', etc."
|
||||
},
|
||||
"FX_STARTUP_MIGRATION_DATA_RECENCY": {
|
||||
"bug_numbers": [1276694],
|
||||
"alert_emails": ["gijs@mozilla.com"],
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче