Use Puppeteer's networkidle2 heuristic event for page idleness

[rsimha]

The visual tests have been flaky of late due to errors of this sort:

[18:56:18] ERROR: Error in test amp-story: embed mode 1
[18:56:18] ERROR: Exception thrown: Error: Execution context was destroyed, most likely because of a navigation.
    at rewriteError (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/node_modules/puppeteer/lib/ExecutionContext.js:144:15)
    at process._tickCallback (internal/process/next_tick.js:68:7)
  -- ASYNC --
    at ExecutionContext.<anonymous> (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/node_modules/puppeteer/lib/helper.js:144:27)
    at _documentPromise._contextPromise.then (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/node_modules/puppeteer/lib/FrameManager.js:459:38)
    at process._tickCallback (internal/process/next_tick.js:68:7)
  -- ASYNC --
    at Frame.<anonymous> (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/node_modules/puppeteer/lib/helper.js:144:27)
    at Page.$$ (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/node_modules/puppeteer/lib/Page.js:323:29)
    at Page.<anonymous> (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/node_modules/puppeteer/lib/helper.js:145:23)
    at waitForElementVisibility (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/helpers.js:150:44)
    at waitForLoaderDots (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/helpers.js:122:35)
    at page.goto.then.then (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/index.js:404:19)
    at process._tickCallback (internal/process/next_tick.js:68:7)
[18:56:18] FATAL: Some tests have failed locally.

This PR uses Puppeteer's networkidle2 heuristic event for page idleness, which allows for a small amount of background activity. This should hopefully eliminate the case where puppeteer thinks there was a page navigation.

References:

• Execution context was destroyed, most likely because of a navigation puppeteer/puppeteer#3323
• When to use networkidle2? puppeteer/puppeteer#1552

Attempted fix for #21056

[rsimha]

This is now ready for review.

Visual tests seem to look good: https://travis-ci.org/ampproject/amphtml/jobs/518482686 (I've re-run this job a few times with all green results)

Edit: Hmm. Percy seems to have more visual diffs now: https://percy.io/ampproject/amphtml/builds/1721261. Not sure if it's an old problem that's being exposed by this change, or if it's resulting from the browser not waiting long enough due to this change.

Since it affects the internals of how puppeteer works, I'd rather wait for @danielrozenberg to review this before merging / trying the other approaches in #21056 (comment).

[danielrozenberg]

I don't think this is related at all. This line skips the case where the page has timed out: https://github.com/ampproject/amphtml/pull/21808/files#diff-30e37e8e6a17b1048622b9988ca0252eR394 - I tested both network idle timeout strategies when I wrote this and found that both of them tend to time out, e.g., when there's an iframe that takes a while to load or when the VM is congested.

I still don't understand why there would be page navigations at all, given that each test is executed in a separate tab and tabs are not being reused

[rsimha]

Fair enough. Closing.