fix(Page): Page.waitForNavigation should correctly handle mixed conte…

…nt (#2339)

This patch teaches Page.waitForNavigation to correctly handle navigation
to pages that have frames that might never load.

These frames include:
- frames which main resource loading was aborted due to mixed-content
  error
- frames that artificially called `window.stop()` to interrupt loading
  themselves

Fixes #1936.

lib/FrameManager.js

@@ -41,6 +41,7 @@ class FrameManager extends EventEmitter {
     this._client.on('Page.frameNavigated', event => this._onFrameNavigated(event.frame));
     this._client.on('Page.navigatedWithinDocument', event => this._onFrameNavigatedWithinDocument(event.frameId, event.url));
     this._client.on('Page.frameDetached', event => this._onFrameDetached(event.frameId));
+    this._client.on('Page.frameStoppedLoading', event => this._onFrameStoppedLoading(event.frameId));
     this._client.on('Runtime.executionContextCreated', event => this._onExecutionContextCreated(event.context));
     this._client.on('Runtime.executionContextDestroyed', event => this._onExecutionContextDestroyed(event.executionContextId));
     this._client.on('Runtime.executionContextsCleared', event => this._onExecutionContextsCleared());
@@ -60,6 +61,17 @@ class FrameManager extends EventEmitter {
     this.emit(FrameManager.Events.LifecycleEvent, frame);
   }
 
+  /**
+   * @param {string} frameId
+   */
+  _onFrameStoppedLoading(frameId) {
+    const frame = this._frames.get(frameId);
+    if (!frame)
+      return;
+    frame._onLoadingStopped();
+    this.emit(FrameManager.Events.LifecycleEvent, frame);
+  }
+
   /**
    * @param {!Protocol.Page.FrameTree} frameTree
    */
@@ -784,6 +796,11 @@ class Frame {
     this._lifecycleEvents.add(name);
   }
 
+  _onLoadingStopped() {
+    this._lifecycleEvents.add('DOMContentLoaded');
+    this._lifecycleEvents.add('load');
+  }
+
   _detach() {
     for (const waitTask of this._waitTasks)
       waitTask.terminate(new Error('waitForFunction failed: frame got detached.'));

test/assets/frames/one-frame.html

@@ -0,0 +1 @@
+<iframe src='./frame.html'></iframe>

test/page.spec.js

@@ -674,6 +674,19 @@ module.exports.addTests = function({testRunner, expect, puppeteer, DeviceDescrip
       ]);
       expect(page.url()).toBe(server.PREFIX + '/second.html');
     });
+    it('should work when subframe issues window.stop()', async({page, server}) => {
+      server.setRoute('/frames/style.css', (req, res) => {});
+      const navigationPromise = page.goto(server.PREFIX + '/frames/one-frame.html');
+      const frame = await utils.waitEvent(page, 'frameattached');
+      await new Promise(fulfill => {
+        page.on('framenavigated', f => {
+          if (f === frame)
+            fulfill();
+        });
+      });
+      frame.evaluate(() => window.stop());
+      await navigationPromise;
+    });
   });
 
   describe('Page.goBack', function() {

test/puppeteer.spec.js

@@ -99,6 +99,17 @@ module.exports.addTests = function({testRunner, expect, PROJECT_ROOT, defaultBro
         await page.close();
         await browser.close();
       });
+      it('should work with mixed content', async({server, httpsServer}) => {
+        httpsServer.setRoute('/mixedcontent.html', (req, res) => {
+          res.end(`<iframe src=${server.EMPTY_PAGE}></iframe>`);
+        });
+        const options = Object.assign({ignoreHTTPSErrors: true}, defaultBrowserOptions);
+        const browser = await puppeteer.launch(options);
+        const page = await browser.newPage();
+        await page.goto(httpsServer.PREFIX + '/mixedcontent.html', {waitUntil: 'load'});
+        await page.close();
+        await browser.close();
+      });
       it('should reject all promises when browser is closed', async() => {
         const browser = await puppeteer.launch(defaultBrowserOptions);
         const page = await browser.newPage();