diff --git a/lighthouse-core/config/default-config.js b/lighthouse-core/config/default-config.js index 33ac027e8e01..fc5f152cbd30 100644 --- a/lighthouse-core/config/default-config.js +++ b/lighthouse-core/config/default-config.js @@ -135,7 +135,6 @@ const defaultConfig = { 'seo/embedded-content', 'seo/robots-txt', 'seo/tap-targets', - // Always run axe last because it scrolls the page down to the bottom 'accessibility', ], }, diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 93f267a8ae85..853cb328ea3b 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -86,6 +86,16 @@ class Driver { this._handleReceivedMessageFromTarget(event, []).catch(this._handleEventError); }); + // We use isolated execution contexts for `evaluateAsync` that can be destroyed through navigation + // and other page actions. Cleanup our relevant bookkeeping as we see those events. + this.on('Runtime.executionContextDestroyed', event => { + if (event.executionContextId === this._isolatedExecutionContextId) { + this._clearIsolatedContextId(); + } + }); + + this.on('Page.frameNavigated', () => this._clearIsolatedContextId()); + connection.on('protocolevent', this._handleProtocolEvent.bind(this)); /** @@ -464,7 +474,20 @@ class Driver { */ async evaluateAsync(expression, options = {}) { const contextId = options.useIsolation ? await this._getOrCreateIsolatedContextId() : undefined; - return this._evaluateInContext(expression, contextId); + + try { + // `await` is not redunant here because we want to `catch` the async errors + return await this._evaluateInContext(expression, contextId); + } catch (err) { + // If we were using isolation and the context disappeared on us, retry one more time. + if (contextId && err.message.includes('Cannot find context')) { + this._clearIsolatedContextId(); + const freshContextId = await this._getOrCreateIsolatedContextId(); + return this._evaluateInContext(expression, freshContextId); + } + + throw err; + } } /** @@ -1324,6 +1347,22 @@ class Driver { return flattenedDocument.nodes ? flattenedDocument.nodes : []; } + /** + * @param {{x: number, y: number}} position + * @return {Promise} + */ + scrollTo(position) { + const scrollExpression = `window.scrollTo(${position.x}, ${position.y})`; + return this.evaluateAsync(scrollExpression, {useIsolation: true}); + } + + /** + * @return {Promise<{x: number, y: number}>} + */ + getScrollPosition() { + return this.evaluateAsync(`({x: window.scrollX, y: window.scrollY})`, {useIsolation: true}); + } + /** * @param {{additionalTraceCategories?: string|null}=} settings * @return {Promise} diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js index 46388f345321..148c4fd33967 100644 --- a/lighthouse-core/gather/gather-runner.js +++ b/lighthouse-core/gather/gather-runner.js @@ -318,12 +318,17 @@ class GatherRunner { * @return {Promise} */ static async afterPass(passContext, loadData, gathererResults) { + const driver = passContext.driver; const config = passContext.passConfig; const gatherers = config.gatherers; const apStatus = {msg: `Running afterPass methods`, id: `lh:gather:afterPass`}; log.time(apStatus, 'verbose'); + // Some gatherers scroll the page which can cause unexpected results for other gatherers. + // We reset the scroll position in between each gatherer. + const scrollPosition = await driver.getScrollPosition(); + for (const gathererDefn of gatherers) { const gatherer = gathererDefn.instance; const status = { @@ -341,6 +346,7 @@ class GatherRunner { gathererResult.push(artifactPromise); gathererResults[gatherer.name] = gathererResult; await artifactPromise.catch(() => {}); + await driver.scrollTo(scrollPosition); log.timeEnd(status); } log.timeEnd(apStatus); diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index d96ee191b1b9..8e3b0d069aee 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -317,6 +317,19 @@ describe('.evaluateAsync', () => { expect.anything() ); }); + + it('recovers from isolation failures', async () => { + connectionStub.sendCommand = createMockSendCommandFn() + .mockResponse('Page.getResourceTree', {frameTree: {frame: {id: 1337}}}) + .mockResponse('Page.createIsolatedWorld', {executionContextId: 9001}) + .mockResponse('Runtime.evaluate', Promise.reject(new Error('Cannot find context'))) + .mockResponse('Page.getResourceTree', {frameTree: {frame: {id: 1337}}}) + .mockResponse('Page.createIsolatedWorld', {executionContextId: 9002}) + .mockResponse('Runtime.evaluate', {result: {value: 'mocked value'}}); + + const value = await driver.evaluateAsync('"magic"', {useIsolation: true}); + expect(value).toEqual('mocked value'); + }); }); describe('.sendCommand', () => { diff --git a/lighthouse-core/test/gather/fake-driver.js b/lighthouse-core/test/gather/fake-driver.js index 1db517772bea..108a40aa6d78 100644 --- a/lighthouse-core/test/gather/fake-driver.js +++ b/lighthouse-core/test/gather/fake-driver.js @@ -6,6 +6,8 @@ 'use strict'; function makeFakeDriver({protocolGetVersionResponse}) { + let scrollPosition = {x: 0, y: 0}; + return { getBrowserVersion() { return Promise.resolve(Object.assign({}, protocolGetVersionResponse, {milestone: 71})); @@ -57,6 +59,13 @@ function makeFakeDriver({protocolGetVersionResponse}) { evaluateAsync() { return Promise.resolve({}); }, + scrollTo(position) { + scrollPosition = position; + return Promise.resolve(); + }, + getScrollPosition() { + return Promise.resolve(scrollPosition); + }, registerPerformanceObserver() { return Promise.resolve(); }, diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js index 6dca2bffd0c3..af2f11ac1cec 100644 --- a/lighthouse-core/test/gather/gather-runner-test.js +++ b/lighthouse-core/test/gather/gather-runner-test.js @@ -374,6 +374,8 @@ describe('GatherRunner', function() { endTrace: asyncFunc, endDevtoolsLog: () => [], getBrowserVersion: async () => ({userAgent: ''}), + getScrollPosition: async () => 1, + scrollTo: async () => {}, }; const passConfig = { passName: 'default', @@ -588,6 +590,34 @@ describe('GatherRunner', function() { }); }); + it('resets scroll position between every gatherer', async () => { + class ScrollMcScrollyGatherer extends TestGatherer { + afterPass(context) { + context.driver.scrollTo({x: 1000, y: 1000}); + } + } + + const url = 'https://example.com'; + const driver = Object.assign({}, fakeDriver); + const scrollToSpy = jest.spyOn(driver, 'scrollTo'); + + const passConfig = { + recordTrace: true, + gatherers: [ + {instance: new ScrollMcScrollyGatherer()}, + {instance: new TestGatherer()}, + ], + }; + + await GatherRunner.afterPass({url, driver, passConfig}, {}, {TestGatherer: []}); + // One time for the afterPass of ScrollMcScrolly, two times for the resets of the two gatherers. + expect(scrollToSpy.mock.calls).toEqual([ + [{x: 1000, y: 1000}], + [{x: 0, y: 0}], + [{x: 0, y: 0}], + ]); + }); + it('does as many passes as are required', () => { const t1 = new TestGatherer(); const t2 = new TestGatherer();