Bug 1349632 - fix hangs when trying to import bookmarks from Edge, r=dao a=gchang FENNEC_53_0b7_BUILD1 FENNEC_53_0b7_RELEASE FIREFOX_53_0b7_BUILD1 FIREFOX_53_0b7_RELEASE
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Sat, 25 Mar 2017 09:49:26 +0000
changeset 376999 46d5fe92c82d36b922703faeb7dd3a17b5df55e1
parent 376998 1d65f4570b053341251db1477486c826a88f5769
child 377000 8016d994c82a9b29a7865020fc52b7521b376b96
push id7108
push usergijskruitbosch@gmail.com
push dateMon, 27 Mar 2017 15:13:42 +0000
treeherdermozilla-beta@46d5fe92c82d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdao, gchang
bugs1349632
milestone53.0
Bug 1349632 - fix hangs when trying to import bookmarks from Edge, r=dao a=gchang MozReview-Commit-ID: 1xF16KddJBv
browser/components/migration/ESEDBReader.jsm
browser/components/migration/EdgeProfileMigrator.js
--- a/browser/components/migration/ESEDBReader.jsm
+++ b/browser/components/migration/ESEDBReader.jsm
@@ -15,16 +15,18 @@ XPCOMUtils.defineLazyGetter(this, "log",
   let ConsoleAPI = Cu.import("resource://gre/modules/Console.jsm", {}).ConsoleAPI;
   let consoleOptions = {
     maxLogLevelPref: "browser.esedbreader.loglevel",
     prefix: "ESEDBReader",
   };
   return new ConsoleAPI(consoleOptions);
 });
 
+XPCOMUtils.defineLazyModuleGetter(this, "OS", "resource://gre/modules/osfile.jsm");
+
 // We have a globally unique identifier for ESE instances. A new one
 // is used for each different database opened.
 let gESEInstanceCounter = 0;
 
 // We limit the length of strings that we read from databases.
 const MAX_STR_LENGTH = 64 * 1024;
 
 // Kernel-related types:
@@ -109,16 +111,17 @@ this.gLibs = gLibs; // ditto
 
 function convertESEError(errorCode) {
   switch (errorCode) {
     case -1213 /* JET_errPageSizeMismatch */:
     case -1002 /* JET_errInvalidName*/:
     case -1507 /* JET_errColumnNotFound */:
       // The DB format has changed and we haven't updated this migration code:
       return "The database format has changed, error code: " + errorCode;
+    case -1032 /* JET_errFileAccessDenied */:
     case -1207 /* JET_errDatabaseLocked */:
     case -1302 /* JET_errTableLocked */:
       return "The database or table is locked, error code: " + errorCode;
     case -1809 /* JET_errPermissionDenied*/:
     case -1907 /* JET_errAccessDenied */:
       return "Access or permission denied, error code: " + errorCode;
     case -1044 /* JET_errInvalidFilename */:
       return "Invalid file name";
@@ -576,15 +579,30 @@ let ESEDBReader = {
       db.incrementReferenceCounter();
       return db;
     }
     // ESE is really picky about the trailing slashes according to the docs,
     // so we do as we're told and ensure those are there:
     return new ESEDB(rootDir.path + "\\", dbFilePath, logDir.path + "\\");
   },
 
+  async dbLocked(dbFile) {
+    let options = {winShare: OS.Constants.Win.FILE_SHARE_READ};
+    let locked = true;
+    await OS.File.open(dbFile.path, {read: true}, options).then(fileHandle => {
+      locked = false;
+      // Return the close promise so we wait for the file to be closed again.
+      // Otherwise the file might still be kept open by this handle by the time
+      // that we try to use the ESE APIs to access it.
+      return fileHandle.close();
+    }, () => {
+      Cu.reportError("ESE DB at " + dbFile.path + " is locked.");
+    });
+    return locked;
+  },
+
   closeDB(db) {
     db.decrementReferenceCounter();
   },
 
   COLUMN_TYPES,
 };
 
--- a/browser/components/migration/EdgeProfileMigrator.js
+++ b/browser/components/migration/EdgeProfileMigrator.js
@@ -32,16 +32,17 @@ XPCOMUtils.defineLazyGetter(this, "gEdge
   }
   edgeDir.appendRelativePath(kEdgeDatabasePath);
   if (!edgeDir.exists() || !edgeDir.isReadable() || !edgeDir.isDirectory()) {
     return null;
   }
   let expectedLocation = edgeDir.clone();
   expectedLocation.appendRelativePath("nouser1\\120712-0049\\DBStore\\spartan.edb");
   if (expectedLocation.exists() && expectedLocation.isReadable() && expectedLocation.isFile()) {
+    expectedLocation.normalize();
     return expectedLocation;
   }
   // We used to recurse into arbitrary subdirectories here, but that code
   // went unused, so it likely isn't necessary, even if we don't understand
   // where the magic folders above come from, they seem to be the same for
   // everyone. Just return null if they're not there:
   return null;
 });
@@ -166,16 +167,19 @@ EdgeReadingListMigrator.prototype = {
       ex => {
         Cu.reportError(ex);
         callback(false);
       }
     );
   },
 
   _migrateReadingList: Task.async(function*(parentGuid) {
+    if (yield ESEDBReader.dbLocked(gEdgeDatabase)) {
+      throw new Error("Edge seems to be running - its database is locked.");
+    }
     let columnFn = db => {
       let columns = [
         {name: "URL", type: "string"},
         {name: "Title", type: "string"},
         {name: "AddedDate", type: "date"}
       ];
 
       // Later versions have an IsDeleted column:
@@ -244,16 +248,19 @@ EdgeBookmarksMigrator.prototype = {
       ex => {
         Cu.reportError(ex);
         callback(false);
       }
     );
   },
 
   _migrateBookmarks: Task.async(function*() {
+    if (yield ESEDBReader.dbLocked(this.db)) {
+      throw new Error("Edge seems to be running - its database is locked.");
+    }
     let {toplevelBMs, toolbarBMs} = this._fetchBookmarksFromDB();
     if (toplevelBMs.length) {
       let parentGuid = PlacesUtils.bookmarks.menuGuid;
       if (!MigrationUtils.isStartupMigration) {
         parentGuid = yield MigrationUtils.createImportedBookmarksFolder("Edge", parentGuid);
       }
       yield MigrationUtils.insertManyBookmarksWrapper(toplevelBMs, parentGuid);
     }