refactor(firefox): migrate onto ExecutionContext events

experimental/puppeteer-firefox/lib/ExecutionContext.js

@@ -15,10 +15,10 @@ class ExecutionContext {
 
   async evaluateHandle(pageFunction, ...args) {
     if (helper.isString(pageFunction)) {
-      const payload = await this._session.send('Page.evaluate', {
-        script: pageFunction,
+      const payload = await this._session.send('Runtime.evaluate', {
+        expression: pageFunction,
         executionContextId: this._executionContextId,
-      });
+      }).catch(rewriteError);
       return createHandle(this, payload.result, payload.exceptionDetails);
     }
     if (typeof pageFunction !== 'function')
@@ -59,10 +59,10 @@ class ExecutionContext {
         return {unserializableValue: 'NaN'};
       return {value: arg};
     });
-    let payload = null;
+    let callFunctionPromise;
     try {
-      payload = await this._session.send('Page.evaluate', {
-        functionText,
+      callFunctionPromise = this._session.send('Runtime.callFunction', {
+        functionDeclaration: functionText,
         args,
         executionContextId: this._executionContextId
       });
@@ -71,7 +71,14 @@ class ExecutionContext {
         err.message += ' Are you passing a nested JSHandle?';
       throw err;
     }
+    const payload = await callFunctionPromise.catch(rewriteError);
     return createHandle(this, payload.result, payload.exceptionDetails);
+
+    function rewriteError(error) {
+      if (error.message.includes('Failed to find execution context with id'))
+        throw new Error('Execution context was destroyed, most likely because of a navigation.');
+      throw error;
+    }
   }
 
   frame() {

experimental/puppeteer-firefox/lib/FrameManager.js

@@ -22,15 +22,41 @@ class FrameManager extends EventEmitter {
     this._timeoutSettings = timeoutSettings;
     this._mainFrame = null;
     this._frames = new Map();
+    /** @type {!Map<string, !ExecutionContext>} */
+    this._contextIdToContext = new Map();
     this._eventListeners = [
       helper.addEventListener(this._session, 'Page.eventFired', this._onEventFired.bind(this)),
       helper.addEventListener(this._session, 'Page.frameAttached', this._onFrameAttached.bind(this)),
       helper.addEventListener(this._session, 'Page.frameDetached', this._onFrameDetached.bind(this)),
       helper.addEventListener(this._session, 'Page.navigationCommitted', this._onNavigationCommitted.bind(this)),
       helper.addEventListener(this._session, 'Page.sameDocumentNavigation', this._onSameDocumentNavigation.bind(this)),
+      helper.addEventListener(this._session, 'Runtime.executionContextCreated', this._onExecutionContextCreated.bind(this)),
+      helper.addEventListener(this._session, 'Runtime.executionContextDestroyed', this._onExecutionContextDestroyed.bind(this)),
     ];
   }
 
+  executionContextById(executionContextId) {
+    return this._contextIdToContext.get(executionContextId) || null;
+  }
+
+  _onExecutionContextCreated({executionContextId, auxData}) {
+    const frameId = auxData ? auxData.frameId : null;
+    const frame = this._frames.get(frameId) || null;
+    const context = new ExecutionContext(this._session, frame, executionContextId);
+    if (frame)
+      frame._setContext(context);
+    this._contextIdToContext.set(executionContextId, context);
+  }
+
+  _onExecutionContextDestroyed({executionContextId}) {
+    const context = this._contextIdToContext.get(executionContextId);
+    if (!context)
+      return;
+    this._contextIdToContext.delete(executionContextId);
+    if (context._frame)
+      context._frame._setContext(null);
+  }
+
   frame(frameId) {
     return this._frames.get(frameId);
   }
@@ -122,19 +148,37 @@ class Frame {
     this._name = '';
     /** @type {!Set<!Frame>} */
     this._children = new Set();
-    this._isDetached = false;
+    this._detached = false;
+
 
     this._firedEvents = new Set();
 
     /** @type {!Set<!WaitTask>} */
     this._waitTasks = new Set();
     this._documentPromise = null;
+    this._contextPromise;
+    this._contextResolveCallback = null;
+    this._setContext(null);
+  }
 
-    this._executionContext = new ExecutionContext(this._session, this, this._frameId);
+  _setContext(context) {
+    if (context) {
+      this._contextResolveCallback.call(null, context);
+      this._contextResolveCallback = null;
+      for (const waitTask of this._waitTasks)
+        waitTask.rerun();
+    } else {
+      this._documentPromise = null;
+      this._contextPromise = new Promise(fulfill => {
+        this._contextResolveCallback = fulfill;
+      });
+    }
   }
 
   async executionContext() {
-    return this._executionContext;
+    if (this._detached)
+      throw new Error(`Execution Context is not available in detached frame "${this.url()}" (are you trying to evaluate?)`);
+    return this._contextPromise;
   }
 
   /**
@@ -267,7 +311,7 @@ class Frame {
   _detach() {
     this._parentFrame._children.delete(this);
     this._parentFrame = null;
-    this._isDetached = true;
+    this._detached = true;
     for (const waitTask of this._waitTasks)
       waitTask.terminate(new Error('waitForFunction failed: frame got detached.'));
   }
@@ -438,7 +482,8 @@ class Frame {
   }
 
   async evaluate(pageFunction, ...args) {
-    return this._executionContext.evaluate(pageFunction, ...args);
+    const context = await this.executionContext();
+    return context.evaluate(pageFunction, ...args);
   }
 
   _document() {
@@ -497,7 +542,8 @@ class Frame {
   }
 
   async evaluateHandle(pageFunction, ...args) {
-    return this._executionContext.evaluateHandle(pageFunction, ...args);
+    const context = await this.executionContext();
+    return context.evaluateHandle(pageFunction, ...args);
   }
 
   /**
@@ -636,7 +682,7 @@ class Frame {
   }
 
   isDetached() {
-    return this._isDetached;
+    return this._detached;
   }
 
   childFrames() {

experimental/puppeteer-firefox/lib/JSHandle.js

@@ -59,7 +59,7 @@ class JSHandle {
    * @return {!Promise<Map<string, !JSHandle>>}
    */
   async getProperties() {
-    const response = await this._session.send('Page.getObjectProperties', {
+    const response = await this._session.send('Runtime.getObjectProperties', {
       executionContextId: this._executionContextId,
       objectId: this._objectId,
     });
@@ -85,10 +85,10 @@ class JSHandle {
   async jsonValue() {
     if (!this._objectId)
       return this._deserializeValue(this._protocolValue);
-    const simpleValue = await this._session.send('Page.evaluate', {
+    const simpleValue = await this._session.send('Runtime.callFunction', {
       executionContextId: this._executionContextId,
       returnByValue: true,
-      functionText: (e => e).toString(),
+      functionDeclaration: (e => e).toString(),
       args: [this._protocolValue],
     });
     return this._deserializeValue(simpleValue.result);
@@ -105,9 +105,13 @@ class JSHandle {
     if (!this._objectId)
       return;
     this._disposed = true;
-    await this._session.send('Page.disposeObject', {
+    await this._session.send('Runtime.disposeObject', {
       executionContextId: this._executionContextId,
       objectId: this._objectId,
+    }).catch(error => {
+      // Exceptions might happen in case of a page been navigated or closed.
+      // Swallow these since they are harmless and we don't leak anything in this case.
+      debugError(error);
     });
   }
 }

experimental/puppeteer-firefox/lib/Page.js

@@ -27,6 +27,7 @@ class Page extends EventEmitter {
     await Promise.all([
       session.send('Page.enable'),
       session.send('Network.enable'),
+      session.send('Runtime.enable'),
     ]);
 
     if (defaultViewport)
@@ -139,7 +140,7 @@ class Page extends EventEmitter {
       else
         expression = helper.evaluationString(deliverErrorValue, name, seq, error);
     }
-    this._session.send('Page.evaluate', { script: expression, executionContextId: event.frameId }).catch(debugError);
+    this._session.send('Runtime.evaluate', { expression, executionContextId: event.executionContextId }).catch(debugError);
 
     /**
      * @param {string} name
@@ -651,9 +652,9 @@ class Page extends EventEmitter {
   _onClosed() {
   }
 
-  _onConsole({type, args, frameId, location}) {
-    const frame = this._frameManager.frame(frameId);
-    this.emit(Events.Page.Console, new ConsoleMessage(type, args.map(arg => createHandle(frame._executionContext, arg)), location));
+  _onConsole({type, args, executionContextId, location}) {
+    const context = this._frameManager.executionContextById(executionContextId);
+    this.emit(Events.Page.Console, new ConsoleMessage(type, args.map(arg => createHandle(context, arg)), location));
   }
 
   /**

experimental/puppeteer-firefox/package.json

@@ -9,7 +9,7 @@
     "node": ">=8.9.4"
   },
   "puppeteer": {
-    "firefox_revision": "3ba79216e3c5ae4e85006047cdd93eac4197427d"
+    "firefox_revision": "764023af0aa07d232984dec6bc81d9e904f25ddb"
   },
   "scripts": {
     "install": "node install.js",

test/evaluation.spec.js

@@ -209,7 +209,7 @@ module.exports.addTests = function({testRunner, expect}) {
       const result = await page.evaluate(() => document.execCommand('copy'));
       expect(result).toBe(true);
     });
-    it_fails_ffox('should throw a nice error after a navigation', async({page, server}) => {
+    it('should throw a nice error after a navigation', async({page, server}) => {
       const executionContext = await page.mainFrame().executionContext();
 
       await Promise.all([