Bug 1684854 - Consistently handle non-existence of usage file in QuotaClient::InitOrigin. r=dom-workers-and-storage-reviewers,janv

Specifically, do not attempt and fail removal of the usage file in
CreateStorageConnection if the usage file did not exist.

Differential Revision: https://phabricator.services.mozilla.com/D100675

dom/localstorage/ActorsParent.cpp

@@ -504,8 +504,19 @@ CreateStorageConnection(nsIFile& aDBFile, nsIFile& aUsageFile,
                   -> Result<std::pair<nsCOMPtr<mozIStorageConnection>, bool>,
                             nsresult> {
                 if (rv == NS_ERROR_FILE_CORRUPTED) {
-                  // Remove the usage file first.
-                  LS_TRY(aUsageFile.Remove(false));
+                  // Remove the usage file first (it might not exist at all due
+                  // to corrupted state, which is ignored here).
+                  LS_TRY(
+                      ToResult(aUsageFile.Remove(false))
+                          .orElse(
+                              [](const nsresult rv) -> Result<Ok, nsresult> {
+                                if (rv == NS_ERROR_FILE_NOT_FOUND ||
+                                    rv == NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) {
+                                  return Ok{};
+                                }
+
+                                return Err(rv);
+                              }));
 
                   // Nuke the database file.
                   LS_TRY(aDBFile.Remove(false));
@@ -8789,6 +8800,9 @@ Result<UsageInfo, nsresult> QuotaClient::InitOrigin(
         &aGroupAndOrigin]() -> Result<UsageInfo, nsresult> {
         if (fileExists) {
           LS_TRY_RETURN(
+              // To simplify control flow, we call LoadUsageFile unconditionally
+              // here, even though it will necessarily fail if usageFileExists
+              // is false.
               LoadUsageFile(*usageFile)
                   .orElse([&file, &usageFile, &usageJournalFile,
                            &aGroupAndOrigin](

dom/localstorage/test/unit/test_corruptedDatabase.js

@@ -3,12 +3,8 @@
  * http://creativecommons.org/publicdomain/zero/1.0/
  */
 
-async function testSteps() {
-  const principal = getPrincipal("http://example.org");
-
-  info("Setting pref");
-
-  Services.prefs.setBoolPref("dom.storage.next_gen", true);
+async function doTest(profile) {
+  info("Testing profile " + profile);
 
   info("Clearing");
 
@@ -17,21 +13,9 @@ async function testSteps() {
 
   info("Installing package");
 
-  // The profile contains one localStorage, all localStorage related files, a
-  // script for localStorage creation and the storage database:
-  // - storage/default/http+++example.org/ls
-  // - storage/ls-archive.sqlite
-  // - create_db.js
-  // - storage.sqlite
-  // - webappsstore.sqlite
-  // The file create_db.js in the package was run locally, specifically it was
-  // temporarily added to xpcshell.ini and then executed:
-  //   mach xpcshell-test --interactive dom/localstorage/test/unit/create_db.js
-  // Note: to make it become the profile in the test, additional manual steps
-  // are needed.
-  // 1. Manually change first 6 chars in data.sqlite to "foobar".
-  // 2. Remove the folder "storage/temporary".
-  installPackage("corruptedDatabase_profile");
+  installPackage(profile);
+
+  const principal = getPrincipal("http://example.org");
 
   let storage = getLocalStorage(principal);
 
@@ -51,3 +35,36 @@ async function testSteps() {
 
   is(request.result.usage, 0, "Correct usage");
 }
+
+async function testSteps() {
+  info("Setting pref");
+
+  Services.prefs.setBoolPref("dom.storage.next_gen", true);
+
+  // XXX This should be refactored into separate sub test cases.
+
+  const profiles = [
+    // This profile contains one localStorage, all localStorage related files, a
+    // script for localStorage creation and the storage database:
+    // - storage/default/http+++example.org/ls
+    // - storage/ls-archive.sqlite
+    // - create_db.js
+    // - storage.sqlite
+    // - webappsstore.sqlite
+    // The file create_db.js in the package was run locally, specifically it was
+    // temporarily added to xpcshell.ini and then executed:
+    //   mach xpcshell-test --interactive dom/localstorage/test/unit/create_db.js
+    // Note: to make it become the profile in the test, additional manual steps
+    // are needed.
+    // 1. Manually change first 6 chars in data.sqlite to "foobar".
+    // 2. Remove the folder "storage/temporary".
+    "corruptedDatabase_profile",
+    // This profile is the same as corruptedDatabase_profile, except that the usage
+    // file (storage/default/http+++example.org/ls/usage) is missing.
+    "corruptedDatabase_missingUsageFile_profile",
+  ];
+
+  for (const profile of profiles) {
+    await doTest(profile);
+  }
+}

dom/localstorage/test/unit/xpcshell.ini

@@ -7,6 +7,7 @@ head = head.js
 support-files =
   archive_profile.zip
   corruptedDatabase_profile.zip
+  corruptedDatabase_missingUsageFile_profile.zip
   groupMismatch_profile.zip
   migration_profile.zip
   schema3upgrade_profile.zip