Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLeo Ufimtsev2017-03-21 19:03:27 +0000
committerLeo Ufimtsev2017-03-21 19:03:27 +0000
commit3f92612774ff0dda4e8e9c86127bb0f3a880c329 (patch)
tree349f12c3a4d7ff60e1675d7f5ace81b6baf08198 /bundles
parent8bd9620059daff78087c993d22fdbe714fe2d2ac (diff)
downloadeclipse.platform.swt-3f92612774ff0dda4e8e9c86127bb0f3a880c329.tar.gz
eclipse.platform.swt-3f92612774ff0dda4e8e9c86127bb0f3a880c329.tar.xz
eclipse.platform.swt-3f92612774ff0dda4e8e9c86127bb0f3a880c329.zip
Bug 503431 Mouse button release event memory leak fix
The previous patch introduced a memory leak as a new callback was instantiated but never disposed. After investigation, that callback is redundant as the only thing it did was to return 'false'. This can be achieved by linking it to gtk_false() instead. Tasks performed: 1) Removed redundant callback in Display.java. 2) Linked Tree/Table/List to gtk_false() instead. 2.b) Implemented custom getter that gets the function pointer to gtk_false. Tested via DNDExample, with Table/List/Tree you can still select multiple items and drag multiple items. (without the calls, you cannot). Gtk3: all jUnits pass. Change-Id: I9d144504acdf3548b2a11ca10dcc19d4fe42d3c6 Signed-off-by: Leo Ufimtsev <lufimtse@redhat.com>
Diffstat (limited to 'bundles')
-rw-r--r--bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c12
-rw-r--r--bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_custom.h8
-rw-r--r--bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c1
-rw-r--r--bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h1
-rw-r--r--bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java16
-rw-r--r--bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java13
-rw-r--r--bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/List.java7
-rw-r--r--bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java7
-rw-r--r--bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java7
9 files changed, 53 insertions, 19 deletions
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c
index 838b847258..29799a9df7 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c
@@ -966,6 +966,18 @@ JNIEXPORT jintLong JNICALL OS_NATIVE(_1GDK_1TYPE_1PIXBUF)
}
#endif
+#ifndef NO__1GET_1FUNCTION_1POINTER_1gtk_1false
+JNIEXPORT jintLong JNICALL OS_NATIVE(_1GET_1FUNCTION_1POINTER_1gtk_1false)
+ (JNIEnv *env, jclass that)
+{
+ jintLong rc = 0;
+ OS_NATIVE_ENTER(env, that, _1GET_1FUNCTION_1POINTER_1gtk_1false_FUNC);
+ rc = (jintLong)GET_FUNCTION_POINTER_gtk_false();
+ OS_NATIVE_EXIT(env, that, _1GET_1FUNCTION_1POINTER_1gtk_1false_FUNC);
+ return rc;
+}
+#endif
+
#ifndef NO__1GString_1len
JNIEXPORT jint JNICALL OS_NATIVE(_1GString_1len)
(JNIEnv *env, jclass that, jintLong arg0)
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_custom.h b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_custom.h
index 468424e89e..d63bb47870 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_custom.h
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_custom.h
@@ -205,6 +205,7 @@
#define gtk_scrolled_window_add_with_viewport_LIB LIB_GTK
#define gtk_settings_set_string_property_LIB LIB_GTK
#define gtk_status_icon_position_menu_LIB LIB_GTK
+#define gtk_false_LIB LIB_GTK
#define gtk_window_get_opacity_LIB LIB_GTK
#define gdk_window_create_similar_surface_LIB LIB_GDK
#define gdk_window_restack_LIB LIB_GDK
@@ -401,6 +402,13 @@ struct _GtkAccelLabelPrivate
#define gtk_style_get_ythickness(arg0) (arg0)->ythickness
#define localeconv_decimal_point() localeconv()->decimal_point
+// Mechanism to get function pointers of C/gtk functions back to java.
+// Idea is that you substitute the return value with the function pointer.
+// NOTE: functions like gtk_false need to be linked to a lib. Eg see gtk_false_LIB above.
+#define GET_FUNCTION_POINTER_gtk_false() 0; \
+OS_LOAD_FUNCTION(fp, gtk_false) \
+rc = (jintLong)fp;
+
#define gtk_status_icon_position_menu_func() 0; \
OS_LOAD_FUNCTION(fp, gtk_status_icon_position_menu) \
rc = (jintLong)fp;
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c
index 68a28c34d4..9bc1fe8c2d 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c
@@ -108,6 +108,7 @@ char * OS_nativeFunctionNames[] = {
"_1GDK_1PIXMAP_1XID",
"_1GDK_1TYPE_1COLOR",
"_1GDK_1TYPE_1PIXBUF",
+ "_1GET_1FUNCTION_1POINTER_1gtk_1false",
"_1GString_1len",
"_1GString_1str",
"_1GTK_1ACCESSIBLE",
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h
index 6b3143bd54..0a14caea6a 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h
@@ -118,6 +118,7 @@ typedef enum {
_1GDK_1PIXMAP_1XID_FUNC,
_1GDK_1TYPE_1COLOR_FUNC,
_1GDK_1TYPE_1PIXBUF_FUNC,
+ _1GET_1FUNCTION_1POINTER_1gtk_1false_FUNC,
_1GString_1len_FUNC,
_1GString_1str_FUNC,
_1GTK_1ACCESSIBLE_FUNC,
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java
index 9cb2962c34..c672271656 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java
@@ -8498,6 +8498,9 @@ public static final long /*int*/ gtk_separator_new(int orientation) {
lock.unlock();
}
}
+
+// Get function pointer to gtk_status_icon_position_menu
+// See os_custom.h
public static final native long /*int*/ _gtk_status_icon_position_menu_func();
public static final long /*int*/ gtk_status_icon_position_menu_func() {
lock.lock();
@@ -8507,6 +8510,19 @@ public static final long /*int*/ gtk_status_icon_position_menu_func() {
lock.unlock();
}
}
+
+// See os_custom.h
+// Dynamically get's the function pointer to gtk_false(). Gtk2/Gtk3.
+public static final native long /*int*/ _GET_FUNCTION_POINTER_gtk_false();
+public static final long /*int*/ GET_FUNCTION_POINTER_gtk_false() {
+ lock.lock();
+ try {
+ return _GET_FUNCTION_POINTER_gtk_false();
+ } finally {
+ lock.unlock();
+ }
+}
+
/**
* @method flags=dynamic [GTK2/GTK3; 3.8 deprecated]
*/
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java
index 8dfe0460a2..c9cef5d410 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java
@@ -226,9 +226,6 @@ public class Display extends Device {
long /*int*/ cellDataProc;
Callback cellDataCallback;
- /* GtkTreeSelection callbacks for multiselection DnD */
- long /*int*/ selectionProc;
- Callback selectionCallback;
/* Set direction callback */
long /*int*/ setDirectionProc;
@@ -809,12 +806,6 @@ long /*int*/ cellDataProc (long /*int*/ tree_column, long /*int*/ cell, long /*i
return widget.cellDataProc (tree_column, cell, tree_model, iter, data);
}
-/*
- * Feature in GTK. GtkTreeSelectionFunc* will return false so that multiselection on tree items using DND will work. Refer to Bug 503431
- */
-long /*int*/ selectionProc (long /*int*/ selection, long /*int*/ model, long /*int*/ path, long /*int*/ path_currently_selected, long /*int*/ data) {
- return 0;
-}
@Override
protected void checkDevice () {
@@ -3397,10 +3388,6 @@ void initializeCallbacks () {
cellDataProc = cellDataCallback.getAddress ();
if (cellDataProc == 0) error (SWT.ERROR_NO_MORE_CALLBACKS);
- selectionCallback = new Callback (this, "selectionProc", 5); //$NON-NLS-1$
- selectionProc = selectionCallback.getAddress ();
- if (selectionProc == 0) error (SWT.ERROR_NO_MORE_CALLBACKS);
-
setDirectionCallback = new Callback (this, "setDirectionProc", 2); //$NON-NLS-1$
setDirectionProc = setDirectionCallback.getAddress ();
if (setDirectionProc == 0) error (SWT.ERROR_NO_MORE_CALLBACKS);
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/List.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/List.java
index d5dec01d53..5c5a1ee7fd 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/List.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/List.java
@@ -826,11 +826,14 @@ long /*int*/ gtk_button_press_event (long /*int*/ widget, long /*int*/ event) {
if (((gdkEvent.state & (OS.GDK_CONTROL_MASK|OS.GDK_SHIFT_MASK)) == 0) ||
((gdkEvent.state & OS.GDK_CONTROL_MASK) != 0)) {
/**
- * disable selection on a mouse click if there are multiple items already selected. Also,
+ * Disable selection on a mouse click if there are multiple items already selected. Also,
* if control is currently being held down, we will designate the selection logic over to release
* instead by first disabling the selection.
+ * E.g to reproduce: Open DNDExample, select "List", select multiple items, try dragging.
+ * without line below, only one item is selected for drag.
*/
- OS.gtk_tree_selection_set_select_function(selection,display.selectionProc,0,0);
+ long /*int*/ gtk_false_funcPtr = OS.GET_FUNCTION_POINTER_gtk_false();
+ OS.gtk_tree_selection_set_select_function(selection, gtk_false_funcPtr, 0, 0);
}
}
}
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java
index a88c142fc6..4885bc3ce3 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java
@@ -1999,11 +1999,14 @@ long /*int*/ gtk_button_press_event (long /*int*/ widget, long /*int*/ event) {
if (((gdkEvent.state & (OS.GDK_CONTROL_MASK|OS.GDK_SHIFT_MASK)) == 0) ||
((gdkEvent.state & OS.GDK_CONTROL_MASK) != 0)) {
/**
- * disable selection on a mouse click if there are multiple items already selected. Also,
+ * Disable selection on a mouse click if there are multiple items already selected. Also,
* if control is currently being held down, we will designate the selection logic over to release
* instead by first disabling the selection.
+ * E.g to reproduce: Open DNDExample, select "Table", select multiple items, try dragging.
+ * without line below, only one item is selected for drag.
*/
- OS.gtk_tree_selection_set_select_function(selection,display.selectionProc,0,0);
+ long /*int*/ gtk_false_funcPtr = OS.GET_FUNCTION_POINTER_gtk_false();
+ OS.gtk_tree_selection_set_select_function(selection, gtk_false_funcPtr, 0, 0);
}
}
}
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java
index f9c20b4981..416eadb18b 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java
@@ -2000,11 +2000,14 @@ long /*int*/ gtk_button_press_event (long /*int*/ widget, long /*int*/ event) {
if (((gdkEvent.state & (OS.GDK_CONTROL_MASK|OS.GDK_SHIFT_MASK)) == 0) ||
((gdkEvent.state & OS.GDK_CONTROL_MASK) != 0)) {
/**
- * disable selection on a mouse click if there are multiple items already selected. Also,
+ * Disable selection on a mouse click if there are multiple items already selected. Also,
* if control is currently being held down, we will designate the selection logic over to release
* instead by first disabling the selection.
+ * E.g to reproduce: Open DNDExample, select "Tree", select multiple items, try dragging.
+ * without line below, only one item is selected for drag.
*/
- OS.gtk_tree_selection_set_select_function(selection,display.selectionProc,0,0);
+ long /*int*/ gtk_false_funcPtr = OS.GET_FUNCTION_POINTER_gtk_false();
+ OS.gtk_tree_selection_set_select_function(selection, gtk_false_funcPtr, 0, 0);
}
}
}

Back to the top