feat(Page): add option to run 'beforeunload' when closing the page (#2478)

Today, `page.close()` method doesn't run page's beforeunload listeners.
This way users can be sure that `page.close()` actually closes the
page.

This patch adds an optional `runBeforeUnload` option to the
`page.close()` method that would run beforeunload listeners. Note:
running beforeunload handlers might cancel page closing.

Fixes #2386.

Author: aslushnikov

docs/api.md

@@ -69,7 +69,7 @@
   * [page.bringToFront()](#pagebringtofront)
   * [page.browser()](#pagebrowser)
   * [page.click(selector[, options])](#pageclickselector-options)
-  * [page.close()](#pageclose)
+  * [page.close(options)](#pagecloseoptions)
   * [page.content()](#pagecontent)
   * [page.cookies(...urls)](#pagecookiesurls)
   * [page.coverage](#pagecoverage)
@@ -747,9 +747,18 @@ const [response] = await Promise.all([
 
 Shortcut for [page.mainFrame().click(selector[, options])](#frameclickselector-options).
 
-#### page.close()
+#### page.close(options)
+- `options` <[Object]>
+  - `runBeforeUnload` <[boolean]> Defaults to `false`. Whether to run the
+    [before unload](https://developer.mozilla.org/en-US/docs/Web/Events/beforeunload)
+    page handlers.
 - returns: <[Promise]>
 
+By default, `page.close()` **does not** run beforeunload handlers.
+
+> **NOTE** if `runBeforeUnload` is passed as true, a `beforeunload` dialog might be summoned
+> and should be handled manually via page's ['dialog'](#event-dialog) event.
+
 #### page.content()
 - returns: <[Promise]<[String]>>
 

lib/Page.js

@@ -823,10 +823,18 @@ class Page extends EventEmitter {
     return this.mainFrame().title();
   }
 
-  async close() {
+  /**
+   * @param {!{runBeforeUnload: (boolean|undefined)}=} options
+   */
+  async close(options = {runBeforeUnload: undefined}) {
     console.assert(!!this._client._connection, 'Protocol error: Connection closed. Most likely the page has been closed.');
-    await this._client._connection.send('Target.closeTarget', { targetId: this._target._targetId });
-    await this._target._isClosedPromise;
+    const runBeforeUnload = !!options.runBeforeUnload;
+    if (runBeforeUnload) {
+      await this._client.send('Page.close');
+    } else {
+      await this._client._connection.send('Target.closeTarget', { targetId: this._target._targetId });
+      await this._target._isClosedPromise;
+    }
   }
 
   /**

test/assets/beforeunload.html

@@ -0,0 +1,5 @@
+<script>
+window.addEventListener('beforeunload', event => {
+  event.returnValue = 'Leave?';
+});
+</script>

test/page.spec.js

@@ -40,6 +40,20 @@ module.exports.addTests = function({testRunner, expect, puppeteer, DeviceDescrip
       await newPage.close();
       expect(await browser.pages()).not.toContain(newPage);
     });
+    it('should run beforeunload if asked for', async({browser, server}) => {
+      const newPage = await browser.newPage();
+      await newPage.goto(server.PREFIX + '/beforeunload.html');
+      // We have to interact with a page so that 'beforeunload' handlers
+      // fire.
+      await newPage.click('body');
+      newPage.close({ runBeforeUnload: true });
+      const dialog = await waitEvent(newPage, 'dialog');
+      expect(dialog.type()).toBe('beforeunload');
+      expect(dialog.defaultValue()).toBe('');
+      expect(dialog.message()).toBe('');
+      dialog.accept();
+      await waitEvent(newPage, 'close');
+    });
   });
 
   describe('Page.Events.error', function() {