Bug 1473687 - Wait for files to finish loading in Scratchpad test. r=ochameau

In devtools/client/scratchpad/test/browser_scratchpad_recent_files.js, we would
import test files and delete them from disk before the read had fully completed,
which caused an error.  (This test also had various other troubles like not
actually writing anything to the test files.)

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

--HG--
extra : moz-landing-system : lando

devtools/client/scratchpad/scratchpad.js

@@ -1118,63 +1118,69 @@ var Scratchpad = {
    *        get the following arguments:
    *        1) the nsresult status code for the import operation.
    *        2) the data that was read from the file, if any.
+   * @return Promise resolved with array of callback args
    */
   importFromFile: function SP_importFromFile(aFile, aSilentError, aCallback) {
-    // Prevent file type detection.
+    return new Promise(resolve => {
-    const channel = NetUtil.newChannel({
+      // Prevent file type detection.
-      uri: NetUtil.newURI(aFile),
+      const channel = NetUtil.newChannel({
-      loadingNode: window.document,
+        uri: NetUtil.newURI(aFile),
-      securityFlags: Ci.nsILoadInfo.SEC_ALLOW_CROSS_ORIGIN_DATA_INHERITS,
+        loadingNode: window.document,
-      contentPolicyType: Ci.nsIContentPolicy.TYPE_OTHER});
+        securityFlags: Ci.nsILoadInfo.SEC_ALLOW_CROSS_ORIGIN_DATA_INHERITS,
-    channel.contentType = "application/javascript";
+        contentPolicyType: Ci.nsIContentPolicy.TYPE_OTHER
+      });
+      channel.contentType = "application/javascript";
 
-    this.notificationBox.removeAllNotifications(false);
+      this.notificationBox.removeAllNotifications(false);
 
-    NetUtil.asyncFetch(channel, (aInputStream, aStatus) => {
+      NetUtil.asyncFetch(channel, (aInputStream, aStatus) => {
-      let content = null;
+        let content = null;
 
-      if (Components.isSuccessCode(aStatus)) {
+        if (Components.isSuccessCode(aStatus)) {
-        const charsets = this._getApplicableCharsets();
+          const charsets = this._getApplicableCharsets();
-        content = NetUtil.readInputStreamToString(aInputStream,
+          content = NetUtil.readInputStreamToString(aInputStream,
-                                                  aInputStream.available());
+            aInputStream.available());
-        content = this._getUnicodeContent(content, charsets);
+          content = this._getUnicodeContent(content, charsets);
-        if (!content) {
+          if (!content) {
-          const message = this.strings.formatStringFromName(
+            const message = this.strings.formatStringFromName(
-            "importFromFile.convert.failed",
+              "importFromFile.convert.failed",
-            [ charsets.join(", ") ],
+              [charsets.join(", ")],
-            1);
+              1);
-          this.notificationBox.appendNotification(
+            this.notificationBox.appendNotification(
-            message,
+              message,
-            "file-import-convert-failed",
+              "file-import-convert-failed",
-            null,
+              null,
-            this.notificationBox.PRIORITY_CRITICAL_MEDIUM,
+              this.notificationBox.PRIORITY_CRITICAL_MEDIUM,
-            null);
+              null);
-          if (aCallback) {
+            if (aCallback) {
-            aCallback.call(this, aStatus, content);
+              aCallback.call(this, aStatus, content);
+            }
+            resolve([aStatus, content]);
+            return;
           }
-          return;
+          // Check to see if the first line is a mode-line comment.
-        }
+          const line = content.split("\n")[0];
-        // Check to see if the first line is a mode-line comment.
+          const modeline = this._scanModeLine(line);
-        const line = content.split("\n")[0];
+          const chrome = Services.prefs.getBoolPref(DEVTOOLS_CHROME_ENABLED);
-        const modeline = this._scanModeLine(line);
-        const chrome = Services.prefs.getBoolPref(DEVTOOLS_CHROME_ENABLED);
 
-        if (chrome && modeline["-sp-context"] === "browser") {
+          if (chrome && modeline["-sp-context"] === "browser") {
-          this.setBrowserContext();
+            this.setBrowserContext();
-        }
+          }
 
-        this.editor.setText(content);
+          this.editor.setText(content);
-        this.editor.clearHistory();
+          this.editor.clearHistory();
-        this.dirty = false;
+          this.dirty = false;
-        document.getElementById("sp-cmd-revert").setAttribute("disabled", true);
+          document.getElementById("sp-cmd-revert").setAttribute("disabled", true);
-      } else if (!aSilentError) {
+        } else if (!aSilentError) {
-        window.alert(this.strings.GetStringFromName("openFile.failed"));
+          window.alert(this.strings.GetStringFromName("openFile.failed"));
-      }
+        }
-      this.setFilename(aFile.path);
+        this.setFilename(aFile.path);
-      this.setRecentFile(aFile);
+        this.setRecentFile(aFile);
-      if (aCallback) {
+        if (aCallback) {
-        aCallback.call(this, aStatus, content);
+          aCallback.call(this, aStatus, content);
-      }
+        }
+        resolve([aStatus, content]);
+      });
     });
   },
 
@@ -1404,24 +1410,28 @@ var Scratchpad = {
    *
    * @param function aCallback
    *        Optional function you want to call when file is saved
+   * @return Promise
    */
   saveFile: function SP_saveFile(aCallback) {
     if (!this.filename) {
       return this.saveFileAs(aCallback);
     }
 
-    const file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
+    return new Promise(resolve => {
-    file.initWithPath(this.filename);
+      const file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
+      file.initWithPath(this.filename);
 
-    this.exportToFile(file, true, false, aStatus => {
+      this.exportToFile(file, true, false, aStatus => {
-      if (Components.isSuccessCode(aStatus)) {
+        if (Components.isSuccessCode(aStatus)) {
-        this.dirty = false;
+          this.dirty = false;
-        document.getElementById("sp-cmd-revert").setAttribute("disabled", true);
+          document.getElementById("sp-cmd-revert").setAttribute("disabled", true);
-        this.setRecentFile(file);
+          this.setRecentFile(file);
-      }
+        }
-      if (aCallback) {
+        if (aCallback) {
-        aCallback(aStatus);
+          aCallback(aStatus);
-      }
+        }
+        resolve(aStatus);
+      });
     });
   },
 
@@ -1430,30 +1440,34 @@ var Scratchpad = {
    *
    * @param function aCallback
    *        Optional function you want to call when file is saved
+   * @return Promise
    */
   saveFileAs: function SP_saveFileAs(aCallback) {
-    const fp = Cc["@mozilla.org/filepicker;1"].createInstance(Ci.nsIFilePicker);
+    return new Promise(resolve => {
-    const fpCallback = aResult => {
+      const fp = Cc["@mozilla.org/filepicker;1"].createInstance(Ci.nsIFilePicker);
-      if (aResult != Ci.nsIFilePicker.returnCancel) {
+      const fpCallback = aResult => {
-        this.setFilename(fp.file.path);
+        if (aResult != Ci.nsIFilePicker.returnCancel) {
-        this.exportToFile(fp.file, true, false, aStatus => {
+          this.setFilename(fp.file.path);
-          if (Components.isSuccessCode(aStatus)) {
+          this.exportToFile(fp.file, true, false, aStatus => {
-            this.dirty = false;
+            if (Components.isSuccessCode(aStatus)) {
-            this.setRecentFile(fp.file);
+              this.dirty = false;
-          }
+              this.setRecentFile(fp.file);
-          if (aCallback) {
+            }
-            aCallback(aStatus);
+            if (aCallback) {
-          }
+              aCallback(aStatus);
-        });
+            }
-      }
+            resolve(aStatus);
-    };
+          });
+        }
+      };
 
-    fp.init(window, this.strings.GetStringFromName("saveFileAs"),
+      fp.init(window, this.strings.GetStringFromName("saveFileAs"),
-            Ci.nsIFilePicker.modeSave);
+        Ci.nsIFilePicker.modeSave);
-    fp.defaultString = "scratchpad.js";
+      fp.defaultString = "scratchpad.js";
-    fp.appendFilter("JavaScript Files", "*.js; *.jsm; *.json");
+      fp.appendFilter("JavaScript Files", "*.js; *.jsm; *.json");
-    fp.appendFilter("All Files", "*.*");
+      fp.appendFilter("All Files", "*.*");
-    fp.open(fpCallback);
+      fp.open(fpCallback);
+    });
   },
 
   /**

devtools/client/scratchpad/test/browser_scratchpad_recent_files.js

@@ -32,24 +32,24 @@ var gFileContent02 = "hello.world.02('bug651942');";
 var gFileContent03 = "hello.world.03('bug651942');";
 var gFileContent04 = "hello.world.04('bug651942');";
 
-function startTest() {
+async function startTest() {
   gScratchpad = gScratchpadWindow.Scratchpad;
 
-  gFile01 = createAndLoadTemporaryFile(gFile01, gFileName01, gFileContent01);
+  gFile01 = await createAndLoadTemporaryFile(gFileName01, gFileContent01);
-  gFile02 = createAndLoadTemporaryFile(gFile02, gFileName02, gFileContent02);
+  gFile02 = await createAndLoadTemporaryFile(gFileName02, gFileContent02);
-  gFile03 = createAndLoadTemporaryFile(gFile03, gFileName03, gFileContent03);
+  gFile03 = await createAndLoadTemporaryFile(gFileName03, gFileContent03);
 }
 
 // Test to see if the three files we created in the 'startTest()'-method have
 // been added to the list of recent files.
-function testAddedToRecent() {
+async function testAddedToRecent() {
   lists.recentFiles01 = gScratchpad.getRecentFiles();
 
   is(lists.recentFiles01.length, 3,
      "Temporary files created successfully and added to list of recent files.");
 
   // Create a 4th file, this should clear the oldest file.
-  gFile04 = createAndLoadTemporaryFile(gFile04, gFileName04, gFileContent04);
+  gFile04 = await createAndLoadTemporaryFile(gFileName04, gFileContent04);
 }
 
 // We have opened a 4th file. Test to see if the oldest recent file was removed,
@@ -175,35 +175,27 @@ function testClearedAll() {
   finishTest();
 }
 
-function createAndLoadTemporaryFile(aFile, aFileName, aFileContent) {
+async function createAndLoadTemporaryFile(aFileName, aFileContent) {
+  info(`Create file: ${aFileName}`);
+
   // Create a temporary file.
-  aFile = FileUtils.getFile("TmpD", [aFileName]);
+  const aFile = FileUtils.getFile("TmpD", [aFileName]);
   aFile.createUnique(Ci.nsIFile.NORMAL_FILE_TYPE, 0o666);
 
   // Write the temporary file.
-  const fout = Cc["@mozilla.org/network/file-output-stream;1"]
+  await OS.File.writeAtomic(aFile.path, aFileContent);
-             .createInstance(Ci.nsIFileOutputStream);
-  fout.init(aFile.QueryInterface(Ci.nsIFile), 0x02 | 0x08 | 0x20,
-            0o644, fout.DEFER_OPEN);
 
   gScratchpad.setFilename(aFile.path);
-  gScratchpad.importFromFile(aFile.QueryInterface(Ci.nsIFile), true,
+  let [status] = await gScratchpad.importFromFile(aFile.QueryInterface(Ci.nsIFile), true);
-                            fileImported);
+  ok(Components.isSuccessCode(status),
-  gScratchpad.saveFile(fileSaved);
-
-  return aFile;
-}
-
-function fileImported(aStatus) {
-  ok(Components.isSuccessCode(aStatus),
      "the temporary file was imported successfully with Scratchpad");
-}
+  status = await gScratchpad.saveFile();
-
+  ok(Components.isSuccessCode(status),
-function fileSaved(aStatus) {
-  ok(Components.isSuccessCode(aStatus),
      "the temporary file was saved successfully with Scratchpad");
-
   checkIfMenuIsPopulated();
+
+  info(`File created: ${aFileName}`);
+  return aFile;
 }
 
 function checkIfMenuIsPopulated() {
@@ -245,47 +237,57 @@ var PreferenceObserver = {
     this._initialized = true;
   },
 
-  observe: function PO_observe(aMessage, aTopic, aData) {
+  observe: async function PO_observe(aMessage, aTopic, aData) {
     if (aTopic != "nsPref:changed") {
       return;
     }
 
-    switch (this.timesFired) {
+    if (this._inProgress) {
-      case 0:
+      await this._inProgress;
-        this.timesFired = 1;
-        break;
-      case 1:
-        this.timesFired = 2;
-        break;
-      case 2:
-        this.timesFired = 3;
-        testAddedToRecent();
-        break;
-      case 3:
-        this.timesFired = 4;
-        testOverwriteRecent();
-        break;
-      case 4:
-        this.timesFired = 5;
-        testOpenOldestRecent();
-        break;
-      case 5:
-        this.timesFired = 6;
-        testHideMenu();
-        break;
-      case 6:
-        this.timesFired = 7;
-        testChangedMaxRecent();
-        break;
-      case 7:
-        this.timesFired = 8;
-        testOpenDeletedFile();
-        break;
-      case 8:
-        this.timesFired = 9;
-        testClearedAll();
-        break;
     }
+
+    this._inProgress = new Promise(async resolve => {
+      info(`Times fired: ${this.timesFired}`);
+      switch (this.timesFired) {
+        case 0:
+          this.timesFired = 1;
+          break;
+        case 1:
+          this.timesFired = 2;
+          break;
+        case 2:
+          this.timesFired = 3;
+          await testAddedToRecent();
+          break;
+        case 3:
+          this.timesFired = 4;
+          testOverwriteRecent();
+          break;
+        case 4:
+          this.timesFired = 5;
+          testOpenOldestRecent();
+          break;
+        case 5:
+          this.timesFired = 6;
+          testHideMenu();
+          break;
+        case 6:
+          this.timesFired = 7;
+          testChangedMaxRecent();
+          break;
+        case 7:
+          this.timesFired = 8;
+          testOpenDeletedFile();
+          break;
+        case 8:
+          this.timesFired = 9;
+          testClearedAll();
+          break;
+      }
+
+      this._inProgress = null;
+      resolve();
+    });
   },
 
   uninit: function PO_uninit() {