Commit 23f28ddb authored by Marco Bonardo's avatar Marco Bonardo
Browse files

Bug 1862694 - places.database.replaceDatabaseOnStartup is not reset if the... 

Bug 1862694 - places.database.replaceDatabaseOnStartup is not reset if the database is cloned on startup.  a=diannaS

Original Revision: https://phabricator.services.mozilla.com/D192677

Differential Revision: https://phabricator.services.mozilla.com/D192922
parent cb22e17a

toolkit/components/places/Database.cpp

+26 −9
Original line number Diff line number Diff line
@@ -143,6 +143,23 @@ bool isRecentCorruptFile(const nsCOMPtr<nsIFile>& aCorruptFile) { 
         lastMod > 0 && (PR_Now() - lastMod) <= RECENT_BACKUP_TIME_MICROSEC;

}



/**

 * Removes a file, optionally adding a suffix to the file name.

 */

void RemoveFileSwallowsErrors(const nsCOMPtr<nsIFile>& aFile,

                              const nsString& aSuffix = u""_ns) {

  nsCOMPtr<nsIFile> file;

  MOZ_ALWAYS_SUCCEEDS(aFile->Clone(getter_AddRefs(file)));

  if (!aSuffix.IsEmpty()) {

    nsAutoString newFileName;

    file->GetLeafName(newFileName);

    newFileName.Append(aSuffix);

    MOZ_ALWAYS_SUCCEEDS(file->SetLeafName(newFileName));

  }

  DebugOnly<nsresult> rv = file->Remove(false);

  NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to remove file.");

}



/**

 * Sets the connection journal mode to one of the JOURNAL_* types.

 *
@@ -795,12 +812,9 @@ nsresult Database::BackupAndReplaceDatabaseFile( 
    };

    eCorruptDBReplaceStage stage = stage_closing;

    auto guard = MakeScopeExit([&]() {

      if (stage != stage_replaced) {

        // Reaching this point means the database is corrupt and we failed to

        // replace it.  For this session part of the application related to

        // bookmarks and history will misbehave.  The frontend may show a

        // "locked" notification to the user though.

        // Set up a pref to try replacing the database at the next startup.

      // In case we failed to close the connection or remove the database file,

      // we want to try again at the next startup.

      if (stage == stage_closing || stage == stage_removing) {

        Preferences::SetString(PREF_FORCE_DATABASE_REPLACEMENT, aDbFilename);

      }

      // Report the corruption through telemetry.
@@ -886,7 +900,7 @@ nsresult Database::TryToCloneTablesFromCorruptDatabase( 
    if (conn) {

      Unused << conn->Close();

    }

    Unused << recoverFile->Remove(false);

    RemoveFileSwallowsErrors(recoverFile);

  });



  rv = aStorage->OpenUnsharedDatabase(recoverFile,
@@ -961,11 +975,14 @@ nsresult Database::TryToCloneTablesFromCorruptDatabase( 
  rv = transaction.Commit();

  NS_ENSURE_SUCCESS(rv, rv);



  Unused << conn->Close();

  MOZ_ALWAYS_SUCCEEDS(conn->Close());

  conn = nullptr;

  rv = recoverFile->RenameTo(nullptr, filename);

  NS_ENSURE_SUCCESS(rv, rv);

  Unused << corruptFile->Remove(false);



  RemoveFileSwallowsErrors(corruptFile);

  RemoveFileSwallowsErrors(corruptFile, u"-wal"_ns);

  RemoveFileSwallowsErrors(corruptFile, u"-shm"_ns);



  guard.release();

  return NS_OK;

toolkit/components/places/tests/maintenance/head.js

+14 −0
Original line number Diff line number Diff line
@@ -99,7 +99,21 @@ async function test_database_replacement(src, filename, shouldClone, dbStatus) { 
      !(await IOUtils.exists(corrupt)),

      "The corrupt db should not exist"

    );

    Assert.ok(

      !(await IOUtils.exists(corrupt + "-wal")),

      "The corrupt db wal should not exist"

    );

    Assert.ok(

      !(await IOUtils.exists(corrupt + "-shm")),

      "The corrupt db shm should not exist"

    );

  } else {

    Assert.ok(await IOUtils.exists(corrupt), "The corrupt db should exist");

  }



  Assert.equal(

    Services.prefs.getCharPref("places.database.replaceDatabaseOnStartup", ""),

    "",

    "The replaceDatabaseOnStartup pref should have been unset"

  );

}