Skip to content

Commit

Permalink
⬆ Upgrade Chromium 78 to 87 (#151)
Browse files Browse the repository at this point in the history 
* ⬆ Upgrade Chromium 78 to 87

* 🔥 Remove old release-notes script

* ✨ Add script for finding Chromium revisions

* ✅ Update test expectation

* 🔊 Improve discovery logs

* 🏗 Unprivate properties for testability

* ✅ Update page crash test

Scripts can no longer navigate to the chrome crash screen to trigger a crash. Instead, private
properties were un-privated so the test can reach in to trigger a crash.
  • Loading branch information
Wil Wilsman authored Feb 1, 2021
1 parent 3903c18 commit 19f0d05
Show file tree
Hide file tree
Showing 10 changed files with 239 additions and 59 deletions.
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
"build": "lerna run build --stream",
"build:watch": "lerna run build --stream -- --watch",
"bump-version": "lerna version --no-git-tag-version --no-push",
"chromium-revision": "./scripts/chromium-revision",
"clean": "rm -rf packages/**/{dist,.nyc_output,coverage,oclif.manifest.json}",
"release-notes": "./scripts/release-notes",
"lint": "eslint --ignore-path .gitignore .",
"readme": "lerna run --parallel readme",
"test": "lerna run --stream --concurrency=1 test -- --colors",
Expand Down
32 changes: 15 additions & 17 deletions packages/core/src/discovery/browser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ import install from '../utils/install-browser';
import Page from './page';

export default class Browser extends EventEmitter {
log = logger('core:discovery:browser');
log = logger('core:browser');
pages = new Map();
closed = false;

#pages = new Map();
#callbacks = new Map();
#closed = false;
#lastid = 0;

defaultArgs = [
Expand Down Expand Up @@ -96,8 +96,8 @@ export default class Browser extends EventEmitter {
}

async close() {
if (this.#closed) return;
this.#closed = true;
if (this.closed) return;
this.closed = true;

// reject any pending callbacks
for (let callback of this.#callbacks.values()) {
Expand All @@ -107,13 +107,13 @@ export default class Browser extends EventEmitter {
}

// trigger rejecting pending page callbacks
for (let page of this.#pages.values()) {
for (let page of this.pages.values()) {
page._handleClose();
}

// clear callback and page references
this.#callbacks.clear();
this.#pages.clear();
this.pages.clear();

// no executable means the browser never launched
/* istanbul ignore next: sanity */
Expand Down Expand Up @@ -154,7 +154,7 @@ export default class Browser extends EventEmitter {
// create and attach to a new page target returning the resulting page instance
let { targetId } = await this.send('Target.createTarget', { url: 'about:blank' });
let { sessionId } = await this.send('Target.attachToTarget', { targetId, flatten: true });
return this.#pages.get(sessionId).init({ meta });
return this.pages.get(sessionId).init({ meta });
}

async send(method, params) {
Expand All @@ -180,10 +180,7 @@ export default class Browser extends EventEmitter {
// stderr and resolves when it emits the devtools protocol address or rejects if the process
// exits for any reason or if the address does not appear after the timeout.
async address(timeout = 30000) {
/* istanbul ignore next: this is not called twice but might be in the future */
if (this._address) return this._address;

this._address = await new Promise((resolve, reject) => {
this._address ||= await new Promise((resolve, reject) => {
let stderr = '';

let handleData = chunk => {
Expand Down Expand Up @@ -229,17 +226,17 @@ export default class Browser extends EventEmitter {

if (data.method === 'Target.attachedToTarget') {
// create a new page reference when attached to a target
this.#pages.set(data.params.sessionId, new Page(this, data));
this.pages.set(data.params.sessionId, new Page(this, data));
} else if (data.method === 'Target.detachedFromTarget') {
// remove the old page reference when detached from a target
let page = this.#pages.get(data.params.sessionId);
this.#pages.delete(data.params.sessionId);
let page = this.pages.get(data.params.sessionId);
this.pages.delete(data.params.sessionId);
page?._handleClose();
}

if (data.sessionId) {
// message was for a specific page that sent it
let page = this.#pages.get(data.sessionId);
let page = this.pages.get(data.sessionId);
page?._handleMessage(data);
} else if (data.id && this.#callbacks.has(data.id)) {
// resolve or reject a pending promise created with #send()
Expand All @@ -250,7 +247,8 @@ export default class Browser extends EventEmitter {
* currently does not happen during asset discovery but it's here just in case */
if (data.error) {
callback.reject(Object.assign(callback.error, {
message: `Protocol error (${callback.method}): ${data.error.message} ${data.error.data}`
message: `Protocol error (${callback.method}): ${data.error.message}` +
('data' in data.error ? `: ${data.error.data}` : '')
}));
} else {
callback.resolve(data.result);
Expand Down
22 changes: 11 additions & 11 deletions packages/core/src/discovery/discoverer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ const ALLOWED_STATUSES = [200, 201, 301, 302, 304, 307, 308];
// additional allowed hostnames are defined. Captured resources are cached so future requests
// resolve much quicker and snapshots can share cached resources.
export default class PercyDiscoverer {
queue = null;
browser = null;
log = logger('core:discovery');

#queue = null
#browser = null
#cache = new Map()
#cache = new Map();

constructor({
// asset discovery concurrency
Expand All @@ -31,8 +31,8 @@ export default class PercyDiscoverer {
// browser launch options
launchOptions
}) {
this.#queue = new Queue(concurrency);
this.#browser = new Browser();
this.queue = new Queue(concurrency);
this.browser = new Browser();

Object.assign(this, {
allowedHostnames,
Expand All @@ -44,18 +44,18 @@ export default class PercyDiscoverer {

// Installs the browser executable if necessary then launches and connects to a browser process.
async launch() {
await this.#browser.launch(this.launchOptions);
await this.browser.launch(this.launchOptions);
}

// Returns true or false when the browser is connected.
isConnected() {
return this.#browser.isConnected();
return this.browser.isConnected();
}

// Clears any unstarted discovery tasks and closes the browser.
async close() {
this.#queue.clear();
await this.#browser.close();
this.queue.clear();
await this.browser.close();
}

// Returns a new browser page.
Expand All @@ -71,7 +71,7 @@ export default class PercyDiscoverer {
width = 1280,
meta
}) {
let page = await this.#browser.page({ meta });
let page = await this.browser.page({ meta });
page.network.timeout = this.networkIdleTimeout;
page.network.authorization = authorization;

Expand Down Expand Up @@ -102,7 +102,7 @@ export default class PercyDiscoverer {
assert(this.isConnected(), 'Browser not connected');

// discover assets concurrently
return this.#queue.push(async () => {
return this.queue.push(async () => {
this.log.debug(`Discovering resources @${width}px for ${rootUrl}`, { ...meta, url: rootUrl });
let page;

Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/discovery/network.js
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import logger from '@percy/logger';
import waitFor from '../utils/wait-for';

// The Interceptor class creates common handlers for dealing with intercepting asset requests
Expand All @@ -8,6 +9,8 @@ export default class Network {
#intercepts = new Map();
#authentications = new Set();

log = logger('core:network');

constructor(page) {
this.page = page;
this.page.on('Fetch.authRequired', this._handleAuthRequired);
Expand All @@ -31,6 +34,8 @@ export default class Network {

// Resolves after the timeout when there are no more in-flight requests.
async idle(timeout = this.timeout) {
this.log.debug(`Wait for ${timeout}ms idle`, this.page.meta);

await waitFor(() => {
if (this.page.closedReason) {
throw new Error(`Network error: ${this.page.closedReason}`);
Expand Down
35 changes: 19 additions & 16 deletions packages/core/src/discovery/page.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ export default class Page extends EventEmitter {
#targetId = null;
#frameId = null;
#contextId = null;
closedReason = null;

#callbacks = new Map();
#lifecycle = new Set();

closedReason = null;
log = logger('core:page');

constructor(browser, { params }) {
Expand Down Expand Up @@ -70,14 +70,14 @@ export default class Page extends EventEmitter {
waitForTimeout,
waitForSelector
} = {}) {
let handleNavigate = ({ frame }) => {
this.log.debug('Handle page navigation', { ...this.meta, frame });
let handleNavigate = event => {
/* istanbul ignore next: sanity check */
if (this.#frameId === frame.id) handleNavigate.done = true;
if (this.#frameId === event.frame.id) handleNavigate.done = true;
};

try {
this.once('Page.frameNavigated', handleNavigate);
this.log.debug(`Navigate to: ${url}`, this.meta);

// trigger navigation and handle error responses
let navigate = this.send('Page.navigate', { url })
Expand All @@ -98,6 +98,7 @@ export default class Page extends EventEmitter {
});
}

this.log.debug('Page navigated', this.meta);
// wait for the network to idle
await this.network.idle();

Expand Down Expand Up @@ -146,8 +147,6 @@ export default class Page extends EventEmitter {
) + '}, ...arguments)'
) + '}';

this.log.debug('Evaluate function', this.meta);

// send the call function command
let { result, exceptionDetails } = await this.send('Runtime.callFunctionOn', {
functionDeclaration: fnbody,
Expand Down Expand Up @@ -192,7 +191,9 @@ export default class Page extends EventEmitter {

if (data.error) {
callback.reject(Object.assign(callback.error, {
message: `Protocol error (${callback.method}): ${data.error.message} ${data.error.data}`
message: `Protocol error (${callback.method}): ${data.error.message}` +
/* istanbul ignore next: doesn't always exist so don't print undefined */
('data' in data.error ? `: ${data.error.data}` : '')
}));
} else {
callback.resolve(data.result);
Expand All @@ -204,6 +205,7 @@ export default class Page extends EventEmitter {
}

_handleClose() {
this.log.debug('Page closing', this.meta);
this.closedReason ||= 'Page closed.';

// reject any pending callbacks
Expand All @@ -217,21 +219,22 @@ export default class Page extends EventEmitter {
this.#browser = null;
}

_handleLifecycleEvent = ({ frameId, loaderId, name }) => {
if (this.#frameId === frameId) {
if (name === 'init') this.#lifecycle.clear();
this.#lifecycle.add(name);
_handleLifecycleEvent = event => {
if (this.#frameId === event.frameId) {
if (event.name === 'init') this.#lifecycle.clear();
this.#lifecycle.add(event.name);
}
}

_handleExecutionContextCreated = ({ context }) => {
if (this.#frameId === context.auxData.frameId) {
this.#contextId = context.id;
_handleExecutionContextCreated = event => {
if (this.#frameId === event.context.auxData.frameId) {
this.#contextId = event.context.id;
}
}

_handleExecutionContextDestroyed = ({ executionContextId }) => {
if (this.#contextId === executionContextId) {
_handleExecutionContextDestroyed = event => {
/* istanbul ignore next: context cleared is usually called first */
if (this.#contextId === event.executionContextId) {
this.#contextId = null;
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/percy.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ export default class Percy {
// multiple snapshots can be captured on a single page
for (let { name, execute } of snapshots) {
if (execute) {
this.log.debug('Executing page JS', { ...meta, execute });
this.log.debug('Executing JavaScript', { ...meta, execute });
// accept function bodies as strings
if (typeof execute === 'string') execute = `async execute({ waitFor }) {\n${execute}\n}`;
// execute the provided function
Expand Down
9 changes: 7 additions & 2 deletions packages/core/src/utils/install-browser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,13 @@ const platform = (
export default async function install({
// default discovery browser is chromium
browser = 'Chromium',
// default chromium version is 78.0.3904.x
revision = platform === 'win64' ? /* istanbul ignore next */ '693951' : '693954',
// default chromium version is 87.0.4280.xx
revision = {
linux: '812847',
win64: '812845',
win32: '812822',
darwin: '812851'
}[platform],
// default download directory is in @percy/core root
directory = path.resolve(__dirname, '../../.local-chromium'),
// default download url is dependent on platform and revision
Expand Down
13 changes: 10 additions & 3 deletions packages/core/test/capture.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,8 @@ describe('Percy Capture', () => {
expect(logger.stderr).toEqual([
'[percy] Encountered an error taking snapshot: invalid snapshot\n',
'[percy] Error: Protocol error (Emulation.setDeviceMetricsOverride): ' +
'Invalid parameters width: integer value expected\n'
'Invalid parameters: Failed to deserialize params.width ' +
'- BINDINGS: int32 value expected at position 50\n'
]);
});

Expand Down Expand Up @@ -330,12 +331,18 @@ describe('Percy Capture', () => {
});

it('handles page crashes', async () => {
await percy.capture({
let capture = percy.capture({
name: 'crash snapshot',
url: 'http://localhost:8000',
execute: () => window.location.replace('chrome://crash')
execute: () => new Promise(r => setTimeout(r, 1000))
});

// wait for page creation
await new Promise(r => setTimeout(r, 500));
let [[, page]] = percy.discoverer.browser.pages;
await page.send('Page.crash').catch(() => {});
await capture;

expect(logger.stdout).toEqual([]);
expect(logger.stderr).toEqual([
expect.stringMatching('Encountered an error'),
Expand Down
Loading

0 comments on commit 19f0d05

Please sign in to comment.