[Win32] Ensure Image#getImageData() returns no internal data

A caching mechanism for ImageData introduced to some provider
implementations inside the Image class leads to Image#getImageData()
returning an ImageData object, which if modified affects the underlying
Image. This contradicts the contract of that method, which states that
changes to that object will not affect the Image.

This change ensures that Image#getImagetData() only returns objects that
are not used internal of the Image instance. To this end, clones of the
ImageData are created where necessary.

Author: HeikoKlare

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java

@@ -2017,7 +2017,7 @@ ImageData newImageData(ZoomContext zoomContext) {
 			handle.destroy();
 			return data;
 		};
-		return cachedImageData.computeIfAbsent(zoomContext.targetZoom(), imageDataRetrieval);
+		return (ImageData) cachedImageData.computeIfAbsent(zoomContext.targetZoom(), imageDataRetrieval).clone();
 	}
 
 	@Override
@@ -2297,7 +2297,7 @@ ImageData newImageData(ZoomContext zoomContext) {
 			handle.destroy();
 			return data;
 		};
-		return cachedImageData.computeIfAbsent(zoomContext.targetZoom(), imageDataRetrival);
+		return (ImageData) cachedImageData.computeIfAbsent(zoomContext.targetZoom(), imageDataRetrival).clone();
 	}
 
 

tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_Image.java

@@ -20,7 +20,6 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.assumeFalse;
@@ -33,6 +32,8 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Comparator;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Consumer;
 
 import org.eclipse.swt.SWT;
@@ -683,6 +684,25 @@ public void test_getImageData() {
 	getImageData2(32, new PaletteData(0xff0000, 0xff00, 0xff));
 }
 
+@Test
+public void test_getImageData_changingImageDataDoesNotAffectImage() {
+	List<Image> images = List.of( //
+			new Image(display, imageFileNameProvider), //
+			new Image(display, imageDataProvider), //
+			new Image(display, new ImageData(10, 10, 32, new PaletteData(0xff0000, 0xff00, 0xff))) //
+	);
+
+	try {
+		for (Image image : images) {
+			ImageData originalImageData = image.getImageData();
+			originalImageData.setPixel(0, 0, originalImageData.getPixel(0, 0) + 1);
+			assertNotEquals(image.getImageData().getPixel(0, 0), originalImageData.getPixel(0, 0));
+		}
+	} finally {
+		images.forEach(Image::dispose);
+	}
+}
+
 @Test
 public void test_getImageData_100() {
         getImageData_int(100);
@@ -1032,11 +1052,17 @@ public void test_updateWidthHeightAfterDPIChange() {
 public void test_imageDataIsCached() {
 	assumeTrue("On-demand image creation only implemented for Windows", SwtTestUtil.isWindows);
 	String imagePath = getPath("collapseall.png");
+	AtomicInteger callCount = new AtomicInteger();
 	ImageFileNameProvider imageFileNameProvider = __ -> {
+		callCount.incrementAndGet();
 		return imagePath;
 	};
 	Image fileNameProviderImage = new Image(display, imageFileNameProvider);
-	assertSame(fileNameProviderImage.getImageData(100), fileNameProviderImage.getImageData(100));
+	callCount.set(0);
+	fileNameProviderImage.getImageData(100);
+	fileNameProviderImage.getImageData(100);
+	fileNameProviderImage.getImageData(100);
+	assertEquals(0, callCount.get());
 }
 
 @Test