diff --git a/lighthouse-core/config/default-config.js b/lighthouse-core/config/default-config.js index d309072f7aa8..e88145c53304 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 38fe383209c8..c80f9d930ef7 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 f7efa385fa02..e3f0200f7a35 100644 --- a/lighthouse-core/gather/gather-runner.js +++ b/lighthouse-core/gather/gather-runner.js @@ -319,6 +319,10 @@ class GatherRunner { await driver.setThrottling(passContext.settings, {useThrottling: false}); 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 = pageLoadError ? null : await driver.getScrollPosition(); + for (const gathererDefn of gatherers) { const gatherer = gathererDefn.instance; const status = { @@ -340,6 +344,7 @@ class GatherRunner { gathererResult.push(artifactPromise); gathererResults[gatherer.name] = gathererResult; await artifactPromise.catch(() => {}); + if (scrollPosition) 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 cdc961b2a1aa..7bbb29a1b57e 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 36ab4f962d67..eaeac50296e7 100644 --- a/lighthouse-core/test/gather/gather-runner-test.js +++ b/lighthouse-core/test/gather/gather-runner-test.js @@ -574,6 +574,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();