[win32] Adapt/copy all handles on Image copy

[akoch-yatta]

This PR adapts and copies all handles in the copy constructor in Image. As each image now manages possibly more than one OS handles at the same time in the windows implementation, it is important to apply the operation on all of them.

[HeikoKlare]

The change looks sound. Just wondering if it is possible and would make sense to move the handle copy logic to the ImageHandle class? I.e., having an ImageHandle::copy operation or the like to extract the rather complex functionaltiy from the constructor?

[github-actions]

Test Results

   539 files   -  6     539 suites   - 6   30m 24s ⏱️ - 1m 38s
 4 339 tests  - 37   4 323 ✅  - 35   15 💤  - 3  1 ❌ +1 
16 606 runs   - 37  16 468 ✅  - 35  137 💤  - 3  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 570f321. ± Comparison against base commit 44f0d56.

This pull request removes 37 tests.

AllWin32Tests org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageDataForDifferentFractionalZoomsShouldBeDifferent
AllWin32Tests org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageShouldHaveDimesionAsPerZoomLevel
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testByteArrayTransfer
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testFileTransfer
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testHtmlTransfer
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromCopiedImage
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImage
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImageData
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImageDataFromImage
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testRtfTransfer
…

♻️ This comment has been updated with latest results.

[akoch-yatta]

The change looks sound. Just wondering if it is possible and would make sense to move the handle copy logic to the ImageHandle class? I.e., having an ImageHandle::copy operation or the like to extract the rather complex functionaltiy from the constructor?

Sure, we could. Only argument from my side against it would be that it is just not used anywhere else. But I would be fine with both.

[HeikoKlare]

Sure, we could. Only argument from my side against it would be that it is just not used anywhere else.

That's true. But wouldn't you say it's more a concern of an ImageHandle to implement how it has to be copied rather than the concern of a consumer (in this case the Image constructor requiring a copy of the handle)? I always found that Image constructor rather complex and now that it is refactored to obviously copy multiple handles it seems quite natural to me to have than be encapsulated in the ImageHandle.
But in the end, this is more about readability and encapsulation of concerns than about functionality.

[fedejeanne]

The changes look fine, I'm just missing some way to test it more "heavily". By playing around with the Runtime Workspace Eclipse Application, I only seem to navigate the case SWT.IMAGE_DISABLE but not the other ones. @akoch-yatta do you have any Snippet or use case that tests the other cases in the switch?

I have no strong opinion regarding whether or not the code should be moved to ImageHandle so it's your call :-)

[HeikoKlare]

Since the case distinction for bitmap/icon would need to be replaced in the ImageHandle copy and in the Image constructor, let's keep it like it is for now.

Failing test is unrelated: #1843