Bug 1331800 - catch errors from history removals and don't block undo on them, r?dolske draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 18 Jan 2017 18:07:47 +0000
changeset 463223 02219252c1f00499c3373e41c61b78d967195e95
parent 462769 80eac484366ad881c6a10bf81e8d9b8f7a676c75
child 542610 4b9b3c5316ae564753b095ae6095200af76e885e
push id41992
push userbmo:gijskruitbosch+bugs@gmail.com
push dateWed, 18 Jan 2017 18:08:27 +0000
reviewersdolske
bugs1331800
milestone53.0a1
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
@@ -445,36 +445,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.");
+});