[api] Launcher: Close gracefully when a userDataDir is specified (#700)

This patch:
- makes `browser.close()` return a promise that resolves when browser gets closed
- starts closing chrome gracefully if a custom `userDataDir` is supplied

Fixes #527

docs/api.md

@@ -211,6 +211,7 @@ puppeteer.launch().then(async browser => {
 ```
 
 #### browser.close()
+- returns: <[Promise]>
 
 Closes browser with all the pages (if any were opened). The browser object itself is considered to be disposed and could not be used anymore.
 

lib/Browser.js

@@ -16,14 +16,16 @@
 
 const {helper} = require('./helper');
 const Page = require('./Page');
+const EventEmitter = require('events');
 
-class Browser {
+class Browser extends EventEmitter {
   /**
    * @param {!Connection} connection
    * @param {boolean} ignoreHTTPSErrors
-   * @param {function()=} closeCallback
+   * @param {function():Promise=} closeCallback
    */
   constructor(connection, ignoreHTTPSErrors, closeCallback) {
+    super();
     this._ignoreHTTPSErrors = ignoreHTTPSErrors;
     this._screenshotTaskQueue = new TaskQueue();
     this._connection = connection;
@@ -54,9 +56,9 @@ class Browser {
     return version.product;
   }
 
-  close() {
+  async close() {
     this._connection.dispose();
-    this._closeCallback.call(null);
+    await this._closeCallback.call(null);
   }
 }
 

lib/Launcher.js

@@ -84,9 +84,14 @@ class Launcher {
       chromeProcess.stderr.pipe(process.stderr);
     }
 
-    // Cleanup as processes exit.
-    if (temporaryUserDataDir)
-      chromeProcess.once('close', () => removeSync(temporaryUserDataDir));
+    const waitForChromeToClose = new Promise(fulfill => {
+      chromeProcess.once('close', () => {
+        // Cleanup as processes exit.
+        if (temporaryUserDataDir)
+          removeSync(temporaryUserDataDir);
+        fulfill();
+      });
+    });
 
     const listeners = [ helper.addEventListener(process, 'exit', killChrome) ];
     if (options.handleSIGINT !== false)
@@ -102,12 +107,29 @@ class Launcher {
       throw e;
     }
 
+    /**
+     * @return {Promise}
+     */
     function killChrome() {
       helper.removeEventListeners(listeners);
-      if (process.platform === 'win32')
-        childProcess.execSync(`taskkill /pid ${chromeProcess.pid} /T /F`);
-      else
-        process.kill(-chromeProcess.pid);
+      if (temporaryUserDataDir) {
+        // Force kill chrome.
+        if (process.platform === 'win32')
+          childProcess.execSync(`taskkill /pid ${chromeProcess.pid} /T /F`);
+        else
+          process.kill(-chromeProcess.pid, 'SIGKILL');
+        // Attempt to remove temporary profile directory to avoid littering.
+        try {
+          removeSync(temporaryUserDataDir);
+        } catch (e) { }
+      } else {
+        // Terminate chrome gracefully.
+        if (process.platform === 'win32')
+          chromeProcess.kill();
+        else
+          process.kill(-chromeProcess.pid, 'SIGTERM');
+      }
+      return waitForChromeToClose;
     }
   }
 

test/test.js

@@ -107,7 +107,7 @@ describe('Puppeteer', function() {
       const options = Object.assign({userDataDir}, defaultBrowserOptions);
       const browser = await puppeteer.launch(options);
       expect(fs.readdirSync(userDataDir).length).toBeGreaterThan(0);
-      browser.close();
+      await browser.close();
       expect(fs.readdirSync(userDataDir).length).toBeGreaterThan(0);
       rm(userDataDir);
     }));
@@ -117,7 +117,7 @@ describe('Puppeteer', function() {
       options.args = [`--user-data-dir=${userDataDir}`].concat(options.args);
       const browser = await puppeteer.launch(options);
       expect(fs.readdirSync(userDataDir).length).toBeGreaterThan(0);
-      browser.close();
+      await browser.close();
       expect(fs.readdirSync(userDataDir).length).toBeGreaterThan(0);
       rm(userDataDir);
     }));

utils/doclint/check_public_api/index.js

@@ -165,9 +165,9 @@ function checkDuplicates(doc) {
     classes.add(cls.name);
     const members = new Set();
     for (const member of cls.membersArray) {
-      if (members.has(member.name))
-        errors.push(`Duplicate declaration of method ${cls.name}.${member.name}()`);
-      members.add(member.name);
+      if (members.has(member.type + ' ' + member.name))
+        errors.push(`Duplicate declaration of ${member.type} ${cls.name}.${member.name}()`);
+      members.add(member.type + ' ' + member.name);
       const args = new Set();
       for (const arg of member.argsArray) {
         if (args.has(arg.name))