Bug 1331800 - catch errors from history removals and don't block undo on them, r=Dolske
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 18 Jan 2017 18:07:47 +0000
changeset 330033 7400d6afc2d2190e9276647a944f35008dfdf267
parent 330032 f5fdf8bb9729298c05cea82e9cebd90ed9d231ec
child 330034 2b174f46eb04be4bb428dbf21c82e03efdb5bb91
push id31229
push usercbook@mozilla.com
push dateThu, 19 Jan 2017 15:01:46 +0000
treeherdermozilla-central@168ea3e9ff0b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersDolske
bugs1331800
milestone53.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 1331800 - catch errors from history removals and don't block undo on them, r=Dolske MozReview-Commit-ID: JhWAs6rvBnW
browser/components/migration/AutoMigrate.jsm
browser/components/migration/tests/unit/test_automigration.js
--- a/browser/components/migration/AutoMigrate.jsm
+++ b/browser/components/migration/AutoMigrate.jsm
@@ -458,36 +458,50 @@ const AutoMigrate = {
 
   _removeUnchangedLogins: Task.async(function* (logins) {
     for (let login of logins) {
       let foundLogins = LoginHelper.searchLoginsWithObject({guid: login.guid});
       if (foundLogins.length) {
         let foundLogin = foundLogins[0];
         foundLogin.QueryInterface(Ci.nsILoginMetaInfo);
         if (foundLogin.timePasswordChanged == login.timePasswordChanged) {
-          Services.logins.removeLogin(foundLogin);
+          try {
+            Services.logins.removeLogin(foundLogin);
+          } catch (ex) {
+            Cu.reportError("Failed to remove a login for " + foundLogins.hostname);
+            Cu.reportError(ex);
+          }
         }
       }
     }
   }),
 
   _removeSomeVisits: Task.async(function* (visits) {
     for (let urlVisits of visits) {
       let urlObj;
       try {
         urlObj = new URL(urlVisits.url);
       } catch (ex) {
         continue;
       }
-      yield PlacesUtils.history.removeVisitsByFilter({
+      let visitData = {
         url: urlObj,
         beginDate: PlacesUtils.toDate(urlVisits.first),
         endDate: PlacesUtils.toDate(urlVisits.last),
         limit: urlVisits.visitCount,
-      });
+      };
+      try {
+        yield PlacesUtils.history.removeVisitsByFilter(visitData);
+      } catch(ex) {
+        try {
+          visitData.url = visitData.url.href;
+        } catch (ex) {}
+        Cu.reportError("Failed to remove a visit: " + JSON.stringify(visitData));
+        Cu.reportError(ex);
+      }
     }
   }),
 
   QueryInterface: XPCOMUtils.generateQI(
     [Ci.nsIObserver, Ci.nsINavBookmarkObserver, Ci.nsISupportsWeakReference]
   ),
 };
 
--- a/browser/components/migration/tests/unit/test_automigration.js
+++ b/browser/components/migration/tests/unit/test_automigration.js
@@ -605,8 +605,13 @@ add_task(function* checkUndoVisitsState(
   Assert.equal(yield visitsForURL("http://www.mozilla.org/"), 0,
                "0 mozilla.org visits should have persisted (out of 1).");
   Assert.equal(yield visitsForURL("http://www.example.org/"), 2,
                "2 example.org visits should have persisted (out of 4).");
   Assert.equal(yield visitsForURL("http://www.unrelated.org/"), 1,
                "1 unrelated.org visits should have persisted as it's not involved in the import.");
   yield PlacesTestUtils.clearHistory();
 });
+
+add_task(function* checkHistoryRemovalCompletion() {
+  yield AutoMigrate._removeSomeVisits([{url: "http://www.example.com/", limit: -1}]);
+  ok(true, "Removing visits should complete even if removing some visits failed.");
+});