debug: cleanup how breakpoint decorations are handled

src/vs/workbench/parts/debug/browser/debugEditorModelManager.ts

@@ -11,16 +11,20 @@ import { Constants } from 'vs/editor/common/core/uint';
 import { Range } from 'vs/editor/common/core/range';
 import { IModel, TrackedRangeStickiness, IModelDeltaDecoration, IModelDecorationOptions } from 'vs/editor/common/editorCommon';
 import { IWorkbenchContribution } from 'vs/workbench/common/contributions';
-import { IDebugService, IBreakpoint, IRawBreakpoint, State } from 'vs/workbench/parts/debug/common/debug';
+import { IDebugService, IBreakpoint, State } from 'vs/workbench/parts/debug/common/debug';
 import { IModelService } from 'vs/editor/common/services/modelService';
 import { MarkdownString } from 'vs/base/common/htmlContent';
 
+interface IBreakpointDecoration {
+	decorationId: string;
+	modelId: string;
+	range: Range;
+}
+
 interface IDebugEditorModelData {
 	model: IModel;
 	toDispose: lifecycle.IDisposable[];
-	breakpointDecorationIds: string[];
-	breakpointModelIds: string[];
-	breakpointDecorationsAsMap: Map<string, Range>;
+	breakpointDecorations: IBreakpointDecoration[];
 	currentStackDecorations: string[];
 	dirty: boolean;
 	topStackFrameRange: Range;
@@ -51,7 +55,7 @@ export class DebugEditorModelManager implements IWorkbenchContribution {
 	public dispose(): void {
 		this.modelDataMap.forEach(modelData => {
 			lifecycle.dispose(modelData.toDispose);
-			modelData.model.deltaDecorations(modelData.breakpointDecorationIds, []);
+			modelData.model.deltaDecorations(modelData.breakpointDecorations.map(bpd => bpd.decorationId), []);
 			modelData.model.deltaDecorations(modelData.currentStackDecorations, []);
 		});
 		this.toDispose = lifecycle.dispose(this.toDispose);
@@ -82,18 +86,13 @@ export class DebugEditorModelManager implements IWorkbenchContribution {
 
 		const currentStackDecorations = model.deltaDecorations([], this.createCallStackDecorations(modelUrlStr));
 		const desiredDecorations = this.createBreakpointDecorations(model, breakpoints);
-		const breakPointDecorations = model.deltaDecorations([], desiredDecorations);
-
+		const breakpointDecorationIds = model.deltaDecorations([], desiredDecorations);
 		const toDispose: lifecycle.IDisposable[] = [model.onDidChangeDecorations((e) => this.onModelDecorationsChanged(modelUrlStr))];
-		const breakpointDecorationsAsMap = new Map<string, Range>();
-		breakPointDecorations.forEach((decorationId, index) => breakpointDecorationsAsMap.set(decorationId, desiredDecorations[index].range));
 
 		this.modelDataMap.set(modelUrlStr, {
 			model: model,
 			toDispose: toDispose,
-			breakpointDecorationIds: breakPointDecorations,
-			breakpointModelIds: breakpoints.map(bp => bp.getId()),
-			breakpointDecorationsAsMap,
+			breakpointDecorations: breakpointDecorationIds.map((decorationId, index) => ({ decorationId, modelId: breakpoints[index].getId(), range: desiredDecorations[index].range })),
 			currentStackDecorations: currentStackDecorations,
 			dirty: false,
 			topStackFrameRange: undefined
@@ -188,17 +187,17 @@ export class DebugEditorModelManager implements IWorkbenchContribution {
 	// breakpoints management. Represent data coming from the debug service and also send data back.
 	private onModelDecorationsChanged(modelUrlStr: string): void {
 		const modelData = this.modelDataMap.get(modelUrlStr);
-		if (modelData.breakpointDecorationsAsMap.size === 0 || this.ignoreDecorationsChangedEvent) {
+		if (modelData.breakpointDecorations.length === 0 || this.ignoreDecorationsChangedEvent) {
 			// I have no decorations
 			return;
 		}
 		let somethingChanged = false;
-		modelData.breakpointDecorationsAsMap.forEach((breakpointRange, decorationId) => {
+		modelData.breakpointDecorations.forEach(breakpointDecoration => {
 			if (somethingChanged) {
 				return;
 			}
-			const newBreakpointRange = modelData.model.getDecorationRange(decorationId);
-			if (newBreakpointRange && (breakpointRange.startColumn !== newBreakpointRange.startColumn || breakpointRange.endLineNumber !== newBreakpointRange.endLineNumber)) {
+			const newBreakpointRange = modelData.model.getDecorationRange(breakpointDecoration.decorationId);
+			if (newBreakpointRange && (breakpointDecoration.range.startColumn !== newBreakpointRange.startColumn || breakpointDecoration.range.endLineNumber !== newBreakpointRange.endLineNumber)) {
 				somethingChanged = true;
 			}
 		});
@@ -207,34 +206,32 @@ export class DebugEditorModelManager implements IWorkbenchContribution {
 			return;
 		}
 
-		const data: IRawBreakpoint[] = [];
-
+		const data: { [id: string]: DebugProtocol.Breakpoint } = Object.create(null);
 		const breakpoints = this.debugService.getModel().getBreakpoints();
 		const modelUri = modelData.model.uri;
-		for (let i = 0, len = modelData.breakpointDecorationIds.length; i < len; i++) {
-			const decorationRange = modelData.model.getDecorationRange(modelData.breakpointDecorationIds[i]);
+		const toRemove: string[] = [];
+		for (let i = 0, len = modelData.breakpointDecorations.length; i < len; i++) {
+			const breakpointDecoration = modelData.breakpointDecorations[i];
+			const decorationRange = modelData.model.getDecorationRange(breakpointDecoration.decorationId);
 			// check if the line got deleted.
-			if (decorationRange && decorationRange.endColumn - decorationRange.startColumn > 0) {
-				const breakpoint = breakpoints.filter(bp => bp.getId() === modelData.breakpointModelIds[i]).pop();
+			if (decorationRange && decorationRange.endColumn > decorationRange.startColumn) {
+				const breakpoint = breakpoints.filter(bp => bp.getId() === breakpointDecoration.modelId).pop();
 				// since we know it is collapsed, it cannot grow to multiple lines
 				if (breakpoint) {
-					data.push({
-						lineNumber: decorationRange.startLineNumber,
-						enabled: breakpoint.enabled,
-						condition: breakpoint.condition,
-						hitCondition: breakpoint.hitCondition,
-						column: breakpoint.column ? decorationRange.startColumn : undefined
-					});
+					data[breakpoint.getId()] = {
+						line: decorationRange.startLineNumber,
+						column: breakpoint.column ? decorationRange.startColumn : undefined,
+						verified: breakpoint.verified
+					};
 				}
+			} else {
+				toRemove.push(breakpointDecoration.modelId);
 			}
 		}
 		modelData.dirty = this.debugService.state !== State.Inactive;
 
-		const toRemove = this.debugService.getModel().getBreakpoints()
-			.filter(bp => bp.uri.toString() === modelUri.toString());
-
-		TPromise.join(toRemove.map(bp => this.debugService.removeBreakpoints(bp.getId()))).then(() => {
-			this.debugService.addBreakpoints(modelUri, data);
+		TPromise.join(toRemove.map(id => this.debugService.removeBreakpoints(id, true))).then(() => {
+			this.debugService.updateBreakpoints(modelUri, data);
 		});
 	}
 
@@ -263,15 +260,16 @@ export class DebugEditorModelManager implements IWorkbenchContribution {
 
 	private updateBreakpoints(modelData: IDebugEditorModelData, newBreakpoints: IBreakpoint[]): void {
 		const desiredDecorations = this.createBreakpointDecorations(modelData.model, newBreakpoints);
+		let breakpointDecorationIds: string[];
 		try {
 			this.ignoreDecorationsChangedEvent = true;
-			modelData.breakpointDecorationIds = modelData.model.deltaDecorations(modelData.breakpointDecorationIds, desiredDecorations);
+			breakpointDecorationIds = modelData.model.deltaDecorations(modelData.breakpointDecorations.map(bpd => bpd.decorationId), desiredDecorations);
 		} finally {
 			this.ignoreDecorationsChangedEvent = false;
 		}
-		modelData.breakpointModelIds = newBreakpoints.map(nbp => nbp.getId());
-		modelData.breakpointDecorationsAsMap.clear();
-		modelData.breakpointDecorationIds.forEach((decorationId, index) => modelData.breakpointDecorationsAsMap.set(decorationId, desiredDecorations[index].range));
+
+		modelData.breakpointDecorations = breakpointDecorationIds.map((decorationId, index) =>
+			({ decorationId, modelId: newBreakpoints[index].getId(), range: desiredDecorations[index].range }));
 	}
 
 	private createBreakpointDecorations(model: IModel, breakpoints: IBreakpoint[]): { range: Range; options: IModelDecorationOptions; }[] {

src/vs/workbench/parts/debug/common/debug.ts

@@ -510,6 +510,11 @@ export interface IDebugService {
 	 */
 	addBreakpoints(uri: uri, rawBreakpoints: IRawBreakpoint[]): TPromise<void>;
 
+	/**
+	 * Updates the breakpoints and notifies the debug adapter of breakpoint changes.
+	 */
+	updateBreakpoints(uri: uri, data: { [id: string]: DebugProtocol.Breakpoint }): TPromise<void>;
+
 	/**
 	 * Enables or disables all breakpoints. If breakpoint is passed only enables or disables the passed breakpoint.
 	 * Notifies debug adapter of breakpoint changes.
@@ -526,7 +531,7 @@ export interface IDebugService {
 	 * Removes all breakpoints. If id is passed only removes the breakpoint associated with that id.
 	 * Notifies debug adapter of breakpoint changes.
 	 */
-	removeBreakpoints(id?: string): TPromise<any>;
+	removeBreakpoints(id?: string, skipEmit?: boolean): TPromise<any>;
 
 	/**
 	 * Adds a new no name function breakpoint. The function breakpoint should be renamed once user enters the name.

src/vs/workbench/parts/debug/common/debugModel.ts

@@ -875,9 +875,11 @@ export class Model implements IModel {
 		return newBreakpoints;
 	}
 
-	public removeBreakpoints(toRemove: IBreakpoint[]): void {
+	public removeBreakpoints(toRemove: IBreakpoint[], skipEvent?: boolean): void {
 		this.breakpoints = this.breakpoints.filter(bp => !toRemove.some(toRemove => toRemove.getId() === bp.getId()));
-		this._onDidChangeBreakpoints.fire();
+		if (!skipEvent) {
+			this._onDidChangeBreakpoints.fire();
+		}
 	}
 
 	public updateBreakpoints(data: { [id: string]: DebugProtocol.Breakpoint }): void {

src/vs/workbench/parts/debug/electron-browser/debugService.ts

@@ -573,12 +573,21 @@ export class DebugService implements debug.IDebugService {
 		return this.sendBreakpoints(uri);
 	}
 
-	public removeBreakpoints(id?: string): TPromise<any> {
+	public updateBreakpoints(uri: uri, data: { [id: string]: DebugProtocol.Breakpoint }): TPromise<void> {
+		this.model.updateBreakpoints(data);
+		return this.sendBreakpoints(uri);
+	}
+
+	public removeBreakpoints(id?: string, skipEmit?: boolean): TPromise<any> {
 		const toRemove = this.model.getBreakpoints().filter(bp => !id || bp.getId() === id);
 		toRemove.forEach(bp => aria.status(nls.localize('breakpointRemoved', "Removed breakpoint, line {0}, file {1}", bp.lineNumber, bp.uri.fsPath)));
 		const urisToClear = distinct(toRemove, bp => bp.uri.toString()).map(bp => bp.uri);
 
-		this.model.removeBreakpoints(toRemove);
+		this.model.removeBreakpoints(toRemove, skipEmit);
+		if (skipEmit) {
+			return TPromise.as(null);
+		}
+
 		return TPromise.join(urisToClear.map(uri => this.sendBreakpoints(uri)));
 	}
 

src/vs/workbench/parts/debug/test/common/mockDebug.ts

@@ -44,6 +44,10 @@ export class MockDebugService implements debug.IDebugService {
 		return TPromise.as(null);
 	}
 
+	public updateBreakpoints(uri: uri, data: { [id: string]: DebugProtocol.Breakpoint }): TPromise<void> {
+		return TPromise.as(null);
+	}
+
 	public enableOrDisableBreakpoints(enabled: boolean): TPromise<void> {
 		return TPromise.as(null);
 	}