Skip to content
This repository has been archived by the owner on Oct 2, 2021. It is now read-only.

Fix first line bp chrome 69 #352

Merged
merged 3 commits into from
Sep 12, 2018
Merged
Show file tree
Hide file tree
Changes from 2 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
10 changes: 8 additions & 2 deletions src/chrome/breakOnLoadHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@ import { ChromeDebugAdapter } from './chromeDebugAdapter';
import * as ChromeUtils from './chromeUtils';
import * as assert from 'assert';
import { InternalSourceBreakpoint } from './internalSourceBreakpoint';
import { utils } from '..';
import { utils, Version } from '..';

export interface UrlRegexAndFileSet {
urlRegex: string;
fileSet: Set<string>;
}

export class BreakOnLoadHelper {
private _doesDOMInstrumentationRecieveExtraEvent = false;

private _instrumentationBreakpointSet = false;

Expand Down Expand Up @@ -57,6 +58,10 @@ export class BreakOnLoadHelper {
return this._chromeDebugAdapter.scriptsById.get(scriptId).url;
}

public async setBrowserVersion(version: Version): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some comments on why we care about the version number. Please open (and document here) a Chromium bug if you haven't already on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done:
// On version 69 Chrome stopped sending an extra event for DOM Instrumentation: See https://bugs.chromium.org/p/chromium/issues/detail?id=882909
// On Chrome 68 we were relying on that event to make Break on load work on breakpoints on the first line of a file. On Chrome 69 we need an alternative way to make it work.

this._doesDOMInstrumentationRecieveExtraEvent = !version.isAtLeastVersion(69, 0);
}

/**
* Handles the onpaused event.
* Checks if the event is caused by a stopOnEntry breakpoint of using the regex approach, or the paused event due to the Chrome's instrument approach
Expand All @@ -77,7 +82,8 @@ export class BreakOnLoadHelper {
// Now we wait for all the pending breakpoints to be resolved and then continue
await this._chromeDebugAdapter.getBreakpointsResolvedDefer(pausedScriptId).promise;
logger.log('BreakOnLoadHelper: Finished waiting for breakpoints to get resolved.');
return true;
let shouldContinue = this._doesDOMInstrumentationRecieveExtraEvent || await this.handleStopOnEntryBreakpointAndContinue(notification);
return shouldContinue;
}

return false;
Expand Down
8 changes: 4 additions & 4 deletions src/chrome/chromeConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -27,7 +27,7 @@ export interface ITarget {
type: string;
url?: string;
webSocketDebuggerUrl: string;
version: Promise<ProtocolSchema>;
version: Promise<TargetVersions>;
}

export type ITargetFilter = (target: ITarget) => boolean;
Expand Down Expand Up @@ -169,7 +169,7 @@ export class ChromeConnection implements IObservableEvents<IStepStartedEventsEmi
this.api.Runtime.runIfWaitingForDebugger(),
(<any>this.api.Runtime).run()
])
.then(() => { }, e => { });
.then(() => { }, () => { });
}

public close(): void {
Expand All @@ -180,7 +180,7 @@ export class ChromeConnection implements IObservableEvents<IStepStartedEventsEmi
this._socket.on('close', handler);
}

public get version(): Promise<ProtocolSchema> {
public get version(): Promise<TargetVersions> {
return this._attachedTarget.version;
}
}
4 changes: 4 additions & 0 deletions src/chrome/chromeDebugAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,10 @@ export abstract class ChromeDebugAdapter implements IDebugAdapter {
} catch (e) {
// Not supported by older runtimes, ignore it.
}

if (this._breakOnLoadHelper) {
this._breakOnLoadHelper.setBrowserVersion((await this._chromeConnection.version).browser);
}
}
}

Expand Down
41 changes: 29 additions & 12 deletions src/chrome/chromeTargetDiscoveryStrategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand All @@ -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<IStepStartedEventsEmitter> {
private logger: Logger.ILogger;
private telemetry: telemetry.ITelemetryReporter;
Expand Down Expand Up @@ -67,7 +78,7 @@ export class ChromeTargetDiscovery implements ITargetDiscoveryStrategy, IObserva
return this._getMatchingTargets(targets, targetFilter, targetUrl);
}

private async _getVersionData(address: string, port: number): Promise<ProtocolSchema> {
private async _getVersionData(address: string, port: number): Promise<TargetVersions> {

const url = `http://${address}:${port}/json/version`;
this.logger.log(`Getting browser and debug protocol version via ${url}`);
Expand All @@ -78,26 +89,32 @@ 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" : {
"debugProtocolVersion" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" },
"${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<ITarget[]> {
Expand Down
4 changes: 2 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -55,7 +55,7 @@ export {
NullLogger,
executionTimingsReporter,

ProtocolSchema,
Version,

Crdp
};
8 changes: 4 additions & 4 deletions test/chrome/chromeTargetDiscoveryStrategy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -218,23 +218,23 @@ 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));
assert.ok(!schema0dot1.isAtLeastVersion(1, 0));
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));
assert.ok(!schema0dot2.isAtLeastVersion(1, 0));
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));
Expand Down