Skip to content

Commit 67642bd

Browse files
jonahgrahamakurtakov
authored andcommitted
[GTK4] Clone instead of reusing objects on local clipbpard operations
When calling gdk_clipboard_read_value_async and related functions, if the clipboard is owned locally it returns the instance that was placed on the clipboard and avoid the serialization and deserialization. This presents a problem for SWT because SWT has always returned a copy (javaToNative/nativeToJava) of the object between the setContents and the getContents. For some objects, like String it may be ok to return the same instance since String is immutable, but for others, such as ImageData or custom types a copy definitely needs to be made to preserve the pre-existing behavior. However, to maintain compatibility with the pre-existing implementations for even String type Transfers that have returned a copy, we copy String types too in this implementation. (Includes removing invalid comment "Put a type on the clipboard and ensure we don't match it" in the test_internalClone tests) Part of #2126
1 parent d6e01f9 commit 67642bd

File tree

8 files changed

+51
-3
lines changed

8 files changed

+51
-3
lines changed

bundles/org.eclipse.swt/Eclipse SWT Drag and Drop/gtk/org/eclipse/swt/dnd/ContentProviders.java

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,8 @@ public Object getObject(long gvalue) {
457457
// Object is from within app, so get contents from clipboard data
458458
// Do not remove our reference to the object yet as it may be copied
459459
// multiple times
460-
return fromSourceId.data.get(transferKey(transfer));
460+
Object object = fromSourceId.data.get(transferKey(transfer));
461+
return clone(transfer, object);
461462
} else {
462463
// Object is from outside app or otherwise needed deserializing
463464
// Remove our reference to it and return it
@@ -471,4 +472,39 @@ public Object getObject(long gvalue) {
471472
// should have returned 0 in this case
472473
return null;
473474
}
475+
476+
/**
477+
* When calling gdk_clipboard_read_value_async and related functions, if the
478+
* clipboard is owned locally it returns the instance that was placed on the
479+
* clipboard and avoid the serialization and deserialization.
480+
*
481+
* This presents a problem for SWT because SWT has always returned a copy
482+
* (javaToNative/nativeToJava) of the object between the setContents and the
483+
* getContents.
484+
*
485+
* For some objects, like String it may be ok to return the same instance since
486+
* String is immutable, but for others, such as ImageData or custom types a copy
487+
* definitely needs to be made to preserve the pre-existing behavior. However,
488+
* to maintain compatibility with the pre-existing implementations for even
489+
* String type Transfers that have returned a copy, we copy String types too in
490+
* this implementation.
491+
*/
492+
private Object clone(Transfer transfer, Object object) {
493+
TransferData td = new TransferData();
494+
td.result = 0;
495+
int[] typeIds = transfer.getTypeIds();
496+
if (typeIds.length == 0 || typeIds[0] == 0) {
497+
return null;
498+
}
499+
// Which type id we use doesn't matter, as long as it is the same in both
500+
// directions
501+
td.type = typeIds[0];
502+
transfer.javaToNative(object, td);
503+
if (td.result == 0 || td.pValue == 0 || td.length == 0) {
504+
return null;
505+
}
506+
Object clonedObject = transfer.nativeToJava(td);
507+
OS.g_free(td.pValue);
508+
return clonedObject;
509+
}
474510
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import static org.junit.jupiter.api.Assertions.assertEquals;
1414
import static org.junit.jupiter.api.Assertions.assertFalse;
1515
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
16+
import static org.junit.jupiter.api.Assertions.assertNotSame;
1617
import static org.junit.jupiter.api.Assertions.assertThrows;
1718
import static org.junit.jupiter.api.Assertions.assertTrue;
1819

@@ -180,6 +181,7 @@ public void test_internalClone() throws Exception {
180181
setContents(test);
181182
MyType contents = getContents();
182183
assertMyTypeEquals(test, contents);
184+
assertNotSame(test, contents);
183185
}
184186

185187
@Test

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
1414
import static org.junit.jupiter.api.Assertions.assertFalse;
1515
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
16+
import static org.junit.jupiter.api.Assertions.assertNotSame;
1617
import static org.junit.jupiter.api.Assertions.assertThrows;
1718
import static org.junit.jupiter.api.Assertions.assertTrue;
1819

@@ -152,10 +153,10 @@ public void test_internalClone() throws Exception {
152153
String[] fileList = getFileList();
153154

154155
openAndFocusShell(false);
155-
// Put a type on the clipboard and ensure we don't match it
156156
setContents(fileList);
157157
String[] contents = getContents();
158158
assertArrayEquals(fileList, contents);
159+
assertNotSame(fileList, contents);
159160
}
160161

161162
@Test

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import static org.junit.jupiter.api.Assertions.assertEquals;
1414
import static org.junit.jupiter.api.Assertions.assertFalse;
1515
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
16+
import static org.junit.jupiter.api.Assertions.assertNotSame;
1617
import static org.junit.jupiter.api.Assertions.assertThrows;
1718
import static org.junit.jupiter.api.Assertions.assertTrue;
1819

@@ -122,6 +123,7 @@ public void test_internalClone() throws Exception {
122123
setContents(test);
123124
String contents = getContents();
124125
assertEquals(test, contents);
126+
assertNotSame(test, contents);
125127
}
126128

127129
static Stream<Arguments> testData() {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import static org.junit.jupiter.api.Assertions.assertEquals;
1515
import static org.junit.jupiter.api.Assertions.assertFalse;
1616
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
17+
import static org.junit.jupiter.api.Assertions.assertNotSame;
1718
import static org.junit.jupiter.api.Assertions.assertThrows;
1819
import static org.junit.jupiter.api.Assertions.assertTrue;
1920

@@ -128,10 +129,10 @@ public void test_internalClone() throws Exception {
128129
ImageData imageData = getImageData();
129130

130131
openAndFocusShell(false);
131-
// Put a type on the clipboard and ensure we don't match it
132132
setContents(imageData);
133133
ImageData contents = getContents();
134134
assertEquals(0, imageDataComparator().compare(imageData, contents));
135+
assertNotSame(imageData, contents);
135136
}
136137

137138
@Test

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import static org.junit.jupiter.api.Assertions.assertEquals;
1414
import static org.junit.jupiter.api.Assertions.assertFalse;
1515
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
16+
import static org.junit.jupiter.api.Assertions.assertNotSame;
1617
import static org.junit.jupiter.api.Assertions.assertThrows;
1718
import static org.junit.jupiter.api.Assertions.assertTrue;
1819

@@ -118,6 +119,7 @@ public void test_internalClone() throws Exception {
118119
setContents(test);
119120
String contents = getContents();
120121
assertEquals(test, contents);
122+
assertNotSame(test, contents);
121123
}
122124

123125
@Test

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import static org.junit.jupiter.api.Assertions.assertEquals;
1414
import static org.junit.jupiter.api.Assertions.assertFalse;
1515
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
16+
import static org.junit.jupiter.api.Assertions.assertNotSame;
1617
import static org.junit.jupiter.api.Assertions.assertThrows;
1718
import static org.junit.jupiter.api.Assertions.assertTrue;
1819

@@ -126,6 +127,7 @@ public void test_internalClone(String name, String test) throws Exception {
126127
setContents(test);
127128
String contents = getContents();
128129
assertEquals(test, contents);
130+
assertNotSame(test, contents);
129131
}
130132

131133
@ParameterizedTest(name = "{0}")

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import static org.junit.jupiter.api.Assertions.assertEquals;
1414
import static org.junit.jupiter.api.Assertions.assertFalse;
1515
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
16+
import static org.junit.jupiter.api.Assertions.assertNotSame;
1617
import static org.junit.jupiter.api.Assertions.assertThrows;
1718
import static org.junit.jupiter.api.Assertions.assertTrue;
1819

@@ -128,6 +129,7 @@ public void test_internalClone() throws Exception {
128129
setContents(test);
129130
String contents = getContents();
130131
assertEquals(test, contents);
132+
assertNotSame(test, contents);
131133
}
132134

133135
@Test

0 commit comments

Comments
 (0)