From 6fd67e426da1130e41feb8c3d5a1c529af71129a Mon Sep 17 00:00:00 2001
From: Natalia Milovanova <hameleonka28@gmail.com>
Date: Mon, 11 Dec 2017 21:48:41 -0500
Subject: [PATCH 1/3] refactors io except directory.js

---
 src/io/base.js  |  49 ++++++--------
 src/io/crx.js   |  54 ++++++++--------
 src/io/utils.js |  47 ++++++--------
 src/io/xpi.js   | 169 +++++++++++++++++++++++-------------------------
 4 files changed, 147 insertions(+), 172 deletions(-)

diff --git a/src/io/base.js b/src/io/base.js
index 2530390b8fe..3c3dac57147 100644
--- a/src/io/base.js
+++ b/src/io/base.js
@@ -45,50 +45,41 @@ export class IOBase {
     }
   }
 
-  getFilesByExt(...extensions) {
+  async getFilesByExt(...extensions) {
     for (let i = 0; i < extensions.length; i++) {
       const ext = extensions[i];
       if (ext.indexOf('.') !== 0) {
-        // We use Promise.reject as we're not inside a `then()` or a
-        // Promise constructor callback.
-        // If we throw here it won't be caught.
-        return Promise.reject(new Error("File extension must start with '.'"));
+        throw new Error("File extension must start with '.'");
       }
     }
 
-    return this.getFiles()
-      .then((filesObject) => {
-        const files = [];
+    const filesObject = await this.getFiles();
+    const files = [];
 
-        Object.keys(filesObject).forEach((filename) => {
-          extensions.forEach((ext) => {
-            if (filename.endsWith(ext)) {
-              files.push(filename);
-            }
-          });
-        });
-
-        return files;
+    Object.keys(filesObject).forEach((filename) => {
+      extensions.forEach((ext) => {
+        if (filename.endsWith(ext)) {
+          files.push(filename);
+        }
       });
+    });
+
+    return files;
   }
 
-  getFiles() {
-    return Promise.reject(
-      new Error('getFiles is not implemented'));
+  async getFiles() {
+    throw new Error('getFiles is not implemented');
   }
 
-  getFileAsStream() {
-    return Promise.reject(
-      new Error('getFileAsStream is not implemented'));
+  async getFileAsStream() {
+    throw new Error('getFileAsStream is not implemented');
   }
 
-  getFileAsString() {
-    return Promise.reject(
-      new Error('getFileAsString is not implemented'));
+  async getFileAsString() {
+    throw new Error('getFileAsString is not implemented');
   }
 
-  getChunkAsBuffer() {
-    return Promise.reject(
-      new Error('getChunkAsBuffer is not implemented'));
+  async getChunkAsBuffer() {
+    throw new Error('getChunkAsBuffer is not implemented');
   }
 }
diff --git a/src/io/crx.js b/src/io/crx.js
index e12d37dfbfc..748ce7e54ee 100644
--- a/src/io/crx.js
+++ b/src/io/crx.js
@@ -50,37 +50,35 @@ export class Crx extends Xpi {
     });
   }
 
-  getFiles(_onEventsSubscribed) {
-    return new Promise((resolve, reject) => {
-      // If we have already processed the file and have data
-      // on this instance return that.
-      if (Object.keys(this.files).length) {
-        return resolve(this.files);
-      }
+  async getFiles(_onEventsSubscribed) {
+    // If we have already processed the file and have data
+    // on this instance return that.
+    if (Object.keys(this.files).length) {
+      return this.files;
+    }
 
-      return this.open()
-        .then((zipfile) => {
-          zipfile.on('entry', (entry) => {
-            this.handleEntry(entry, reject);
-          });
+    const zipfile = await this.open();
 
-          // We use the 'end' event here because we're reading the CRX in
-          // from a buffer (because we have to unpack the header info from it
-          // first). The 'close' event is never fired when using yauzl's
-          // `fromBuffer()` method.
-          zipfile.on('end', () => {
-            resolve(this.files);
-          });
+    // We use the 'end' event here because we're reading the CRX in
+    // from a buffer (because we have to unpack the header info from it
+    // first). The 'close' event is never fired when using yauzl's
+    // `fromBuffer()` method.
+    return new Promise((resolve) => {
+      zipfile.on('entry', (entry) => {
+        this.handleEntry(entry);
+      });
 
-          if (_onEventsSubscribed) {
-            // Run optional callback when we know the event handlers
-            // have been inited. Useful for testing.
-            if (typeof _onEventsSubscribed === 'function') {
-              _onEventsSubscribed();
-            }
-          }
-        })
-        .catch(reject);
+      zipfile.on('end', () => {
+        resolve(this.files);
+      });
+
+      if (_onEventsSubscribed) {
+        // Run optional callback when we know the event handlers
+        // have been inited. Useful for testing.
+        if (typeof _onEventsSubscribed === 'function') {
+          _onEventsSubscribed();
+        }
+      }
     });
   }
 }
diff --git a/src/io/utils.js b/src/io/utils.js
index 2aa9bd325ba..87ea721e3b1 100644
--- a/src/io/utils.js
+++ b/src/io/utils.js
@@ -14,32 +14,27 @@ export function walkPromise(curPath, { shouldIncludePath = () => true } = {}) {
   // so all file paths (the result keys) can
   // be relative to the starting point.
   const basePath = curPath;
+  // eslint-disable-next-line consistent-return
+  return (async function walk(_curPath) {
+    const stat = await lstatPromise(_curPath);
+    const relPath = path.relative(basePath, _curPath);
 
-  return (function walk(_curPath) {
-    return lstatPromise(_curPath)
-      // eslint-disable-next-line consistent-return
-      .then((stat) => {
-        const relPath = path.relative(basePath, _curPath);
-        if (!shouldIncludePath(relPath, stat.isDirectory())) {
-          log.debug(`Skipping file path: ${relPath}`);
-          return result;
-        } else if (stat.isFile()) {
-          const { size } = stat;
-          result[relPath] = { size };
-        } else if (stat.isDirectory()) {
-          return readdirPromise(_curPath)
-            .then((files) => {
-              // Map the list of files and make a list of readdir
-              // promises to pass to Promise.all so we can recursively
-              // get the data on all the files in the directory.
-              return Promise.all(files.map((fileName) => {
-                return walk(path.join(_curPath, fileName));
-              }));
-            })
-            .then(() => {
-              return result;
-            });
-        }
-      });
+    if (!shouldIncludePath(relPath, stat.isDirectory())) {
+      log.debug(`Skipping file path: ${relPath}`);
+      return result;
+    } else if (stat.isFile()) {
+      const { size } = stat;
+      result[relPath] = { size };
+    } else if (stat.isDirectory()) {
+      const files = await readdirPromise(_curPath);
+
+      // Map the list of files and make a list of readdir
+      // promises to pass to Promise.all so we can recursively
+      // get the data on all the files in the directory.
+      await Promise.all(files.map(async (fileName) => {
+        await walk(path.join(_curPath, fileName));
+      }));
+      return result;
+    }
   }(curPath));
 }
diff --git a/src/io/xpi.js b/src/io/xpi.js
index dd9ccda08cb..235e3e7cf13 100644
--- a/src/io/xpi.js
+++ b/src/io/xpi.js
@@ -33,7 +33,7 @@ export class Xpi extends IOBase {
     });
   }
 
-  handleEntry(entry, reject) {
+  handleEntry(entry) {
     if (/\/$/.test(entry.fileName)) {
       return;
     }
@@ -43,53 +43,51 @@ export class Xpi extends IOBase {
     }
     if (this.entries.includes(entry.fileName)) {
       log.info('Found duplicate file entry: "%s" in package', entry.fileName);
-      reject(new Error(oneLine`DuplicateZipEntry: Entry
-        "${entry.fileName}" has already been seen`));
+      throw new Error(oneLine`DuplicateZipEntry: Entry
+        "${entry.fileName}" has already been seen`);
     }
     this.entries.push(entry.fileName);
     this.files[entry.fileName] = entry;
   }
 
-  getFiles(_onEventsSubscribed) {
-    return new Promise((resolve, reject) => {
-      // If we have already processed the file and have data
-      // on this instance return that.
-      if (Object.keys(this.files).length) {
-        const wantedFiles = {};
-        Object.keys(this.files).forEach((fileName) => {
-          if (this.shouldScanFile(fileName)) {
-            wantedFiles[fileName] = this.files[fileName];
-          } else {
-            log.debug(`Skipping cached file: ${fileName}`);
-          }
-        });
-        return resolve(wantedFiles);
-      }
+  async getFiles(_onEventsSubscribed) {
+    // If we have already processed the file and have data
+    // on this instance return that.
+    if (Object.keys(this.files).length) {
+      const wantedFiles = {};
+      Object.keys(this.files).forEach((fileName) => {
+        if (this.shouldScanFile(fileName)) {
+          wantedFiles[fileName] = this.files[fileName];
+        } else {
+          log.debug(`Skipping cached file: ${fileName}`);
+        }
+      });
+      return wantedFiles;
+    }
 
-      return this.open()
-        .then((zipfile) => {
-          zipfile.on('entry', (entry) => {
-            this.handleEntry(entry, reject);
-          });
-
-          // When the last entry has been processed
-          // and the fd is closed resolve the promise.
-          // Note: we cannot use 'end' here as 'end' is fired
-          // after the last entry event is emitted and streams
-          // may still be being read with openReadStream.
-          zipfile.on('close', () => {
-            resolve(this.files);
-          });
-
-          if (_onEventsSubscribed) {
-            // Run optional callback when we know the event handlers
-            // have been inited. Useful for testing.
-            if (typeof _onEventsSubscribed === 'function') {
-              _onEventsSubscribed();
-            }
-          }
-        })
-        .catch(reject);
+    const zipfile = await this.open();
+
+    return new Promise((resolve) => {
+      zipfile.on('entry', (entry) => {
+        this.handleEntry(entry);
+      });
+
+      // When the last entry has been processed
+      // and the fd is closed resolve the promise.
+      // Note: we cannot use 'end' here as 'end' is fired
+      // after the last entry event is emitted and streams
+      // may still be being read with openReadStream.
+      zipfile.on('close', () => {
+        resolve(this.files);
+      });
+
+      if (_onEventsSubscribed) {
+        // Run optional callback when we know the event handlers
+        // have been inited. Useful for testing.
+        if (typeof _onEventsSubscribed === 'function') {
+          _onEventsSubscribed();
+        }
+      }
     });
   }
 
@@ -103,62 +101,55 @@ export class Xpi extends IOBase {
     }
   }
 
-  getFileAsStream(path) {
+  async getFileAsStream(path) {
+    this.checkPath(path);
+    const zipfile = await this.open();
     return new Promise((resolve, reject) => {
-      this.checkPath(path);
-      return this.open()
-        .then((zipfile) => {
-          zipfile.openReadStream(this.files[path], (err, readStream) => {
-            if (err) {
-              return reject(err);
-            }
-            return resolve(readStream.pipe(stripBomStream()));
-          });
-        })
-        .catch(reject);
+      zipfile.openReadStream(this.files[path], (err, readStream) => {
+        if (err) {
+          return reject(err);
+        }
+        return resolve(readStream.pipe(stripBomStream()));
+      });
     });
   }
 
-  getFileAsString(path) {
-    return this.getFileAsStream(path)
-      .then((fileStream) => {
-        return new Promise((resolve, reject) => {
-          let buf = Buffer.from('');
-          fileStream.on('data', (chunk) => {
-            buf = Buffer.concat([buf, chunk]);
-          });
-
-          // Once the file is assembled, resolve the promise.
-          fileStream.on('end', () => {
-            const fileString = buf.toString('utf8');
-            resolve(fileString);
-          });
-
-          fileStream.on('error', reject);
-        });
+  async getFileAsString(path) {
+    const fileStream = await this.getFileAsStream(path);
+
+    return new Promise((resolve, reject) => {
+      let buf = Buffer.from('');
+      fileStream.on('data', (chunk) => {
+        buf = Buffer.concat([buf, chunk]);
+      });
+
+      // Once the file is assembled, resolve the promise.
+      fileStream.on('end', () => {
+        const fileString = buf.toString('utf8');
+        resolve(fileString);
       });
+
+      fileStream.on('error', reject);
+    });
   }
 
-  getChunkAsBuffer(path, chunkLength) {
+  async getChunkAsBuffer(path, chunkLength) {
+    this.checkPath(path);
+    const zipfile = await this.open();
     return new Promise((resolve, reject) => {
-      this.checkPath(path);
-      return this.open()
-        .then((zipfile) => {
-          // eslint-disable-next-line consistent-return
-          zipfile.openReadStream(this.files[path], (err, readStream) => {
-            if (err) {
-              return reject(err);
+      // eslint-disable-next-line consistent-return
+      zipfile.openReadStream(this.files[path], (err, readStream) => {
+        if (err) {
+          return reject(err);
+        }
+        readStream.pipe(
+          firstChunkStream({ chunkLength },
+            (_, enc) => {
+              resolve(enc);
             }
-            readStream.pipe(
-              firstChunkStream({ chunkLength },
-                (_, enc) => {
-                  resolve(enc);
-                }
-              )
-            );
-          });
-        })
-        .catch(reject);
+          )
+        );
+      });
     });
   }
 }

From 3d222dfb214e75b5df13595fbcf88efc3b12ce9f Mon Sep 17 00:00:00 2001
From: Natalia Milovanova <hameleonka28@gmail.com>
Date: Wed, 20 Dec 2017 20:55:21 -0500
Subject: [PATCH 2/3] reverses changes to handleEntry function, fixes tests
 callback call

---
 src/io/crx.js |  6 +++---
 src/io/xpi.js | 17 +++++++++--------
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/src/io/crx.js b/src/io/crx.js
index 748ce7e54ee..df565b39850 100644
--- a/src/io/crx.js
+++ b/src/io/crx.js
@@ -63,9 +63,9 @@ export class Crx extends Xpi {
     // from a buffer (because we have to unpack the header info from it
     // first). The 'close' event is never fired when using yauzl's
     // `fromBuffer()` method.
-    return new Promise((resolve) => {
+    return new Promise((resolve, reject) => {
       zipfile.on('entry', (entry) => {
-        this.handleEntry(entry);
+        this.handleEntry(entry, reject);
       });
 
       zipfile.on('end', () => {
@@ -76,7 +76,7 @@ export class Crx extends Xpi {
         // Run optional callback when we know the event handlers
         // have been inited. Useful for testing.
         if (typeof _onEventsSubscribed === 'function') {
-          _onEventsSubscribed();
+          Promise.resolve().then(() => _onEventsSubscribed(reject));
         }
       }
     });
diff --git a/src/io/xpi.js b/src/io/xpi.js
index 235e3e7cf13..69c9d569427 100644
--- a/src/io/xpi.js
+++ b/src/io/xpi.js
@@ -33,7 +33,7 @@ export class Xpi extends IOBase {
     });
   }
 
-  handleEntry(entry) {
+  handleEntry(entry, reject) {
     if (/\/$/.test(entry.fileName)) {
       return;
     }
@@ -43,8 +43,9 @@ export class Xpi extends IOBase {
     }
     if (this.entries.includes(entry.fileName)) {
       log.info('Found duplicate file entry: "%s" in package', entry.fileName);
-      throw new Error(oneLine`DuplicateZipEntry: Entry
-        "${entry.fileName}" has already been seen`);
+      reject(new Error(oneLine`DuplicateZipEntry: Entry
+        "${entry.fileName}" has already been seen`));
+      return;
     }
     this.entries.push(entry.fileName);
     this.files[entry.fileName] = entry;
@@ -67,9 +68,9 @@ export class Xpi extends IOBase {
 
     const zipfile = await this.open();
 
-    return new Promise((resolve) => {
+    return new Promise((resolve, reject) => {
       zipfile.on('entry', (entry) => {
-        this.handleEntry(entry);
+        this.handleEntry(entry, reject);
       });
 
       // When the last entry has been processed
@@ -85,7 +86,7 @@ export class Xpi extends IOBase {
         // Run optional callback when we know the event handlers
         // have been inited. Useful for testing.
         if (typeof _onEventsSubscribed === 'function') {
-          _onEventsSubscribed();
+          Promise.resolve().then(() => _onEventsSubscribed(reject));
         }
       }
     });
@@ -137,10 +138,10 @@ export class Xpi extends IOBase {
     this.checkPath(path);
     const zipfile = await this.open();
     return new Promise((resolve, reject) => {
-      // eslint-disable-next-line consistent-return
       zipfile.openReadStream(this.files[path], (err, readStream) => {
         if (err) {
-          return reject(err);
+          reject(err);
+          return;
         }
         readStream.pipe(
           firstChunkStream({ chunkLength },

From 70201ca3ef88c3b0ce552e58ffeea7f23821bdeb Mon Sep 17 00:00:00 2001
From: Natalia Milovanova <hameleonka28@gmail.com>
Date: Tue, 26 Dec 2017 14:53:45 -0500
Subject: [PATCH 3/3] changes onEventSubscribed call and walkPromise function

---
 src/io/crx.js   | 2 +-
 src/io/utils.js | 4 +---
 src/io/xpi.js   | 2 +-
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/io/crx.js b/src/io/crx.js
index df565b39850..612ab5e5c09 100644
--- a/src/io/crx.js
+++ b/src/io/crx.js
@@ -76,7 +76,7 @@ export class Crx extends Xpi {
         // Run optional callback when we know the event handlers
         // have been inited. Useful for testing.
         if (typeof _onEventsSubscribed === 'function') {
-          Promise.resolve().then(() => _onEventsSubscribed(reject));
+          Promise.resolve().then(() => _onEventsSubscribed());
         }
       }
     });
diff --git a/src/io/utils.js b/src/io/utils.js
index 87ea721e3b1..1058823c040 100644
--- a/src/io/utils.js
+++ b/src/io/utils.js
@@ -14,14 +14,12 @@ export function walkPromise(curPath, { shouldIncludePath = () => true } = {}) {
   // so all file paths (the result keys) can
   // be relative to the starting point.
   const basePath = curPath;
-  // eslint-disable-next-line consistent-return
   return (async function walk(_curPath) {
     const stat = await lstatPromise(_curPath);
     const relPath = path.relative(basePath, _curPath);
 
     if (!shouldIncludePath(relPath, stat.isDirectory())) {
       log.debug(`Skipping file path: ${relPath}`);
-      return result;
     } else if (stat.isFile()) {
       const { size } = stat;
       result[relPath] = { size };
@@ -34,7 +32,7 @@ export function walkPromise(curPath, { shouldIncludePath = () => true } = {}) {
       await Promise.all(files.map(async (fileName) => {
         await walk(path.join(_curPath, fileName));
       }));
-      return result;
     }
+    return result;
   }(curPath));
 }
diff --git a/src/io/xpi.js b/src/io/xpi.js
index 69c9d569427..f62c2983087 100644
--- a/src/io/xpi.js
+++ b/src/io/xpi.js
@@ -86,7 +86,7 @@ export class Xpi extends IOBase {
         // Run optional callback when we know the event handlers
         // have been inited. Useful for testing.
         if (typeof _onEventsSubscribed === 'function') {
-          Promise.resolve().then(() => _onEventsSubscribed(reject));
+          Promise.resolve().then(() => _onEventsSubscribed());
         }
       }
     });