misc(fix master): tmp revert of scroll change in #8625 (#9059)

This reverts commit 6bc4f89.
GoogleChrome · May 25, 2019 · 24ea72a · 24ea72a
1 parent 6bc4f89
commit 24ea72a
Show file tree

Hide file tree

Showing 6 changed files with 2 additions and 95 deletions.
diff --git a/lighthouse-core/config/default-config.js b/lighthouse-core/config/default-config.js
@@ -135,6 +135,7 @@ 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
@@ -86,16 +86,6 @@ 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));
 
     /**
@@ -474,20 +464,7 @@ class Driver {
    */
   async evaluateAsync(expression, options = {}) {
     const contextId = options.useIsolation ? await this._getOrCreateIsolatedContextId() : undefined;
-
-    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;
-    }
+    return this._evaluateInContext(expression, contextId);
   }
 
   /**
@@ -1347,22 +1324,6 @@ class Driver {
     return flattenedDocument.nodes ? flattenedDocument.nodes : [];
   }
 
-  /**
-   * @param {{x: number, y: number}} position
-   * @return {Promise<void>}
-   */
-  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<void>}

diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js
@@ -324,10 +324,6 @@ class GatherRunner {
     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 = pageLoadError ? null : await driver.getScrollPosition();
-
     for (const gathererDefn of gatherers) {
       const gatherer = gathererDefn.instance;
       const status = {
@@ -345,7 +341,6 @@ 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
@@ -317,19 +317,6 @@ 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
@@ -6,8 +6,6 @@
 'use strict';
 
 function makeFakeDriver({protocolGetVersionResponse}) {
-  let scrollPosition = {x: 0, y: 0};
-
   return {
     getBrowserVersion() {
       return Promise.resolve(Object.assign({}, protocolGetVersionResponse, {milestone: 71}));
@@ -59,13 +57,6 @@ 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
@@ -588,34 +588,6 @@ 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();