Skip to content

Commit

Permalink
fix(Frame): postpone evaluations until execution context gets created
Browse files Browse the repository at this point in the history
In Blink, frames don't necesserily have execution context all the time.
DevTools Protocol precisely reports this situation, which results in
Puppeteer's frame.executionContext() being null occasionally.

However, from puppeteer point of view every frame will have at least a
default executions context, sooner or later:
- frame's execution context might be created naturally to run frame's
  javascript
- if frame has no javascript, devtools protocol will issue execution
  context creation

This patch builds up on this assumption and introduces a PendingContext:
- Initially, frame is created with a PendingContext instead of a real
  context
- When frame clients request evaluations, PendingContext records
  evaluation requests
- When a real execution context is created, the PendingContext runs all
  the postponed evaluations in the real context

Fixes puppeteer#827, puppeteer#1325
  • Loading branch information
aslushnikov committed Nov 18, 2017
1 parent 48ccf1e commit b2c8fee
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 8 deletions.
44 changes: 43 additions & 1 deletion lib/ExecutionContext.js
Expand Up @@ -110,6 +110,48 @@ class ExecutionContext {
}
}

class PendingContext {
constructor() {
this._invocations = [];
}

/**
* @return {!Object}
*/
createProxy() {
const invocations = this._invocations;
return new Proxy({}, {
get: function(target, propertyName) {
const property = Reflect.get(ExecutionContext.prototype, propertyName);
if (property && typeof property === 'function')
return recordInvocation.bind(null, propertyName);
return property;
}
});

function recordInvocation(methodName, ...args) {
const invocation = { methodName, args };
const promise = new Promise((resolve, reject) => {
invocation.resolve = resolve;
invocation.reject = reject;
});
invocations.push(invocation);
return promise;
}
}

/**
* @param {!ExecutionContext} context
*/
resolvePendingInvocations(context) {
for (const invocation of this._invocations) {
context[invocation.methodName].apply(context, invocation.args)
.then(invocation.resolve, invocation.reject);
}
this._invocations = [];
}
}

class JSHandle {
/**
* @param {!ExecutionContext} context
Expand Down Expand Up @@ -207,4 +249,4 @@ class JSHandle {
}

helper.tracePublicAPI(JSHandle);
module.exports = {ExecutionContext, JSHandle};
module.exports = {ExecutionContext, PendingContext, JSHandle};
60 changes: 54 additions & 6 deletions lib/FrameManager.js
Expand Up @@ -17,7 +17,7 @@
const fs = require('fs');
const EventEmitter = require('events');
const {helper} = require('./helper');
const {ExecutionContext, JSHandle} = require('./ExecutionContext');
const {ExecutionContext, PendingContext, JSHandle} = require('./ExecutionContext');
const ElementHandle = require('./ElementHandle');

const readFileAsync = helper.promisify(fs.readFile);
Expand All @@ -36,11 +36,15 @@ class FrameManager extends EventEmitter {
this._frames = new Map();
/** @type {!Map<string, !ExecutionContext>} */
this._contextIdToContext = new Map();
/** @type {!Map<!ExecutionContext, !Frame>} */
this._contextToFrame = new Map();

this._client.on('Page.frameAttached', event => this._onFrameAttached(event.frameId, event.parentFrameId));
this._client.on('Page.frameNavigated', event => this._onFrameNavigated(event.frame));
this._client.on('Page.frameDetached', event => this._onFrameDetached(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());
this._client.on('Page.lifecycleEvent', event => this._onLifecycleEvent(event));

this._handleFrameTree(frameTree);
Expand Down Expand Up @@ -147,17 +151,43 @@ class FrameManager extends EventEmitter {
const context = new ExecutionContext(this._client, contextPayload.id, this.createJSHandle.bind(this, contextPayload.id));
this._contextIdToContext.set(contextPayload.id, context);

const frameId = contextPayload.auxData && contextPayload.auxData.isDefault ? contextPayload.auxData.frameId : null;
const frame = this._frames.get(frameId);
const auxData = contextPayload.auxData || {};
const frame = auxData.frameId ? this._frames.get(auxData.frameId) : null;
if (!frame)
return;
frame._context = context;
this._contextToFrame.set(context, frame);
if (!auxData.isDefault)
return;
frame._setDefaultContext(context);
for (const waitTask of frame._waitTasks)
waitTask.rerun();
}

_onExecutionContextDestroyed(contextPayload) {
this._contextIdToContext.delete(contextPayload.id);
/**
* @param {!ExecutionContext} context
*/
_removeContext(context) {
const frame = this._contextToFrame.get(context);
this._contextToFrame.delete(context);
if (frame && frame._context === context)
frame._setDefaultContext(null);
}

/**
* @param {string} executionContextId
*/
_onExecutionContextDestroyed(executionContextId) {
const context = this._contextIdToContext.get(executionContextId);
if (!context)
return;
this._contextIdToContext.delete(executionContextId);
this._removeContext(context);
}

_onExecutionContextsCleared() {
for (const context of this._contextIdToContext.values())
this._removeContext(context);
this._contextIdToContext.clear();
}

/**
Expand Down Expand Up @@ -208,7 +238,11 @@ class Frame {
this._parentFrame = parentFrame;
this._url = '';
this._id = frameId;

this._context = null;
this._pendingContext = null;
this._setDefaultContext(null);

/** @type {!Set<!WaitTask>} */
this._waitTasks = new Set();
this._loaderId = '';
Expand All @@ -221,6 +255,20 @@ class Frame {
this._parentFrame._childFrames.add(this);
}

/**
* @param {?ExecutionContext} context
*/
_setDefaultContext(context) {
if (context) {
this._pendingContext.resolvePendingInvocations(context);
this._pendingContext = null;
this._context = context;
} else {
this._pendingContext = new PendingContext();
this._context = this._pendingContext.createProxy();
}
}

/**
* @return {!ExecutionContext}
*/
Expand Down
8 changes: 8 additions & 0 deletions test/test.js
Expand Up @@ -311,6 +311,14 @@ describe('Page', function() {
const result = await page.evaluate(() => Promise.resolve(8 * 7));
expect(result).toBe(56);
}));
it('should work right after framenavigated', SX(async function() {
let frameEvaluation = null;
page.on('framenavigated', async frame => {
frameEvaluation = frame.evaluate(() => 6 * 7);
});
await page.goto(EMPTY_PAGE);
expect(await frameEvaluation).toBe(42);
}));
it('should work from-inside an exposed function', SX(async function() {
// Setup inpage callback, which calls Page.evaluate
await page.exposeFunction('callController', async function(a, b) {
Expand Down
2 changes: 1 addition & 1 deletion utils/doclint/check_public_api/index.js
Expand Up @@ -28,7 +28,7 @@ const EXCLUDE_CLASSES = new Set([
'Multimap',
'NavigatorWatcher',
'NetworkManager',
'ProxyStream',
'PendingContext',
'Session',
'TaskQueue',
'WaitTask',
Expand Down

0 comments on commit b2c8fee

Please sign in to comment.