Images do not show updated when changed on disk (fixes #7951)

src/vs/base/browser/ui/resourceviewer/resourceViewer.ts

@@ -13,6 +13,7 @@ import paths = require('vs/base/common/paths');
 import {Builder, $} from 'vs/base/browser/builder';
 import DOM = require('vs/base/browser/dom');
 import {DomScrollableElement} from 'vs/base/browser/ui/scrollbar/scrollableElement';
+import {BoundedLinkedMap} from 'vs/base/common/map';
 
 // Known media mimes that we can handle
 const mapExtToMediaMimes = {
@@ -67,6 +68,29 @@ export interface IResourceDescriptor {
 	resource: URI;
 	name: string;
 	size: number;
+	etag: string;
+}
+
+// Chrome is caching images very aggressively and so we use the ETag information to find out if
+// we need to bypass the cache or not. We could always bypass the cache everytime we show the image
+// however that has very bad impact on memory consumption because each time the image gets shown,
+// memory grows (see also https://github.com/electron/electron/issues/6275)
+const IMAGE_RESOURCE_ETAG_CACHE = new BoundedLinkedMap<{ etag: string, src: string }>(100);
+function imageSrc(descriptor: IResourceDescriptor): string {
+	const src = descriptor.resource.toString();
+
+	let cached = IMAGE_RESOURCE_ETAG_CACHE.get(src);
+	if (!cached) {
+		cached = { etag: descriptor.etag, src };
+		IMAGE_RESOURCE_ETAG_CACHE.set(src, cached);
+	}
+
+	if (cached.etag !== descriptor.etag) {
+		cached.etag = descriptor.etag;
+		cached.src = `${src}?${Date.now()}`; // bypass cache with this trick
+	}
+
+	return cached.src;
 }
 
 /**
@@ -98,9 +122,8 @@ export class ResourceViewer {
 			$(container)
 				.empty()
 				.addClass('image')
-				.img({
-					src: descriptor.resource.toString() // disabled due to https://github.com/electron/electron/issues/6275  + '?' + Date.now() // We really want to avoid the browser from caching this resource, so we add a fake query param that is unique
-				}).on(DOM.EventType.LOAD, (e, img) => {
+				.img({ src: imageSrc(descriptor) })
+				.on(DOM.EventType.LOAD, (e, img) => {
 					const imgElement = <HTMLImageElement>img.getHTMLElement();
 					if (imgElement.naturalWidth > imgElement.width || imgElement.naturalHeight > imgElement.height) {
 						$(container).addClass('oversized');
@@ -119,47 +142,6 @@ export class ResourceViewer {
 				});
 		}
 
-		// Embed Object (only PDF for now)
-		else if (false /* PDF is currently not supported in Electron it seems */ && mime.indexOf('pdf') >= 0) {
-			$(container)
-				.empty()
-				.element('object')
-				.attr({
-					data: descriptor.resource.toString(), // disabled due to https://github.com/electron/electron/issues/6275  + '?' + Date.now(), // We really want to avoid the browser from caching this resource, so we add a fake query param that is unique
-					width: '100%',
-					height: '100%',
-					type: mime
-				});
-		}
-
-		// Embed Audio (if supported in browser)
-		else if (false /* disabled due to unknown impact on memory usage */ && mime.indexOf('audio/') >= 0) {
-			$(container)
-				.empty()
-				.element('audio')
-				.attr({
-					src: descriptor.resource.toString(), // disabled due to https://github.com/electron/electron/issues/6275  + '?' + Date.now(), // We really want to avoid the browser from caching this resource, so we add a fake query param that is unique
-					text: nls.localize('missingAudioSupport', "Sorry but playback of audio files is not supported."),
-					controls: 'controls'
-				}).on(DOM.EventType.LOAD, () => {
-					scrollbar.scanDomNode();
-				});
-		}
-
-		// Embed Video (if supported in browser)
-		else if (false /* disabled due to unknown impact on memory usage */ && mime.indexOf('video/') >= 0) {
-			$(container)
-				.empty()
-				.element('video')
-				.attr({
-					src: descriptor.resource.toString(), // disabled due to https://github.com/electron/electron/issues/6275 + '?' + Date.now(), // We really want to avoid the browser from caching this resource, so we add a fake query param that is unique
-					text: nls.localize('missingVideoSupport', "Sorry but playback of video files is not supported."),
-					controls: 'controls'
-				}).on(DOM.EventType.LOAD, () => {
-					scrollbar.scanDomNode();
-				});
-		}
-
 		// Handle generic Binary Files
 		else {
 			$(container)

src/vs/workbench/browser/parts/editor/binaryDiffEditor.ts

@@ -109,15 +109,15 @@ export class BinaryResourceDiffEditor extends BaseEditor implements IVerticalSas
 
 			// Render original
 			let original = <BinaryEditorModel>resolvedModel.originalModel;
-			this.renderInput(original.getName(), original.getResource(), original.getSize(), true);
+			this.renderInput(original.getName(), original.getResource(), original.getSize(), original.getETag(), true);
 
 			// Render modified
 			let modified = <BinaryEditorModel>resolvedModel.modifiedModel;
-			this.renderInput(modified.getName(), modified.getResource(), modified.getSize(),  false);
+			this.renderInput(modified.getName(), modified.getResource(), modified.getSize(), modified.getETag(), false);
 		});
 	}
 
-	private renderInput(name: string, resource: URI, size: number, isOriginal: boolean): void {
+	private renderInput(name: string, resource: URI, size: number, etag: string, isOriginal: boolean): void {
 
 		// Reset Sash to default 50/50 ratio if needed
 		if (this.leftContainerWidth && this.dimension && this.leftContainerWidth !== this.dimension.width / 2) {
@@ -130,7 +130,7 @@ export class BinaryResourceDiffEditor extends BaseEditor implements IVerticalSas
 		let container = isOriginal ? this.leftBinaryContainer : this.rightBinaryContainer;
 		let scrollbar = isOriginal ? this.leftScrollbar : this.rightScrollbar;
 
-		ResourceViewer.show({ name, resource, size }, container, scrollbar);
+		ResourceViewer.show({ name, resource, size, etag }, container, scrollbar);
 	}
 
 	public clearInput(): void {

src/vs/workbench/browser/parts/editor/binaryEditor.ts

@@ -76,7 +76,7 @@ export abstract class BaseBinaryResourceEditor extends BaseEditor {
 
 			// Render Input
 			let model = <BinaryEditorModel>resolvedModel;
-			ResourceViewer.show({ name: model.getName(), resource: model.getResource(), size: model.getSize() }, this.binaryContainer, this.scrollbar);
+			ResourceViewer.show({ name: model.getName(), resource: model.getResource(), size: model.getSize(), etag: model.getETag() }, this.binaryContainer, this.scrollbar);
 
 			return TPromise.as<void>(null);
 		});

src/vs/workbench/common/editor/binaryEditorModel.ts

@@ -16,6 +16,7 @@ export class BinaryEditorModel extends EditorModel {
 	private name: string;
 	private resource: URI;
 	private size: number;
+	private etag: string;
 
 	constructor(
 		resource: URI,
@@ -49,8 +50,16 @@ export class BinaryEditorModel extends EditorModel {
 		return this.size;
 	}
 
+	/**
+	 * The etag of the binary file if known.
+	 */
+	public getETag(): string {
+		return this.etag;
+	}
+
 	public load(): TPromise<EditorModel> {
 		return this.fileService.resolveFile(this.resource).then(stat => {
+			this.etag = stat.etag;
 			this.size = stat.size;
 
 			return this;