Merge from vscode 3a6dcb42008d509900b3a3b2d695564eeb4dbdac (#5098)

src/vs/base/node/encoding.ts

@@ -18,15 +18,20 @@ export const UTF16be_BOM = [0xFE, 0xFF];
 export const UTF16le_BOM = [0xFF, 0xFE];
 export const UTF8_BOM = [0xEF, 0xBB, 0xBF];
 
+const ZERO_BYTE_DETECTION_BUFFER_MAX_LEN = 512; // number of bytes to look at to decide about a file being binary or not
+const NO_GUESS_BUFFER_MAX_LEN = 512; 			// when not auto guessing the encoding, small number of bytes are enough
+const AUTO_GUESS_BUFFER_MAX_LEN = 512 * 8; 		// with auto guessing we want a lot more content to be read for guessing
+
 export interface IDecodeStreamOptions {
-	guessEncoding?: boolean;
+	guessEncoding: boolean;
 	minBytesRequiredForDetection?: number;
-	overwriteEncoding?(detectedEncoding: string | null): string;
+
+	overwriteEncoding(detectedEncoding: string | null): string;
 }
 
 export interface IDecodeStreamResult {
-	detected: IDetectedEncodingResult;
 	stream: NodeJS.ReadableStream;
+	detected: IDetectedEncodingResult;
 }
 
 export function toDecodeStream(readable: Readable, options: IDecodeStreamOptions): Promise<IDecodeStreamResult> {
@@ -34,78 +39,82 @@ export function toDecodeStream(readable: Readable, options: IDecodeStreamOptions
 		options.minBytesRequiredForDetection = options.guessEncoding ? AUTO_GUESS_BUFFER_MAX_LEN : NO_GUESS_BUFFER_MAX_LEN;
 	}
 
-	if (!options.overwriteEncoding) {
-		options.overwriteEncoding = detected => detected || UTF8;
-	}
-
 	return new Promise<IDecodeStreamResult>((resolve, reject) => {
 		const writer = new class extends Writable {
 			private decodeStream: NodeJS.ReadWriteStream;
-			private decodeStreamConstruction: Promise<void>;
-			private buffer: Buffer[] = [];
+			private decodeStreamPromise: Promise<void>;
+
+			private bufferedChunks: Buffer[] = [];
 			private bytesBuffered = 0;
 
-			_write(chunk: any, encoding: string, callback: Function): void {
+			_write(chunk: Buffer, encoding: string, callback: (error: Error | null) => void): void {
 				if (!Buffer.isBuffer(chunk)) {
-					callback(new Error('data must be a buffer'));
+					return callback(new Error('toDecodeStream(): data must be a buffer'));
 				}
 
+				// if the decode stream is ready, we just write directly
 				if (this.decodeStream) {
-					this.decodeStream.write(chunk, callback); // just a forwarder now
+					this.decodeStream.write(chunk, callback);
 
 					return;
 				}
 
-				this.buffer.push(chunk);
-				this.bytesBuffered += chunk.length;
+				// otherwise we need to buffer the data until the stream is ready
+				this.bufferedChunks.push(chunk);
+				this.bytesBuffered += chunk.byteLength;
 
 				// waiting for the decoder to be ready
-				if (this.decodeStreamConstruction) {
-					this.decodeStreamConstruction.then(() => callback(), err => callback(err));
+				if (this.decodeStreamPromise) {
+					this.decodeStreamPromise.then(() => callback(null), error => callback(error));
 				}
 
-				// buffered enough data, create stream and forward data
+				// buffered enough data for encoding detection, create stream and forward data
 				else if (typeof options.minBytesRequiredForDetection === 'number' && this.bytesBuffered >= options.minBytesRequiredForDetection) {
 					this._startDecodeStream(callback);
 				}
 
-				// only buffering
+				// only buffering until enough data for encoding detection is there
 				else {
-					callback();
+					callback(null);
 				}
 			}
 
-			_startDecodeStream(callback: Function): void {
-				this.decodeStreamConstruction = Promise.resolve(detectEncodingFromBuffer({
-					buffer: Buffer.concat(this.buffer),
+			_startDecodeStream(callback: (error: Error | null) => void): void {
+
+				// detect encoding from buffer
+				this.decodeStreamPromise = Promise.resolve(detectEncodingFromBuffer({
+					buffer: Buffer.concat(this.bufferedChunks),
 					bytesRead: this.bytesBuffered
 				}, options.guessEncoding)).then(detected => {
-					if (options.overwriteEncoding) {
-						detected.encoding = options.overwriteEncoding(detected.encoding);
-					}
 
+					// ensure to respect overwrite of encoding
+					detected.encoding = options.overwriteEncoding(detected.encoding);
+
+					// decode and write buffer
 					this.decodeStream = decodeStream(detected.encoding);
+					this.decodeStream.write(Buffer.concat(this.bufferedChunks), callback);
+					this.bufferedChunks.length = 0;
 
-					for (const buffer of this.buffer) {
-						this.decodeStream.write(buffer);
-					}
-
-					callback();
+					// signal to the outside our detected encoding
+					// and final decoder stream
 					resolve({ detected, stream: this.decodeStream });
-				}, err => {
-					this.emit('error', err);
-					callback(err);
+				}, error => {
+					this.emit('error', error);
+
+					callback(error);
 				});
 			}
 
-			_final(callback: (err?: any) => any) {
+			_final(callback: (error: Error | null) => void) {
 
 				// normal finish
 				if (this.decodeStream) {
 					this.decodeStream.end(callback);
 				}
 
-				// we were still waiting for data...
+				// we were still waiting for data to do the encoding
+				// detection. thus, wrap up starting the stream even
+				// without all the data to get things going
 				else {
 					this._startDecodeStream(() => this.decodeStream.end(callback));
 				}
@@ -149,7 +158,7 @@ function toNodeEncoding(enc: string | null): string {
 }
 
 export function detectEncodingByBOMFromBuffer(buffer: Buffer | VSBuffer | null, bytesRead: number): string | null {
-	if (!buffer || bytesRead < 2) {
+	if (!buffer || bytesRead < UTF16be_BOM.length) {
 		return null;
 	}
 
@@ -166,7 +175,7 @@ export function detectEncodingByBOMFromBuffer(buffer: Buffer | VSBuffer | null,
 		return UTF16le;
 	}
 
-	if (bytesRead < 3) {
+	if (bytesRead < UTF8_BOM.length) {
 		return null;
 	}
 
@@ -256,10 +265,6 @@ export function toCanonicalName(enc: string): string {
 	}
 }
 
-const ZERO_BYTE_DETECTION_BUFFER_MAX_LEN = 512; // number of bytes to look at to decide about a file being binary or not
-const NO_GUESS_BUFFER_MAX_LEN = 512; 			// when not auto guessing the encoding, small number of bytes are enough
-const AUTO_GUESS_BUFFER_MAX_LEN = 512 * 8; 		// with auto guessing we want a lot more content to be read for guessing
-
 export interface IDetectedEncodingResult {
 	encoding: string | null;
 	seemsBinary: boolean;

src/vs/base/parts/contextmenu/electron-main/contextmenu.ts

@@ -16,7 +16,9 @@ export function registerContextMenuListener(): void {
 			y: options ? options.y : undefined,
 			positioningItem: options ? options.positioningItem : undefined,
 			callback: () => {
-				event.sender.send(CONTEXT_MENU_CLOSE_CHANNEL, contextMenuId);
+				if (menu) {
+					event.sender.send(CONTEXT_MENU_CLOSE_CHANNEL, contextMenuId);
+				}
 			}
 		});
 	});

src/vs/base/parts/ipc/common/ipc.net.ts

@@ -126,7 +126,8 @@ const enum ProtocolMessageType {
 	Regular = 1,
 	Control = 2,
 	Ack = 3,
-	KeepAlive = 4
+	KeepAlive = 4,
+	Disconnect = 5
 }
 
 export const enum ProtocolConstants {
@@ -373,6 +374,10 @@ export class Protocol extends Disposable implements IMessagePassingProtocol {
 		return this._socket;
 	}
 
+	sendDisconnect(): void {
+		// Nothing to do...
+	}
+
 	send(buffer: VSBuffer): void {
 		this._socketWriter.write(new ProtocolMessage(ProtocolMessageType.Regular, 0, 0, buffer));
 	}
@@ -393,6 +398,7 @@ export class Client<TContext = string> extends IPCClient<TContext> {
 	dispose(): void {
 		super.dispose();
 		const socket = this.protocol.getSocket();
+		this.protocol.sendDisconnect();
 		this.protocol.dispose();
 		socket.end();
 	}
@@ -572,7 +578,6 @@ export class PersistentProtocol {
 		this._socketDisposables.push(this._socketReader);
 		this._socketDisposables.push(this._socketReader.onMessage(msg => this._receiveMessage(msg)));
 		this._socketDisposables.push(this._socket.onClose(() => this._onSocketClose.fire()));
-		this._socketDisposables.push(this._socket.onEnd(() => this._onClose.fire()));
 		if (initialChunk) {
 			this._socketReader.acceptChunk(initialChunk);
 		}
@@ -601,6 +606,12 @@ export class PersistentProtocol {
 		this._socketDisposables = dispose(this._socketDisposables);
 	}
 
+	sendDisconnect(): void {
+		const msg = new ProtocolMessage(ProtocolMessageType.Disconnect, 0, 0, getEmptyBuffer());
+		this._socketWriter.write(msg);
+		this._socketWriter.flush();
+	}
+
 	private _sendKeepAliveCheck(): void {
 		if (this._outgoingKeepAliveTimeout) {
 			// there will be a check in the near future
@@ -659,7 +670,6 @@ export class PersistentProtocol {
 		this._socketDisposables.push(this._socketReader);
 		this._socketDisposables.push(this._socketReader.onMessage(msg => this._receiveMessage(msg)));
 		this._socketDisposables.push(this._socket.onClose(() => this._onSocketClose.fire()));
-		this._socketDisposables.push(this._socket.onEnd(() => this._onClose.fire()));
 		this._socketReader.acceptChunk(initialDataChunk);
 	}
 
@@ -703,6 +713,8 @@ export class PersistentProtocol {
 			}
 		} else if (msg.type === ProtocolMessageType.Control) {
 			this._onControlMessage.fire(msg.data);
+		} else if (msg.type === ProtocolMessageType.Disconnect) {
+			this._onClose.fire();
 		}
 	}
 

src/vs/base/test/node/encoding/encoding.test.ts

@@ -240,7 +240,7 @@ suite('Encoding', () => {
 			}
 		});
 
-		let { detected, stream } = await encoding.toDecodeStream(source, { minBytesRequiredForDetection: 4 });
+		let { detected, stream } = await encoding.toDecodeStream(source, { minBytesRequiredForDetection: 4, guessEncoding: false, overwriteEncoding: detected => detected || encoding.UTF8 });
 
 		assert.ok(detected);
 		assert.ok(stream);
@@ -260,7 +260,7 @@ suite('Encoding', () => {
 			}
 		});
 
-		let { detected, stream } = await encoding.toDecodeStream(source, { minBytesRequiredForDetection: 64 });
+		let { detected, stream } = await encoding.toDecodeStream(source, { minBytesRequiredForDetection: 64, guessEncoding: false, overwriteEncoding: detected => detected || encoding.UTF8 });
 
 		assert.ok(detected);
 		assert.ok(stream);
@@ -277,7 +277,7 @@ suite('Encoding', () => {
 			}
 		});
 
-		let { detected, stream } = await encoding.toDecodeStream(source, { minBytesRequiredForDetection: 512 });
+		let { detected, stream } = await encoding.toDecodeStream(source, { minBytesRequiredForDetection: 512, guessEncoding: false, overwriteEncoding: detected => detected || encoding.UTF8 });
 
 		assert.ok(detected);
 		assert.ok(stream);
@@ -292,7 +292,7 @@ suite('Encoding', () => {
 		let path = getPathFromAmdModule(require, './fixtures/some_utf16be.css');
 		let source = fs.createReadStream(path);
 
-		let { detected, stream } = await encoding.toDecodeStream(source, { minBytesRequiredForDetection: 64 });
+		let { detected, stream } = await encoding.toDecodeStream(source, { minBytesRequiredForDetection: 64, guessEncoding: false, overwriteEncoding: detected => detected || encoding.UTF8 });
 
 		assert.equal(detected.encoding, 'utf16be');
 		assert.equal(detected.seemsBinary, false);
@@ -307,7 +307,7 @@ suite('Encoding', () => {
 
 		let path = getPathFromAmdModule(require, './fixtures/empty.txt');
 		let source = fs.createReadStream(path);
-		let { detected, stream } = await encoding.toDecodeStream(source, {});
+		let { detected, stream } = await encoding.toDecodeStream(source, { guessEncoding: false, overwriteEncoding: detected => detected || encoding.UTF8 });
 
 		let expected = await readAndDecodeFromDisk(path, detected.encoding);
 		let actual = await readAllAsString(stream);