Fix paste edit providers incorrectly overwriting data transfer values (microsoft#182169)

At present, copy providers on the ext host side return the full data transfer object passed to them (which includes any additions they make). We then use this to construct a new data transfer that is passed on to paste providers

There are a few bugs with this approach:

- If there are multiple copy providers, they can incorrectly end up overwriting each other. For example if the initial data transfer contains `text/plain` and there are two providers, the first of which tries writing a new `text/plain`, the second provider will overwrite this with the initial `text/plain` value

- If the original data transfer contains multiple entries for a mime, these always end up being overwritten with a single value

- We shouldn't waste type transferring value back to the main thread if the main thread already has them

This PR tries to fix this by skipping ext host to main thread transfer of non-modified data transfer items. As part of this work, I reworked a few internal structures and introduced a new `IReadonlyVSDataTransfer` interface to make it more clear when a internal data transfer is or is not expected to be modified

Author: mjbvz

src/vs/base/common/dataTransfer.ts

@@ -8,80 +8,90 @@ import { Iterable } from 'vs/base/common/iterator';
 import { URI } from 'vs/base/common/uri';
 import { generateUuid } from 'vs/base/common/uuid';
 
-interface IDataTransferFile {
+export interface IDataTransferFile {
+	readonly id: string;
 	readonly name: string;
 	readonly uri?: URI;
 	data(): Promise<Uint8Array>;
 }
 
 export interface IDataTransferItem {
-	readonly id: string;
 	asString(): Thenable<string>;
 	asFile(): IDataTransferFile | undefined;
 	value: any;
 }
 
 export function createStringDataTransferItem(stringOrPromise: string | Promise<string>): IDataTransferItem {
 	return {
-		id: generateUuid(),
 		asString: async () => stringOrPromise,
 		asFile: () => undefined,
 		value: typeof stringOrPromise === 'string' ? stringOrPromise : undefined,
 	};
 }
 
 export function createFileDataTransferItem(fileName: string, uri: URI | undefined, data: () => Promise<Uint8Array>): IDataTransferItem {
+	const file = { id: generateUuid(), name: fileName, uri, data };
 	return {
-		id: generateUuid(),
 		asString: async () => '',
-		asFile: () => ({ name: fileName, uri, data }),
+		asFile: () => file,
 		value: undefined,
 	};
 }
 
-export class VSDataTransfer {
-
-	private readonly _entries = new Map<string, IDataTransferItem[]>();
-
+export interface IReadonlyVSDataTransfer extends Iterable<readonly [string, IDataTransferItem]> {
 	/**
 	 * Get the total number of entries in this data transfer.
 	 */
-	public get size(): number {
-		let size = 0;
-		this.forEach(() => size++);
-		return size;
-	}
+	get size(): number;
 
 	/**
 	 * Check if this data transfer contains data for `mimeType`.
 	 *
 	 * This uses exact matching and does not support wildcards.
 	 */
-	public has(mimeType: string): boolean {
-		return this._entries.has(this.toKey(mimeType));
-	}
-
+	has(mimeType: string): boolean;
 	/**
 	 * Check if this data transfer contains data matching `pattern`.
 	 *
 	 * This allows matching for wildcards, such as `image/*`.
 	 *
 	 * Use the special `files` mime type to match any file in the data transfer.
 	 */
+	matches(pattern: string): boolean;
+
+	/**
+	 * Retrieve the first entry for `mimeType`.
+	 *
+	 * Note that if you want to find all entries for a given mime type, use {@link IReadonlyVSDataTransfer.entries} instead.
+	 */
+	get(mimeType: string): IDataTransferItem | undefined;
+}
+
+export class VSDataTransfer implements IReadonlyVSDataTransfer {
+
+	private readonly _entries = new Map<string, IDataTransferItem[]>();
+
+	public get size(): number {
+		let size = 0;
+		for (const _ of this._entries) {
+			size++;
+		}
+		return size;
+	}
+
+	public has(mimeType: string): boolean {
+		return this._entries.has(this.toKey(mimeType));
+	}
+
 	public matches(pattern: string): boolean {
 		const mimes = [...this._entries.keys()];
-		if (Iterable.some(this.values(), item => item.asFile())) {
+		if (Iterable.some(this, ([_, item]) => item.asFile())) {
 			mimes.push('files');
 		}
 
 		return matchesMimeType_normalized(normalizeMimeType(pattern), mimes);
 	}
 
-	/**
-	 * Retrieve the first entry for `mimeType`.
-	 *
-	 * Note that if want to find all entries for a given mime type, use {@link VSDataTransfer.entries} instead.
-	 */
 	public get(mimeType: string): IDataTransferItem | undefined {
 		return this._entries.get(this.toKey(mimeType))?.[0];
 	}
@@ -121,34 +131,14 @@ export class VSDataTransfer {
 	 *
 	 * There may be multiple entries for each mime type.
 	 */
-	public *entries(): Iterable<[string, IDataTransferItem]> {
-		for (const [mine, items] of this._entries.entries()) {
+	public *[Symbol.iterator](): IterableIterator<readonly [string, IDataTransferItem]> {
+		for (const [mine, items] of this._entries) {
 			for (const item of items) {
 				yield [mine, item];
 			}
 		}
 	}
 
-	/**
-	 * Iterate over all items in this data transfer.
-	 *
-	 * There may be multiple entries for each mime type.
-	 */
-	public values(): Iterable<IDataTransferItem> {
-		return Array.from(this._entries.values()).flat();
-	}
-
-	/**
-	 * Call `f` for each item and mime in the data transfer.
-	 *
-	 * There may be multiple entries for each mime type.
-	 */
-	public forEach(f: (value: IDataTransferItem, mime: string) => void) {
-		for (const [mime, item] of this.entries()) {
-			f(item, mime);
-		}
-	}
-
 	private toKey(mimeType: string): string {
 		return normalizeMimeType(mimeType);
 	}

src/vs/editor/common/languages.ts

@@ -7,7 +7,7 @@ import { VSBuffer } from 'vs/base/common/buffer';
 import { CancellationToken } from 'vs/base/common/cancellation';
 import { Codicon } from 'vs/base/common/codicons';
 import { Color } from 'vs/base/common/color';
-import { VSDataTransfer } from 'vs/base/common/dataTransfer';
+import { IReadonlyVSDataTransfer } from 'vs/base/common/dataTransfer';
 import { Event } from 'vs/base/common/event';
 import { IMarkdownString } from 'vs/base/common/htmlContent';
 import { IDisposable } from 'vs/base/common/lifecycle';
@@ -801,9 +801,9 @@ export interface DocumentPasteEditProvider {
 	readonly copyMimeTypes?: readonly string[];
 	readonly pasteMimeTypes: readonly string[];
 
-	prepareDocumentPaste?(model: model.ITextModel, ranges: readonly IRange[], dataTransfer: VSDataTransfer, token: CancellationToken): Promise<undefined | VSDataTransfer>;
+	prepareDocumentPaste?(model: model.ITextModel, ranges: readonly IRange[], dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<undefined | IReadonlyVSDataTransfer>;
 
-	provideDocumentPasteEdits(model: model.ITextModel, ranges: readonly IRange[], dataTransfer: VSDataTransfer, token: CancellationToken): Promise<DocumentPasteEdit | undefined>;
+	provideDocumentPasteEdits(model: model.ITextModel, ranges: readonly IRange[], dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<DocumentPasteEdit | undefined>;
 }
 
 /**
@@ -1960,5 +1960,5 @@ export interface DocumentOnDropEdit {
 export interface DocumentOnDropEditProvider {
 	readonly dropMimeTypes?: readonly string[];
 
-	provideDocumentOnDropEdits(model: model.ITextModel, position: IPosition, dataTransfer: VSDataTransfer, token: CancellationToken): ProviderResult<DocumentOnDropEdit>;
+	provideDocumentOnDropEdits(model: model.ITextModel, position: IPosition, dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): ProviderResult<DocumentOnDropEdit>;
 }

src/vs/editor/contrib/dropOrPasteInto/browser/copyPasteController.ts

@@ -165,14 +165,18 @@ export class CopyPasteController extends Disposable implements IEditorContributi
 		});
 
 		const promise = createCancelablePromise(async token => {
-			const results = await Promise.all(providers.map(provider => {
+			const results = coalesce(await Promise.all(providers.map(provider => {
 				return provider.prepareDocumentPaste!(model, ranges, dataTransfer, token);
-			}));
+			})));
+
+			// Values from higher priority providers should overwrite values from lower priority ones.
+			// Reverse the array to so that the calls to `replace` below will do this
+			results.reverse();
 
 			for (const result of results) {
-				result?.forEach((value, key) => {
-					dataTransfer.replace(key, value);
-				});
+				for (const [mime, value] of result) {
+					dataTransfer.replace(mime, value);
+				}
 			}
 
 			return dataTransfer;
@@ -368,9 +372,9 @@ export class CopyPasteController extends Disposable implements IEditorContributi
 				return;
 			}
 
-			toMergeDataTransfer.forEach((value, key) => {
+			for (const [key, value] of toMergeDataTransfer) {
 				dataTransfer.replace(key, value);
-			});
+			}
 		}
 
 		if (!dataTransfer.has(Mimes.uriList)) {

src/vs/editor/contrib/dropOrPasteInto/browser/defaultProviders.ts

@@ -5,7 +5,7 @@
 
 import { coalesce } from 'vs/base/common/arrays';
 import { CancellationToken } from 'vs/base/common/cancellation';
-import { UriList, VSDataTransfer } from 'vs/base/common/dataTransfer';
+import { IReadonlyVSDataTransfer, UriList } from 'vs/base/common/dataTransfer';
 import { Disposable } from 'vs/base/common/lifecycle';
 import { Mimes } from 'vs/base/common/mime';
 import { Schemas } from 'vs/base/common/network';
@@ -27,17 +27,17 @@ abstract class SimplePasteAndDropProvider implements DocumentOnDropEditProvider,
 	abstract readonly dropMimeTypes: readonly string[] | undefined;
 	abstract readonly pasteMimeTypes: readonly string[];
 
-	async provideDocumentPasteEdits(_model: ITextModel, _ranges: readonly IRange[], dataTransfer: VSDataTransfer, token: CancellationToken): Promise<DocumentPasteEdit | undefined> {
+	async provideDocumentPasteEdits(_model: ITextModel, _ranges: readonly IRange[], dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<DocumentPasteEdit | undefined> {
 		const edit = await this.getEdit(dataTransfer, token);
 		return edit ? { id: this.id, insertText: edit.insertText, label: edit.label, detail: edit.detail, priority: edit.priority } : undefined;
 	}
 
-	async provideDocumentOnDropEdits(_model: ITextModel, _position: IPosition, dataTransfer: VSDataTransfer, token: CancellationToken): Promise<DocumentOnDropEdit | undefined> {
+	async provideDocumentOnDropEdits(_model: ITextModel, _position: IPosition, dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<DocumentOnDropEdit | undefined> {
 		const edit = await this.getEdit(dataTransfer, token);
 		return edit ? { id: this.id, insertText: edit.insertText, label: edit.label, priority: edit.priority } : undefined;
 	}
 
-	protected abstract getEdit(dataTransfer: VSDataTransfer, token: CancellationToken): Promise<DocumentPasteEdit | undefined>;
+	protected abstract getEdit(dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<DocumentPasteEdit | undefined>;
 }
 
 class DefaultTextProvider extends SimplePasteAndDropProvider {
@@ -46,7 +46,7 @@ class DefaultTextProvider extends SimplePasteAndDropProvider {
 	readonly dropMimeTypes = [Mimes.text];
 	readonly pasteMimeTypes = [Mimes.text];
 
-	protected async getEdit(dataTransfer: VSDataTransfer, _token: CancellationToken) {
+	protected async getEdit(dataTransfer: IReadonlyVSDataTransfer, _token: CancellationToken) {
 		const textEntry = dataTransfer.get(Mimes.text);
 		if (!textEntry) {
 			return;
@@ -75,7 +75,7 @@ class PathProvider extends SimplePasteAndDropProvider {
 	readonly dropMimeTypes = [Mimes.uriList];
 	readonly pasteMimeTypes = [Mimes.uriList];
 
-	protected async getEdit(dataTransfer: VSDataTransfer, token: CancellationToken) {
+	protected async getEdit(dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken) {
 		const entries = await extractUriList(dataTransfer);
 		if (!entries.length || token.isCancellationRequested) {
 			return;
@@ -128,7 +128,7 @@ class RelativePathProvider extends SimplePasteAndDropProvider {
 		super();
 	}
 
-	protected async getEdit(dataTransfer: VSDataTransfer, token: CancellationToken) {
+	protected async getEdit(dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken) {
 		const entries = await extractUriList(dataTransfer);
 		if (!entries.length || token.isCancellationRequested) {
 			return;
@@ -155,7 +155,7 @@ class RelativePathProvider extends SimplePasteAndDropProvider {
 	}
 }
 
-async function extractUriList(dataTransfer: VSDataTransfer): Promise<{ readonly uri: URI; readonly originalText: string }[]> {
+async function extractUriList(dataTransfer: IReadonlyVSDataTransfer): Promise<{ readonly uri: URI; readonly originalText: string }[]> {
 	const urlListEntry = dataTransfer.get(Mimes.uriList);
 	if (!urlListEntry) {
 		return [];

src/vs/editor/contrib/dropOrPasteInto/browser/dropIntoEditorController.ts

@@ -145,7 +145,7 @@ export class DropIntoEditorController extends Disposable implements IEditorContr
 				for (const id of data) {
 					const treeDataTransfer = await this._treeViewsDragAndDropService.removeDragOperationTransfer(id.identifier);
 					if (treeDataTransfer) {
-						for (const [type, value] of treeDataTransfer.entries()) {
+						for (const [type, value] of treeDataTransfer) {
 							dataTransfer.replace(type, value);
 						}
 					}