From 37bc1121ebc23f721c04b577bbf14750b4a3570c Mon Sep 17 00:00:00 2001 From: Diego Geffner Date: Mon, 10 Sep 2018 17:58:59 -0700 Subject: [PATCH] Expand the connection Version to support Browser Version too --- src/chrome/breakOnLoadHelper.ts | 4 +- src/chrome/chromeConnection.ts | 8 ++-- src/chrome/chromeDebugAdapter.ts | 2 +- src/chrome/chromeTargetDiscoveryStrategy.ts | 41 +++++++++++++------ src/index.ts | 4 +- .../chromeTargetDiscoveryStrategy.test.ts | 8 ++-- 6 files changed, 42 insertions(+), 25 deletions(-) diff --git a/src/chrome/breakOnLoadHelper.ts b/src/chrome/breakOnLoadHelper.ts index 6a3c01e23..e1d14f36f 100644 --- a/src/chrome/breakOnLoadHelper.ts +++ b/src/chrome/breakOnLoadHelper.ts @@ -10,7 +10,7 @@ import { ChromeDebugAdapter } from './chromeDebugAdapter'; import * as ChromeUtils from './chromeUtils'; import * as assert from 'assert'; import { InternalSourceBreakpoint } from './internalSourceBreakpoint'; -import { utils, ProtocolSchema } from '..'; +import { utils, Version } from '..'; export interface UrlRegexAndFileSet { urlRegex: string; @@ -20,7 +20,7 @@ export interface UrlRegexAndFileSet { export class BreakOnLoadHelper { private _doesDOMInstrumentationRecieveExtraEvent = false; - public async setTargetVersion(version: ProtocolSchema): Promise { + public async setBrowserVersion(version: Version): Promise { this._doesDOMInstrumentationRecieveExtraEvent = !version.isAtLeastVersion(69, 0); } diff --git a/src/chrome/chromeConnection.ts b/src/chrome/chromeConnection.ts index 122f27d32..0714e09ff 100644 --- a/src/chrome/chromeConnection.ts +++ b/src/chrome/chromeConnection.ts @@ -9,7 +9,7 @@ import { StepProgressEventsEmitter, IObservableEvents, IStepStartedEventsEmitter import * as errors from '../errors'; import * as utils from '../utils'; import { logger } from 'vscode-debugadapter'; -import { ChromeTargetDiscovery, ProtocolSchema } from './chromeTargetDiscoveryStrategy'; +import { ChromeTargetDiscovery, TargetVersions } from './chromeTargetDiscoveryStrategy'; import { Client, LikeSocket } from 'noice-json-rpc'; @@ -27,7 +27,7 @@ export interface ITarget { type: string; url?: string; webSocketDebuggerUrl: string; - version: Promise; + version: Promise; } export type ITargetFilter = (target: ITarget) => boolean; @@ -169,7 +169,7 @@ export class ChromeConnection implements IObservableEventsthis.api.Runtime).run() ]) - .then(() => { }, e => { }); + .then(() => { }, () => { }); } public close(): void { @@ -180,7 +180,7 @@ export class ChromeConnection implements IObservableEvents { + public get version(): Promise { return this._attachedTarget.version; } } \ No newline at end of file diff --git a/src/chrome/chromeDebugAdapter.ts b/src/chrome/chromeDebugAdapter.ts index 669ddd48f..b0148c861 100644 --- a/src/chrome/chromeDebugAdapter.ts +++ b/src/chrome/chromeDebugAdapter.ts @@ -587,7 +587,7 @@ export abstract class ChromeDebugAdapter implements IDebugAdapter { // Not supported by older runtimes, ignore it. } - this._breakOnLoadHelper.setTargetVersion(await this._chromeConnection.version); + this._breakOnLoadHelper.setBrowserVersion((await this._chromeConnection.version).browser); } } diff --git a/src/chrome/chromeTargetDiscoveryStrategy.ts b/src/chrome/chromeTargetDiscoveryStrategy.ts index 7c3b2743e..70902741c 100644 --- a/src/chrome/chromeTargetDiscoveryStrategy.ts +++ b/src/chrome/chromeTargetDiscoveryStrategy.ts @@ -14,9 +14,16 @@ import { ITargetDiscoveryStrategy, ITargetFilter, ITarget } from './chromeConnec import * as nls from 'vscode-nls'; const localize = nls.loadMessageBundle(); -export class ProtocolSchema { - public static unknownVersion(): ProtocolSchema { - return new ProtocolSchema(0, 0); // Using 0.0 will make behave isAtLeastVersion as if this was the oldest possible version +export class Version { + static parse(versionString: string): Version { + const majorAndMinor = versionString.split('.'); + const major = parseInt(majorAndMinor[0], 10); + const minor = parseInt(majorAndMinor[1], 10); + return new Version(major, minor); + } + + public static unknownVersion(): Version { + return new Version(0, 0); // Using 0.0 will make behave isAtLeastVersion as if this was the oldest possible version } constructor(private _major: number, private _minor: number) {} @@ -26,6 +33,10 @@ export class ProtocolSchema { } } +export class TargetVersions { + constructor(public readonly protocol: Version, public readonly browser: Version) {} +} + export class ChromeTargetDiscovery implements ITargetDiscoveryStrategy, IObservableEvents { private logger: Logger.ILogger; private telemetry: telemetry.ITelemetryReporter; @@ -67,7 +78,7 @@ export class ChromeTargetDiscovery implements ITargetDiscoveryStrategy, IObserva return this._getMatchingTargets(targets, targetFilter, targetUrl); } - private async _getVersionData(address: string, port: number): Promise { + private async _getVersionData(address: string, port: number): Promise { const url = `http://${address}:${port}/json/version`; this.logger.log(`Getting browser and debug protocol version via ${url}`); @@ -78,9 +89,10 @@ export class ChromeTargetDiscovery implements ITargetDiscoveryStrategy, IObserva try { if (jsonResponse) { const response = JSON.parse(jsonResponse); - const versionString = response['Protocol-Version'] as string; - this.logger.log(`Got browser version: ${response.Browser}`); - this.logger.log(`Got debug protocol version: ${versionString}`); + const protocolVersionString = response['Protocol-Version'] as string; + const browserWithPrefixVersionString = response['Browser'] as string; + this.logger.log(`Got browser version: ${browserWithPrefixVersionString }`); + this.logger.log(`Got debug protocol version: ${protocolVersionString}`); /* __GDPR__ "targetDebugProtocolVersion" : { @@ -88,16 +100,21 @@ export class ChromeTargetDiscovery implements ITargetDiscoveryStrategy, IObserva "${include}": [ "${DebugCommonProperties}" ] } */ + + const chromePrefix = 'Chrome/'; + let browserVersion = Version.unknownVersion(); + if (browserWithPrefixVersionString.startsWith(chromePrefix)) { + const browserVersionString = browserWithPrefixVersionString.substr(chromePrefix.length); + browserVersion = Version.parse(browserVersionString); + } + this.telemetry.reportEvent('targetDebugProtocolVersion', { debugProtocolVersion: response['Protcol-Version'] }); - const majorAndMinor = versionString.split('.'); - const major = parseInt(majorAndMinor[0], 10); - const minor = parseInt(majorAndMinor[1], 10); - return new ProtocolSchema(major, minor); + return new TargetVersions(Version.parse(protocolVersionString), browserVersion); } } catch (e) { this.logger.log(`Didn't get a valid response for /json/version call. Error: ${e.message}. Response: ${jsonResponse}`); } - return ProtocolSchema.unknownVersion(); + return new TargetVersions(Version.unknownVersion(), Version.unknownVersion()); } private async _getTargets(address: string, port: number): Promise { diff --git a/src/index.ts b/src/index.ts index 033b08cfc..425349b9a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -28,7 +28,7 @@ import { NullLogger } from './nullLogger'; import * as executionTimingsReporter from './executionTimingsReporter'; import { Protocol as Crdp } from 'devtools-protocol'; -import { ProtocolSchema } from './chrome/chromeTargetDiscoveryStrategy'; +import { Version } from './chrome/chromeTargetDiscoveryStrategy'; export { chromeConnection, @@ -55,7 +55,7 @@ export { NullLogger, executionTimingsReporter, - ProtocolSchema, + Version, Crdp }; diff --git a/test/chrome/chromeTargetDiscoveryStrategy.test.ts b/test/chrome/chromeTargetDiscoveryStrategy.test.ts index 60eef5f7e..86074b18c 100644 --- a/test/chrome/chromeTargetDiscoveryStrategy.test.ts +++ b/test/chrome/chromeTargetDiscoveryStrategy.test.ts @@ -10,7 +10,7 @@ import { ITargetDiscoveryStrategy } from '../../src/chrome/chromeConnection'; import { NullLogger } from '../../src/nullLogger'; import { NullTelemetryReporter } from '../../src/telemetry'; -import { ProtocolSchema } from '../../src'; +import { Version } from '../../src'; const MODULE_UNDER_TEST = '../../src/chrome/chromeTargetDiscoveryStrategy'; suite('ChromeTargetDiscoveryStrategy', () => { @@ -218,7 +218,7 @@ suite('ChromeTargetDiscoveryStrategy', () => { }); test('ProtocolSchema return if the version is at least', () => { - const schema0dot1 = new ProtocolSchema(0, 1); + const schema0dot1 = new Version(0, 1); assert.ok(schema0dot1.isAtLeastVersion(0, 0)); assert.ok(schema0dot1.isAtLeastVersion(0, 1)); assert.ok(!schema0dot1.isAtLeastVersion(0, 2)); @@ -226,7 +226,7 @@ suite('ChromeTargetDiscoveryStrategy', () => { assert.ok(!schema0dot1.isAtLeastVersion(1, 1)); assert.ok(!schema0dot1.isAtLeastVersion(1, 2)); - const schema0dot2 = new ProtocolSchema(0, 2); + const schema0dot2 = new Version(0, 2); assert.ok(schema0dot2.isAtLeastVersion(0, 0)); assert.ok(schema0dot2.isAtLeastVersion(0, 1)); assert.ok(schema0dot2.isAtLeastVersion(0, 2)); @@ -234,7 +234,7 @@ suite('ChromeTargetDiscoveryStrategy', () => { assert.ok(!schema0dot2.isAtLeastVersion(1, 1)); assert.ok(!schema0dot2.isAtLeastVersion(1, 2)); - const schema1dot0 = new ProtocolSchema(1, 0); + const schema1dot0 = new Version(1, 0); assert.ok(schema1dot0.isAtLeastVersion(0, 0)); assert.ok(schema1dot0.isAtLeastVersion(0, 1)); assert.ok(schema1dot0.isAtLeastVersion(0, 2));