prepare for weak references - use ExtHostDocumentData with a TextDocument property instead of having just ExtHostDocument

src/vs/workbench/api/common/extHostDocuments.ts

@@ -66,8 +66,8 @@ export class ExtHostModelService {
 	private _onDidSaveDocumentEventEmitter: Emitter<vscode.TextDocument>;
 	public onDidSaveDocument: Event<vscode.TextDocument>;
 
-	private _documents: { [modelUri: string]: ExtHostDocumentData; };
+	private _documentData: { [modelUri: string]: ExtHostDocumentData; };
-	private _loadingDocuments: { [modelUri: string]: TPromise<ExtHostDocumentData> };
+	private _documentLoader: { [modelUri: string]: TPromise<ExtHostDocumentData> };
 	private _documentContentProviders: { [scheme: string]: vscode.TextDocumentContentProvider };
 
 	private _proxy: MainThreadDocuments;
@@ -87,15 +87,15 @@ export class ExtHostModelService {
 		this._onDidSaveDocumentEventEmitter = new Emitter<vscode.TextDocument>();
 		this.onDidSaveDocument = this._onDidSaveDocumentEventEmitter.event;
 
-		this._documents = Object.create(null);
+		this._documentData = Object.create(null);
-		this._loadingDocuments = Object.create(null);
+		this._documentLoader = Object.create(null);
 		this._documentContentProviders = Object.create(null);
 	}
 
 	public getDocuments(): vscode.TextDocument[] {
 		let r: vscode.TextDocument[] = [];
-		for (let key in this._documents) {
+		for (let key in this._documentData) {
-			r.push(this._documents[key]);
+			r.push(this._documentData[key].document);
 		}
 		return r;
 	}
@@ -104,7 +104,10 @@ export class ExtHostModelService {
 		if (!resource) {
 			return null;
 		}
-		return this._documents[resource.toString()] || null;
+		const data = this._documentData[resource.toString()];
+		if (data) {
+			return data.document;
+		}
 	}
 
 	public openDocument(uriOrFileName: vscode.Uri | string): TPromise<vscode.TextDocument> {
@@ -118,23 +121,24 @@ export class ExtHostModelService {
 			throw new Error('illegal argument - uriOrFileName');
 		}
 
-		let cached = this._documents[uri.toString()];
+		let cached = this._documentData[uri.toString()];
 		if (cached) {
-			return TPromise.as(cached);
+			return TPromise.as(cached.document);
 		}
 
-		let promise = this._loadingDocuments[uri.toString()];
+		let promise = this._documentLoader[uri.toString()];
-		if (promise) {
+		if (!promise) {
-			return promise;
+			promise = this._proxy._tryOpenDocument(uri).then(() => {
+				delete this._documentLoader[uri.toString()];
+				return this._documentData[uri.toString()];
+			}, err => {
+				delete this._documentLoader[uri.toString()];
+				return TPromise.wrapError(err);
+			});
+			this._documentLoader[uri.toString()] = promise;
 		}
 
-		return this._loadingDocuments[uri.toString()] = this._proxy._tryOpenDocument(uri).then(() => {
+		return promise.then(data => data.document);
-			delete this._loadingDocuments[uri.toString()];
-			return this._documents[uri.toString()];
-		}, err => {
-			delete this._loadingDocuments[uri.toString()];
-			return TPromise.wrapError(err);
-		});
 	}
 
 	public registerTextDocumentContentProvider(scheme: string, provider: vscode.TextDocumentContentProvider): vscode.Disposable {
@@ -158,58 +162,58 @@ export class ExtHostModelService {
 		});
 	}
 
-	public _acceptModelAdd(data:IModelAddedData): void {
+	public _acceptModelAdd(initData:IModelAddedData): void {
-		let document = new ExtHostDocumentData(this._proxy, data.url, data.value.lines, data.value.EOL, data.modeId, data.versionId, data.isDirty);
+		let data = new ExtHostDocumentData(this._proxy, initData.url, initData.value.lines, initData.value.EOL, initData.modeId, initData.versionId, initData.isDirty);
-		let key = document.uri.toString();
+		let key = data.document.uri.toString();
-		if (this._documents[key]) {
+		if (this._documentData[key]) {
 			throw new Error('Document `' + key + '` already exists.');
 		}
-		this._documents[key] = document;
+		this._documentData[key] = data;
-		this._onDidAddDocumentEventEmitter.fire(document);
+		this._onDidAddDocumentEventEmitter.fire(data.document);
 	}
 
 	public _acceptModelModeChanged(url: URI, oldModeId:string, newModeId:string): void {
-		let document = this._documents[url.toString()];
+		let data = this._documentData[url.toString()];
 
 		// Treat a mode change as a remove + add
 
-		this._onDidRemoveDocumentEventEmitter.fire(document);
+		this._onDidRemoveDocumentEventEmitter.fire(data.document);
-		document._acceptLanguageId(newModeId);
+		data._acceptLanguageId(newModeId);
-		this._onDidAddDocumentEventEmitter.fire(document);
+		this._onDidAddDocumentEventEmitter.fire(data.document);
 	}
 
 	public _acceptModelSaved(url: URI): void {
-		let document = this._documents[url.toString()];
+		let data = this._documentData[url.toString()];
-		document._acceptIsDirty(false);
+		data._acceptIsDirty(false);
-		this._onDidSaveDocumentEventEmitter.fire(document);
+		this._onDidSaveDocumentEventEmitter.fire(data.document);
 	}
 
 	public _acceptModelDirty(url: URI): void {
-		let document = this._documents[url.toString()];
+		let document = this._documentData[url.toString()];
 		document._acceptIsDirty(true);
 	}
 
 	public _acceptModelReverted(url: URI): void {
-		let document = this._documents[url.toString()];
+		let document = this._documentData[url.toString()];
 		document._acceptIsDirty(false);
 	}
 
 	public _acceptModelRemoved(url: URI): void {
 		let key = url.toString();
-		if (!this._documents[key]) {
+		if (!this._documentData[key]) {
 			throw new Error('Document `' + key + '` does not exist.');
 		}
-		let document = this._documents[key];
+		let data = this._documentData[key];
-		delete this._documents[key];
+		delete this._documentData[key];
-		this._onDidRemoveDocumentEventEmitter.fire(document);
+		this._onDidRemoveDocumentEventEmitter.fire(data.document);
-		document.dispose();
+		data.dispose();
 	}
 
 	public _acceptModelChanged(url: URI, events: EditorCommon.IModelContentChangedEvent2[]): void {
-		let document = this._documents[url.toString()];
+		let data = this._documentData[url.toString()];
-		document.onEvents(events);
+		data.onEvents(events);
 		this._onDidChangeDocumentEventEmitter.fire({
-			document: document,
+			document: data.document,
 			contentChanges: events.map((e) => {
 				return {
 					range: TypeConverters.toRange(e.range),
@@ -221,12 +225,13 @@ export class ExtHostModelService {
 	}
 }
 
-export class ExtHostDocumentData extends MirrorModel2 implements vscode.TextDocument {
+export class ExtHostDocumentData extends MirrorModel2 {
 
 	private _proxy: MainThreadDocuments;
 	private _languageId: string;
 	private _isDirty: boolean;
 	private _textLines: vscode.TextLine[];
+	private _document: vscode.TextDocument;
 
 	constructor(proxy: MainThreadDocuments, uri: URI, lines: string[], eol: string,
 		languageId: string, versionId: number, isDirty: boolean) {
@@ -244,32 +249,28 @@ export class ExtHostDocumentData extends MirrorModel2 implements vscode.TextDocu
 		super.dispose();
 	}
 
-	get uri(): URI {
+	get document(): vscode.TextDocument {
-		return this._uri;
+		if (!this._document) {
-	}
+			const document = this;
-
+			this._document = {
-	get fileName(): string {
+				get uri() { return document._uri },
-		return this._uri.fsPath;
+				get fileName() { return document._uri.fsPath },
-	}
+				get isUntitled() { return document._uri.scheme !== 'file' },
-
+				get languageId() { return document._languageId },
-	get isUntitled(): boolean {
+				get version() { return document._versionId },
-		return this._uri.scheme !== 'file';
+				get isDirty() { return document._isDirty },
-	}
+				save() { return document._proxy._trySaveDocument(document._uri) },
-
+				getText(range?) { return range ? document._getTextInRange(range) : document.getText() },
-	get languageId(): string {
+				get lineCount() { return document._lines.length },
-		return this._languageId;
+				lineAt(lineOrPos) { return document.lineAt(lineOrPos) },
-	}
+				offsetAt(pos) { return document.offsetAt(pos) },
-
+				positionAt(offset) { return document.positionAt(offset) },
-	get version(): number {
+				validateRange(ran) { return document.validateRange(ran) },
-		return this._versionId;
+				validatePosition(pos) { return document.validatePosition(pos) },
-	}
+				getWordRangeAtPosition(pos){ return document.getWordRangeAtPosition(pos)}
-
+			}
-	get isDirty(): boolean {
+		}
-		return this._isDirty;
+		return this._document;
-	}
-
-	save(): Thenable<boolean> {
-		return this._proxy._trySaveDocument(this._uri);
 	}
 
 	_acceptLanguageId(newLanguageId:string): void {
@@ -280,15 +281,7 @@ export class ExtHostDocumentData extends MirrorModel2 implements vscode.TextDocu
 		this._isDirty = isDirty;
 	}
 
-	getText(range?: Range): string {
+	private _getTextInRange(_range: vscode.Range): string {
-		if (range) {
-			return this._getTextInRange(range);
-		} else {
-			return super.getText();
-		}
-	}
-
-	private _getTextInRange(_range: Range): string {
 		let range = this.validateRange(_range);
 
 		if (range.isEmpty) {
@@ -313,10 +306,6 @@ export class ExtHostDocumentData extends MirrorModel2 implements vscode.TextDocu
 		return resultLines.join(lineEnding);
 	}
 
-	get lineCount(): number {
-		return this._lines.length;
-	}
-
 	lineAt(lineOrPosition: number | vscode.Position): vscode.TextLine {
 
 		let line: number;
@@ -353,13 +342,13 @@ export class ExtHostDocumentData extends MirrorModel2 implements vscode.TextDocu
 		return result;
 	}
 
-	offsetAt(position: Position): number {
+	offsetAt(position: vscode.Position): number {
 		position = this.validatePosition(position);
 		this._ensureLineStarts();
 		return this._lineStarts.getAccumulatedValue(position.line - 1) + position.character;
 	}
 
-	positionAt(offset: number): Position {
+	positionAt(offset: number): vscode.Position {
 		offset = Math.floor(offset);
 		offset = Math.max(0, offset);
 
@@ -375,7 +364,7 @@ export class ExtHostDocumentData extends MirrorModel2 implements vscode.TextDocu
 
 	// ---- range math
 
-	validateRange(range:Range): Range {
+	validateRange(range:vscode.Range): vscode.Range {
 		if (!(range instanceof Range)) {
 			throw new Error('Invalid argument');
 		}
@@ -386,10 +375,10 @@ export class ExtHostDocumentData extends MirrorModel2 implements vscode.TextDocu
 		if (start === range.start && end === range.end) {
 			return range;
 		}
-		return new Range(start, end);
+		return new Range(start.line, start.character, end.line, end.character);
 	}
 
-	validatePosition(position:Position): Position {
+	validatePosition(position:vscode.Position): vscode.Position {
 		if (!(position instanceof Position)) {
 			throw new Error('Invalid argument');
 		}
@@ -424,7 +413,7 @@ export class ExtHostDocumentData extends MirrorModel2 implements vscode.TextDocu
 		return new Position(line, character);
 	}
 
-	getWordRangeAtPosition(_position:Position): Range {
+	getWordRangeAtPosition(_position: vscode.Position): vscode.Range {
 		let position = this.validatePosition(_position);
 
 		let wordAtText = WordHelper._getWordAtText(

src/vs/workbench/test/common/api/extHostDocuments.test.ts

@@ -15,22 +15,22 @@ import * as EditorCommon from 'vs/editor/common/editorCommon';
 
 suite("PluginHostDocument", () => {
 
-	let doc: ExtHostDocumentData;
+	let data: ExtHostDocumentData;
 
 	function assertPositionAt(offset: number, line: number, character: number) {
-		let position = doc.positionAt(offset);
+		let position = data.positionAt(offset);
 		assert.equal(position.line, line);
 		assert.equal(position.character, character);
 	}
 
 	function assertOffsetAt(line: number, character: number, offset: number) {
 		let pos = new Position(line, character);
-		let actual = doc.offsetAt(pos);
+		let actual = data.offsetAt(pos);
 		assert.equal(actual, offset);
 	}
 
 	setup(function() {
-		doc = new ExtHostDocumentData(undefined, URI.file(''), [
+		data = new ExtHostDocumentData(undefined, URI.file(''), [
 			'This is line one', //16
 			'and this is line number two', //27
 			'it is followed by #3', //20
@@ -40,33 +40,33 @@ suite("PluginHostDocument", () => {
 
 	test('readonly-ness', function() {
 
-		assert.throws(() => doc.uri = null);
+		assert.throws(() => data.document.uri = null);
-		assert.throws(() => doc.fileName = 'foofile');
+		assert.throws(() => data.document.fileName = 'foofile');
-		assert.throws(() => doc.isDirty = false);
+		assert.throws(() => data.document.isDirty = false);
-		assert.throws(() => doc.isUntitled = false);
+		assert.throws(() => data.document.isUntitled = false);
-		assert.throws(() => doc.languageId = 'dddd');
+		assert.throws(() => data.document.languageId = 'dddd');
-		assert.throws(() => doc.lineCount = 9);
+		assert.throws(() => data.document.lineCount = 9);
 	})
 
 	test('lines', function() {
 
-		assert.equal(doc.lineCount, 4);
+		assert.equal(data.document.lineCount, 4);
-		assert.throws(() => doc.lineCount = 9);
+		assert.throws(() => data.document.lineCount = 9);
 
-		assert.throws(() => doc.lineAt(-1));
+		assert.throws(() => data.lineAt(-1));
-		assert.throws(() => doc.lineAt(doc.lineCount));
+		assert.throws(() => data.lineAt(data.document.lineCount));
-		assert.throws(() => doc.lineAt(Number.MAX_VALUE));
+		assert.throws(() => data.lineAt(Number.MAX_VALUE));
-		assert.throws(() => doc.lineAt(Number.MIN_VALUE));
+		assert.throws(() => data.lineAt(Number.MIN_VALUE));
-		assert.throws(() => doc.lineAt(0.8));
+		assert.throws(() => data.lineAt(0.8));
 
-		let line = doc.lineAt(0);
+		let line = data.lineAt(0);
 		assert.equal(line.lineNumber, 0);
 		assert.equal(line.text.length, 16);
 		assert.equal(line.text, 'This is line one');
 		assert.equal(line.isEmptyOrWhitespace, false);
 		assert.equal(line.firstNonWhitespaceCharacterIndex, 0);
 
-		doc.onEvents([{
+		data.onEvents([{
 			range: { startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 1 },
 			text: '\t ',
 			isRedoing: undefined,
@@ -80,7 +80,7 @@ suite("PluginHostDocument", () => {
 		assert.equal(line.firstNonWhitespaceCharacterIndex, 0);
 
 		// fetch line again
-		line = doc.lineAt(0);
+		line = data.lineAt(0);
 		assert.equal(line.text, '\t This is line one');
 		assert.equal(line.firstNonWhitespaceCharacterIndex, 2);
 	});
@@ -102,7 +102,7 @@ suite("PluginHostDocument", () => {
 
 	test('offsetAt, after remove', function() {
 
-		doc.onEvents([{
+		data.onEvents([{
 			range: { startLineNumber: 1, startColumn: 3, endLineNumber: 1, endColumn: 6 },
 			text: '',
 			isRedoing: undefined,
@@ -118,7 +118,7 @@ suite("PluginHostDocument", () => {
 
 	test('offsetAt, after replace', function() {
 
-		doc.onEvents([{
+		data.onEvents([{
 			range: { startLineNumber: 1, startColumn: 3, endLineNumber: 1, endColumn: 6 },
 			text: 'is could be',
 			isRedoing: undefined,
@@ -134,7 +134,7 @@ suite("PluginHostDocument", () => {
 
 	test('offsetAt, after insert line', function() {
 
-		doc.onEvents([{
+		data.onEvents([{
 			range: { startLineNumber: 1, startColumn: 3, endLineNumber: 1, endColumn: 6 },
 			text: 'is could be\na line with number',
 			isRedoing: undefined,
@@ -153,7 +153,7 @@ suite("PluginHostDocument", () => {
 
 	test('offsetAt, after remove line', function() {
 
-		doc.onEvents([{
+		data.onEvents([{
 			range: { startLineNumber: 1, startColumn: 3, endLineNumber: 2, endColumn: 6 },
 			text: '',
 			isRedoing: undefined,
@@ -189,7 +189,7 @@ enum AssertDocumentLineMappingDirection {
 
 suite("PluginHostDocument updates line mapping", () => {
 
-	function positionToStr(position:Position): string {
+	function positionToStr(position: { line: number; character: number;}): string {
 		return '(' + position.line + ',' + position.character + ')';
 	}