Patch targetcreated events (fixes #6) (#7)

* Add semicolon for safety
* Add page creation patch

packages/puppeteer-extra-plugin-anonymize-ua/test/popup.js

@@ -0,0 +1,52 @@
+'use strict'
+
+const { test } = require('ava')
+
+const PUPPETEER_ARGS = ['--no-sandbox', '--disable-setuid-sandbox']
+
+const waitEvent = function (emitter, eventName) {
+  return new Promise(resolve => emitter.once(eventName, resolve))
+}
+
+test.beforeEach(t => {
+  // Make sure we work with pristine modules
+  delete require.cache[require.resolve('puppeteer-extra')]
+  delete require.cache[require.resolve('puppeteer-extra-plugin-anonymize-ua')]
+})
+
+test('known issue: will not remove headless from implicitly created popup pages', async (t) => {
+  const puppeteer = require('puppeteer-extra')
+  puppeteer.use(require('puppeteer-extra-plugin-anonymize-ua')())
+  const browser = await puppeteer.launch({ args: PUPPETEER_ARGS })
+
+  const pages = await Promise.all(
+    [...Array(10)].map(slot => browser.newPage())
+  )
+  for (const page of pages) {
+    // Works
+    const ua = await page.evaluate(() => window.navigator.userAgent)
+    t.true(!ua.includes('HeadlessChrome'))
+
+    // Works
+    await page.goto('about:blank')
+    const ua2 = await page.evaluate(() => window.navigator.userAgent)
+    t.true(!ua2.includes('HeadlessChrome'))
+
+    // Does NOT work:
+    // https://github.com/GoogleChrome/puppeteer/issues/2669
+    page.evaluate(url => window.open(url), 'about:blank')
+    const popupTarget = await waitEvent(browser, 'targetcreated')
+    const popupPage = await popupTarget.page()
+    const ua3 = await popupPage.evaluate(() => window.navigator.userAgent)
+    // Test against the problem until it's fixed
+    t.true(ua3.includes('HeadlessChrome')) // should be: !ua3.includes('HeadlessChrome')
+
+    // Works: The bug only affects newly created popups, subsequent page navigations are fine.
+    await popupPage.goto('about:blank')
+    const ua4 = await page.evaluate(() => window.navigator.userAgent)
+    t.true(!ua4.includes('HeadlessChrome'))
+  }
+
+  await browser.close()
+  t.true(true)
+})

packages/puppeteer-extra-plugin-anonymize-ua/test/stresstest.js

@@ -0,0 +1,89 @@
+'use strict'
+
+const { test } = require('ava')
+
+const PUPPETEER_ARGS = ['--no-sandbox', '--disable-setuid-sandbox']
+
+test.beforeEach(t => {
+  // Make sure we work with pristine modules
+  delete require.cache[require.resolve('puppeteer-extra')]
+  delete require.cache[require.resolve('puppeteer-extra-plugin-anonymize-ua')]
+})
+
+test('will remove headless from the user-agent on multiple browsers', async (t) => {
+  const puppeteer = require('puppeteer-extra')
+  puppeteer.use(require('puppeteer-extra-plugin-anonymize-ua')())
+  const browser = await puppeteer.launch({ args: PUPPETEER_ARGS })
+
+  const browsers = await Promise.all(
+    [...Array(5)].map(slot => puppeteer.launch({ args: PUPPETEER_ARGS }))
+  )
+  for (const browser of browsers) {
+    const page = await browser.newPage()
+    const ua = await page.evaluate(() => window.navigator.userAgent)
+    t.true(ua.includes('Windows NT 10.0'))
+    t.true(!ua.includes('HeadlessChrome'))
+  }
+
+  await browser.close()
+  t.true(true)
+})
+
+test('will remove headless from the user-agent on many pages', async (t) => {
+  const puppeteer = require('puppeteer-extra')
+  puppeteer.use(require('puppeteer-extra-plugin-anonymize-ua')())
+  const browser = await puppeteer.launch({ args: PUPPETEER_ARGS })
+
+  const pages = await Promise.all(
+    [...Array(30)].map(slot => browser.newPage())
+  )
+  for (const page of pages) {
+    const ua = await page.evaluate(() => window.navigator.userAgent)
+    t.true(ua.includes('Windows NT 10.0'))
+    t.true(!ua.includes('HeadlessChrome'))
+  }
+
+  await browser.close()
+  t.true(true)
+})
+
+test('will remove headless from the user-agent on many incognito pages', async (t) => {
+  const puppeteer = require('puppeteer-extra')
+  puppeteer.use(require('puppeteer-extra-plugin-anonymize-ua')())
+  const browser = await puppeteer.launch({ args: PUPPETEER_ARGS })
+
+  // Requires puppeteer@next currrently
+  if (browser.createIncognitoBrowserContext) {
+    const contexts = await Promise.all(
+      [...Array(30)].map(slot => browser.createIncognitoBrowserContext())
+    )
+    for (const context of contexts) {
+      const page = await context.newPage()
+      const ua = await page.evaluate(() => window.navigator.userAgent)
+      t.true(ua.includes('Windows NT 10.0'))
+      t.true(!ua.includes('HeadlessChrome'))
+    }
+  }
+
+  await browser.close()
+  t.true(true)
+})
+
+test('will remove headless from the user-agent on many pages in parallel', async (t) => {
+  const puppeteer = require('puppeteer-extra')
+  puppeteer.use(require('puppeteer-extra-plugin-anonymize-ua')())
+  const browser = await puppeteer.launch({ args: PUPPETEER_ARGS })
+
+  const testCase = async () => {
+    const page = await browser.newPage()
+    const ua = await page.evaluate(() => window.navigator.userAgent)
+    t.true(ua.includes('Windows NT 10.0'))
+    t.true(!ua.includes('HeadlessChrome'))
+  }
+  await Promise.all(
+    [...Array(30)].map(slot => testCase())
+  )
+
+  await browser.close()
+  t.true(true)
+})

packages/puppeteer-extra-plugin/index.js

@@ -37,7 +37,7 @@ const merge = require('merge-deep')
  * const puppeteer = require('puppeteer-extra')
  * puppeteer.use(require('./hello-world-plugin')())
  *
- * (async () => {
+ * ;(async () => {
  *   const browser = await puppeteer.launch({headless: false})
  *   const page = await browser.newPage()
  *   await page.goto('http://example.com', {waitUntil: 'domcontentloaded'})

packages/puppeteer-extra-plugin/readme.md

@@ -73,7 +73,7 @@ module.exports = function (pluginConfig) { return new Plugin(pluginConfig) }
 const puppeteer = require('puppeteer-extra')
 puppeteer.use(require('./hello-world-plugin')())
 
-(async () => {
+;(async () => {
   const browser = await puppeteer.launch({headless: false})
   const page = await browser.newPage()
   await page.goto('http://example.com', {waitUntil: 'domcontentloaded'})

packages/puppeteer-extra/index.js

@@ -45,7 +45,7 @@ class PuppeteerExtra {
     this._plugins = []
 
     // Ensure there are certain properties (e.g. the `options.args` array)
-    this._defaultOptions = { args: [] }
+    this._defaultLaunchOptions = { args: [] }
   }
 
   /**
@@ -92,7 +92,7 @@ class PuppeteerExtra {
    * @return {Puppeteer.Browser}
    */
   async launch (options = {}) {
-    options = merge(this._defaultOptions, options)
+    options = merge(this._defaultLaunchOptions, options)
     this.resolvePluginDependencies()
     this.orderPlugins()
 
@@ -102,51 +102,46 @@ class PuppeteerExtra {
     this.checkPluginRequirements(options)
 
     const browser = await Puppeteer.launch(options)
-    this._delayTargetCreationMethods(browser)
+    this._patchPageCreationMethods(browser)
     await this.callPlugins('_bindBrowserEvents', browser, options)
 
     return browser
   }
 
   /**
-   * Delays an arbitrary async function to resolve by a specified number of milliseconds.
+   * Patch page creation methods (both regular and incognito contexts).
    *
-   * Unfortunately we currently need to add a minimal delay to methods that can create
-   * a new target, as there's a small chance that event listeners are not ready
-   * yet when the first target is created. :-/
+   * Unfortunately it's possible that the `targetcreated` events are not triggered
+   * early enough for listeners (e.g. plugins using `onPageCreated`) to be able to
+   * modify the page instance (e.g. user-agent) before the browser request occurs.
+   *
+   * This only affects the first request of a newly created page target.
+   *
+   * As a workaround I've noticed that navigating to `about:blank` (again),
+   * right after a page has been created reliably fixes this issue and adds
+   * no noticable delay or side-effects.
+   *
+   * This problem is not specific to `puppeteer-extra` but default Puppeteer behaviour.
+   *
+   * Note: This patch only fixes explicitly created pages, implicitly created ones
+   * (e.g. through `window.open`) are still subject to this issue. I didn't find a
+   * reliable mitigation for implicitly created pages yet.
    *
    * Puppeteer issues:
+   * https://github.com/GoogleChrome/puppeteer/issues/2669
    * https://github.com/GoogleChrome/puppeteer/issues/386#issuecomment-343059315
    * https://github.com/GoogleChrome/puppeteer/issues/1378#issue-273733905
    *
-   * @param  {Number} timeout - Delay in milliseconds
-   * @param  {Function} method - The async method to use
-   * @param  {any} context - the this to use
-   * @return {Promise}
    * @private
    */
-  _delayAsync (timeout = 10, method, context = this) {
-    const delay = ms => new Promise(resolve => setTimeout(resolve, ms))
-    return async () => {
-      const result = await method.apply(context, arguments)
-      await delay(timeout)
-      return result
-    }
-  }
-
-  /**
-   * @see _delayAsync
-   * @private
-   */
-  _delayTargetCreationMethods (browser) {
-    browser.newPage = this._delayAsync(50, browser.newPage, browser)
-
-    browser._createIncognitoBrowserContext = browser.createIncognitoBrowserContext
-    browser.createIncognitoBrowserContext = async () => {
-      const context = await browser._createIncognitoBrowserContext.apply(browser, arguments)
-      context.newPage = this._delayAsync(50, context.newPage, context)
-      return context
-    }
+  _patchPageCreationMethods (browser) {
+    browser._createPageInContext = (function (originalMethod, context) {
+      return async (contextId) => {
+        const page = await originalMethod.apply(context, arguments)
+        await page.goto('about:blank')
+        return page
+      }
+    })(browser._createPageInContext, browser)
   }
 
   /**