Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core(fr): cleanup driver on run completion #13062

Merged
merged 5 commits into from
Sep 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions lighthouse-core/fraggle-rock/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const defaultSession = {
addSessionAttachedListener: throwNotConnectedFn,
removeSessionAttachedListener: throwNotConnectedFn,
sendCommand: throwNotConnectedFn,
dispose: throwNotConnectedFn,
};

/** @implements {LH.Gatherer.FRTransitionalDriver} */
Expand Down Expand Up @@ -72,6 +73,12 @@ class Driver {
this._executionContext = new ExecutionContext(this._session);
this._fetcher = new Fetcher(this._session, this._executionContext);
}

/** @return {Promise<void>} */
async disconnect() {
if (!this._session) return;
await this._session.dispose();
}
}

module.exports = Driver;
2 changes: 2 additions & 0 deletions lighthouse-core/fraggle-rock/gather/navigation-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,8 @@ async function _navigations({driver, config, requestedUrl, baseArtifacts, comput
async function _cleanup({requestedUrl, driver, config}) {
const didResetStorage = !config.settings.disableStorageReset;
if (didResetStorage) await storage.clearDataForOrigin(driver.defaultSession, requestedUrl);

await driver.disconnect();
}

/**
Expand Down
9 changes: 9 additions & 0 deletions lighthouse-core/fraggle-rock/gather/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,15 @@ class ProtocolSession {
if (timeout) clearTimeout(timeout);
});
}

/**
* Disposes of a session so that it can no longer talk to Chrome.
* @return {Promise<void>}
*/
async dispose() {
this._session.removeAllListeners();
await this._session.detach();
}
}

module.exports = ProtocolSession;
2 changes: 2 additions & 0 deletions lighthouse-core/fraggle-rock/gather/snapshot-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ async function snapshot(options) {
settings: config.settings,
});

await driver.disconnect();

const artifacts = await awaitArtifacts(artifactState);
return finalizeArtifacts(baseArtifacts, artifacts);
},
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/fraggle-rock/gather/timespan-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ async function startTimespan(options) {
await collectPhaseArtifacts({phase: 'stopSensitiveInstrumentation', ...phaseOptions});
await collectPhaseArtifacts({phase: 'stopInstrumentation', ...phaseOptions});
await collectPhaseArtifacts({phase: 'getArtifact', ...phaseOptions});
await driver.disconnect();

const artifacts = await awaitArtifacts(artifactState);
return finalizeArtifacts(baseArtifacts, artifacts);
Expand Down
5 changes: 5 additions & 0 deletions lighthouse-core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ class Driver {
return this._connection.disconnect();
}

/** @return {Promise<void>} */
dispose() {
return this.disconnect();
}

/**
* Get the browser WebSocket endpoint for devtools protocol clients like Puppeteer.
* Only works with WebSocket connection, not extension or devtools.
Expand Down
13 changes: 13 additions & 0 deletions lighthouse-core/test/fraggle-rock/gather/driver-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,16 @@ describe('.fetcher', () => {
expect(driver.fetcher).toBeTruthy();
});
});

describe('.disconnect', () => {
it('should do nothing if called before connect', async () => {
await driver.disconnect();
});

it('should invoke session dispose', async () => {
await driver.connect();
const dispose = driver.defaultSession.dispose = jest.fn();
await driver.disconnect();
expect(dispose).toHaveBeenCalled();
});
});
2 changes: 2 additions & 0 deletions lighthouse-core/test/fraggle-rock/gather/mock-driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ function createMockSession() {
removeProtocolMessageListener: jest.fn(),
addSessionAttachedListener: createMockOnFn(),
removeSessionAttachedListener: jest.fn(),
dispose: jest.fn(),

/** @return {LH.Gatherer.FRProtocolSession} */
asSession() {
Expand Down Expand Up @@ -131,6 +132,7 @@ function createMockDriver() {
url: () => page.url(),
defaultSession: session,
connect: jest.fn(),
disconnect: jest.fn(),
executionContext: context.asExecutionContext(),

/** @return {Driver} */
Expand Down
14 changes: 14 additions & 0 deletions lighthouse-core/test/fraggle-rock/gather/session-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,20 @@ describe('ProtocolSession', () => {
});
}

describe('.dispose', () => {
it('should detach from the session', async () => {
const detach = jest.fn();
const removeAllListeners = jest.fn();
// @ts-expect-error - we want to use a more limited test.
puppeteerSession = {detach, emit: jest.fn(), removeAllListeners};
session = new ProtocolSession(puppeteerSession);

await session.dispose();
expect(detach).toHaveBeenCalled();
expect(removeAllListeners).toHaveBeenCalled();
});
});

describe('.addProtocolMessageListener', () => {
it('should listen for any event', () => {
// @ts-expect-error - we want to use a more limited test of a real event emitter.
Expand Down
1 change: 1 addition & 0 deletions types/gatherer.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ declare module Gatherer {
removeSessionAttachedListener(callback: (session: FRProtocolSession) => void): void
off<TEvent extends keyof LH.CrdpEvents>(event: TEvent, callback: (...args: LH.CrdpEvents[TEvent]) => void): void;
sendCommand<TMethod extends keyof LH.CrdpCommands>(method: TMethod, ...params: LH.CrdpCommands[TMethod]['paramsType']): Promise<LH.CrdpCommands[TMethod]['returnType']>;
dispose(): Promise<void>;
}

/** The limited driver interface shared between pre and post Fraggle Rock Lighthouse. */
Expand Down