Bug 1400467 - Ensure services/common/logmanager.js awaits it's cleanup function r=markh

MozReview-Commit-ID: thQph1UUA0

--HG--
extra : rebase_source : 705cdd9d07a1a26b568554e07638c0415789b404

services/common/logmanager.js

@@ -266,19 +266,18 @@ LogManager.prototype = {
       // logs are grouped in about:sync-log.
       let filename = reasonPrefix + "-" + this.logFilePrefix + "-" + Date.now() + ".txt";
       await this._fileAppender.flushToFile(this._logFileSubDirectoryEntries, filename, this._log);
-
       // It's not completely clear to markh why we only do log cleanups
       // for errors, but for now the Sync semantics have been copied...
       // (one theory is that only cleaning up on error makes it less
       // likely old error logs would be removed, but that's not true if
       // there are occasional errors - let's address this later!)
       if (reason == this.ERROR_LOG_WRITTEN && !this._cleaningUpFileLogs) {
-        this._log.trace("Scheduling cleanup.");
-        // Note we don't return/await or otherwise wait on this promise - it
-        // continues in the background
-        this.cleanupLogs().catch(err => {
+        this._log.trace("Running cleanup.");
+        try {
+          await this.cleanupLogs();
+        } catch (err) {
           this._log.error("Failed to cleanup logs", err);
-        });
+        }
       }
       return reason;
     } catch (ex) {
@@ -321,7 +320,13 @@ LogManager.prototype = {
                         + entry.name, ex);
       }
     });
-    iterator.close();
+    // Wait for this to close if we need to (but it might fail if OS.File has
+    // shut down)
+    try {
+      await iterator.close();
+    } catch (e) {
+      this._log.warn("Failed to close directory iterator", e);
+    }
     this._cleaningUpFileLogs = false;
     this._log.debug("Done deleting files.");
     // This notification is used only for tests.

services/sync/tests/unit/test_errorhandler_filelog.js

@@ -162,13 +162,6 @@ add_test(function test_sync_error_logOnError_true() {
   const MESSAGE = "this WILL show up";
   log.info(MESSAGE);
 
-  // We need to wait until the log cleanup started by this test is complete
-  // or the next test will fail as it is ongoing.
-  Svc.Obs.add("services-tests:common:log-manager:cleanup-logs", function onCleanupLogs() {
-    Svc.Obs.remove("services-tests:common:log-manager:cleanup-logs", onCleanupLogs);
-    run_next_test();
-  });
-
   Svc.Obs.add("weave:service:reset-file-log", function onResetFileLog() {
     Svc.Obs.remove("weave:service:reset-file-log", onResetFileLog);
 
@@ -194,6 +187,7 @@ add_test(function test_sync_error_logOnError_true() {
       }
 
       Svc.Prefs.resetBranch("");
+      run_next_test();
     });
   });
 
@@ -229,13 +223,6 @@ add_test(function test_login_error_logOnError_true() {
   const MESSAGE = "this WILL show up";
   log.info(MESSAGE);
 
-  // We need to wait until the log cleanup started by this test is complete
-  // or the next test will fail as it is ongoing.
-  Svc.Obs.add("services-tests:common:log-manager:cleanup-logs", function onCleanupLogs() {
-    Svc.Obs.remove("services-tests:common:log-manager:cleanup-logs", onCleanupLogs);
-    run_next_test();
-  });
-
   Svc.Obs.add("weave:service:reset-file-log", function onResetFileLog() {
     Svc.Obs.remove("weave:service:reset-file-log", onResetFileLog);
 
@@ -261,6 +248,7 @@ add_test(function test_login_error_logOnError_true() {
       }
 
       Svc.Prefs.resetBranch("");
+      run_next_test();
     });
   });
 
@@ -301,13 +289,6 @@ add_test(function test_newFailed_errorLog() {
   const MESSAGE = "this WILL show up 2";
   log.info(MESSAGE);
 
-  // We need to wait until the log cleanup started by this test is complete
-  // or the next test will fail as it is ongoing.
-  Svc.Obs.add("services-tests:common:log-manager:cleanup-logs", function onCleanupLogs() {
-    Svc.Obs.remove("services-tests:common:log-manager:cleanup-logs", onCleanupLogs);
-    run_next_test();
-  });
-
   Svc.Obs.add("weave:service:reset-file-log", function onResetFileLog() {
     Svc.Obs.remove("weave:service:reset-file-log", onResetFileLog);
 
@@ -333,7 +314,7 @@ add_test(function test_newFailed_errorLog() {
       }
 
       Svc.Prefs.resetBranch("");
-
+      run_next_test();
     });
   });
   // newFailed is nonzero -- should write a log.
@@ -352,13 +333,6 @@ add_test(function test_newFailed_errorLog() {
 add_test(function test_errorLog_dumpAddons() {
   Svc.Prefs.set("log.appender.file.logOnError", true);
 
-  // We need to wait until the log cleanup started by this test is complete
-  // or the next test will fail as it is ongoing.
-  Svc.Obs.add("services-tests:common:log-manager:cleanup-logs", function onCleanupLogs() {
-    Svc.Obs.remove("services-tests:common:log-manager:cleanup-logs", onCleanupLogs);
-    run_next_test();
-  });
-
   Svc.Obs.add("weave:service:reset-file-log", function onResetFileLog() {
     Svc.Obs.remove("weave:service:reset-file-log", onResetFileLog);
 
@@ -383,6 +357,7 @@ add_test(function test_errorLog_dumpAddons() {
       }
 
       Svc.Prefs.resetBranch("");
+      run_next_test();
     });
   });