Bug 487467: [GTK3] org_eclipse_swt_widgets_Text.test_getTopIndex fails

This patch addresses a short-fall in GTK whereby Text widgets suffer
from a delay in calculating their top index. The root cause of the issue
lies in the fact that GtkTextView does line validation -- which runs in
an idle handler. This causes inaccurate values to be calculated. This
only affects GTK3.

The fix is to use a GtkTextMark on GTK3 to track the position of the
index in the buffer. GtkTextMarks are not subject to line validation.
This allows setTopIndex() and getTopIndex() to be called one after
another without issue. 

For cases when the user scrolls with the GUI (either by scrollbar or by
moving the cursor), getTopIndex() monitors the GtkAdjustment of the
vertical scrollbar. If there is any change in value, the old method of
calculation will be used. This method fetches the visible rectangle of
the GtkTextView and calculates the index of the top line.

Tested on GTK3.20, 3.18, 3.16, 3.14, and 2.24. This fixes a failing
AllNonBrowser test case on GTK3, and introduces no additional failures
on GTK2.

Change-Id: Ia015971786772579dfdca40d1091c6a40c6260cc
Signed-off-by: Eric Williams <ericwill@redhat.com>

7 files changed, 113 insertions, 1 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 10bc606e0a..788d2f6970 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
@@ -14975,6 +14975,26 @@ JNIEXPORT void JNICALL OS_NATIVE(_1gtk_1scale_1set_1draw_1value)
 }
 #endif
 
+#ifndef NO__1gtk_1scrollable_1get_1vadjustment
+JNIEXPORT jlong JNICALL OS_NATIVE(_1gtk_1scrollable_1get_1vadjustment)
+	(JNIEnv *env, jclass that, jintLong arg0)
+{
+	jlong rc = 0;
+	OS_NATIVE_ENTER(env, that, _1gtk_1scrollable_1get_1vadjustment_FUNC);
+/*
+	rc = (jlong)gtk_scrollable_get_vadjustment((GtkScrollable *)arg0);
+*/
+	{
+		OS_LOAD_FUNCTION(fp, gtk_scrollable_get_vadjustment)
+		if (fp) {
+			rc = (jlong)((jlong (CALLING_CONVENTION*)(GtkScrollable *))fp)((GtkScrollable *)arg0);
+		}
+	}
+	OS_NATIVE_EXIT(env, that, _1gtk_1scrollable_1get_1vadjustment_FUNC);
+	return rc;
+}
+#endif
+
 #ifndef NO__1gtk_1scrollbar_1new
 JNIEXPORT jintLong JNICALL OS_NATIVE(_1gtk_1scrollbar_1new)
 	(JNIEnv *env, jclass that, jint arg0, jintLong arg1)
@@ -16174,6 +16194,25 @@ JNIEXPORT void JNICALL OS_NATIVE(_1gtk_1text_1buffer_1copy_1clipboard)
 }
 #endif
 
+#ifndef NO__1gtk_1text_1buffer_1create_1mark
+JNIEXPORT jintLong JNICALL OS_NATIVE(_1gtk_1text_1buffer_1create_1mark)
+	(JNIEnv *env, jclass that, jintLong arg0, jbyteArray arg1, jbyteArray arg2, jboolean arg3)
+{
+	jbyte *lparg1=NULL;
+	jbyte *lparg2=NULL;
+	jintLong rc = 0;
+	OS_NATIVE_ENTER(env, that, _1gtk_1text_1buffer_1create_1mark_FUNC);
+	if (arg1) if ((lparg1 = (*env)->GetByteArrayElements(env, arg1, NULL)) == NULL) goto fail;
+	if (arg2) if ((lparg2 = (*env)->GetByteArrayElements(env, arg2, NULL)) == NULL) goto fail;
+	rc = (jintLong)gtk_text_buffer_create_mark((GtkTextBuffer *)arg0, (const gchar *)lparg1, (GtkTextIter *)lparg2, (gboolean)arg3);
+fail:
+	if (arg2 && lparg2) (*env)->ReleaseByteArrayElements(env, arg2, lparg2, 0);
+	if (arg1 && lparg1) (*env)->ReleaseByteArrayElements(env, arg1, lparg1, 0);
+	OS_NATIVE_EXIT(env, that, _1gtk_1text_1buffer_1create_1mark_FUNC);
+	return rc;
+}
+#endif
+
 #ifndef NO__1gtk_1text_1buffer_1cut_1clipboard
 JNIEXPORT void JNICALL OS_NATIVE(_1gtk_1text_1buffer_1cut_1clipboard)
 	(JNIEnv *env, jclass that, jintLong arg0, jintLong arg1, jboolean arg2)
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.h b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.h
index c9ea289287..a4f40497f5 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.h
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.h
@@ -189,6 +189,7 @@
 #define NO_memmove__JLorg_eclipse_swt_internal_gtk_GdkRGBA_2J
 #define NO__1gdk_1rgba_1to_1string
 #define NO__1gdk_1rgba_1parse
+#define NO__1gtk_1scrollable_1get_1vadjustment
 
 #define NO__1g_1object_1set__I_3BLorg_eclipse_swt_internal_gtk_GdkRGBA_2I
 #define NO__1g_1object_1set__J_3BLorg_eclipse_swt_internal_gtk_GdkRGBA_2J
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 7e41c098c9..e497775374 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
@@ -126,6 +126,7 @@
 #define gtk_adjustment_get_value_LIB LIB_GTK
 #define gtk_adjustment_set_page_increment_LIB LIB_GTK
 #define gtk_adjustment_set_step_increment_LIB LIB_GTK
+#define gtk_scrollable_get_vadjustment_LIB LIB_GTK
 #define gtk_arrow_new_LIB LIB_GTK
 #define gtk_arrow_set_LIB LIB_GTK
 #define gtk_box_new_LIB LIB_GTK
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 092cbad9d9..b7eb2fcb31 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
@@ -1084,6 +1084,7 @@ char * OS_nativeFunctionNames[] = {
 	"_1gtk_1scale_1new",
 	"_1gtk_1scale_1set_1digits",
 	"_1gtk_1scale_1set_1draw_1value",
+	"_1gtk_1scrollable_1get_1vadjustment",
 	"_1gtk_1scrollbar_1new",
 	"_1gtk_1scrolled_1window_1add_1with_1viewport",
 	"_1gtk_1scrolled_1window_1get_1hadjustment",
@@ -1165,6 +1166,7 @@ char * OS_nativeFunctionNames[] = {
 	"_1gtk_1target_1list_1new",
 	"_1gtk_1target_1list_1unref",
 	"_1gtk_1text_1buffer_1copy_1clipboard",
+	"_1gtk_1text_1buffer_1create_1mark",
 	"_1gtk_1text_1buffer_1cut_1clipboard",
 	"_1gtk_1text_1buffer_1delete",
 	"_1gtk_1text_1buffer_1get_1bounds",
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 33e0bfdc55..f36ea694cf 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
@@ -1094,6 +1094,7 @@ typedef enum {
 	_1gtk_1scale_1new_FUNC,
 	_1gtk_1scale_1set_1digits_FUNC,
 	_1gtk_1scale_1set_1draw_1value_FUNC,
+	_1gtk_1scrollable_1get_1vadjustment_FUNC,
 	_1gtk_1scrollbar_1new_FUNC,
 	_1gtk_1scrolled_1window_1add_1with_1viewport_FUNC,
 	_1gtk_1scrolled_1window_1get_1hadjustment_FUNC,
@@ -1175,6 +1176,7 @@ typedef enum {
 	_1gtk_1target_1list_1new_FUNC,
 	_1gtk_1target_1list_1unref_FUNC,
 	_1gtk_1text_1buffer_1copy_1clipboard_FUNC,
+	_1gtk_1text_1buffer_1create_1mark_FUNC,
 	_1gtk_1text_1buffer_1cut_1clipboard_FUNC,
 	_1gtk_1text_1buffer_1delete_FUNC,
 	_1gtk_1text_1buffer_1get_1bounds_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 7fe2331cc6..114630ca83 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
@@ -11335,6 +11335,19 @@ public static final void gtk_scale_set_draw_value(long /*int*/ scale, boolean dr
 	}
 }
 /**
+ * @method flags=dynamic
+ * @param scrollable cast=(GtkScrollable *)
+ */
+public static final native long _gtk_scrollable_get_vadjustment(long /*int*/ scrollable);
+public static final long gtk_scrollable_get_vadjustment(long /*int*/ scrollable) {
+	lock.lock();
+	try {
+		return _gtk_scrollable_get_vadjustment(scrollable);
+	} finally {
+		lock.unlock();
+	}
+}
+/**
  * @param style cast=(GtkRcStyle *)
  * @param color flags=no_out
  */
@@ -12257,6 +12270,21 @@ public static final void gtk_text_buffer_copy_clipboard(long /*int*/ buffer, lon
 }
 /**
  * @param buffer cast=(GtkTextBuffer *)
+ * @param mark_name cast=(const gchar *)
+ * @param where cast=(GtkTextIter *)
+ * @param left_gravity cast=(gboolean)
+ */
+public static final native long /*int*/ _gtk_text_buffer_create_mark(long /*int*/ buffer, byte [] mark_name, byte [] where, boolean left_gravity);
+public static final long /*int*/ gtk_text_buffer_create_mark(long /*int*/ buffer, byte [] mark_name, byte [] where, boolean left_gravity) {
+	lock.lock();
+	try {
+		return _gtk_text_buffer_create_mark(buffer, mark_name, where, left_gravity);
+	} finally {
+		lock.unlock();
+	}
+}
+/**
+ * @param buffer cast=(GtkTextBuffer *)
  * @param clipboard cast=(GtkClipboard *)
  * @param default_editable cast=(gboolean)
  */
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Text.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Text.java
index 371e421ddd..c7d182dfd9 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Text.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Text.java
@@ -102,6 +102,8 @@ public class Text extends Scrollable {
 	 * a global variable to keep track of its background color.
 	 */
 	GdkRGBA background;
+	long /*int*/ indexMark;
+	double cachedAdjustment, currentAdjustment;
 
 /**
  * Constructs a new instance of this class given its parent
@@ -1402,6 +1404,27 @@ public int getTopIndex () {
 	checkWidget ();
 	if ((style & SWT.SINGLE) != 0) return 0;
 	byte [] position = new byte [ITER_SIZEOF];
+	/*
+	 * Feature in GTK: GtkTextView widgets are subject to line validation
+	 * which happens during idle. This causes GtkTextIter to not update quickly
+	 * enough when changes are added to the text buffer. The fix is to use a
+	 * GtkTextMark to track the precise index, then convert it back to a
+	 * GtkTextIter when getTopIndex() is called. See bug 487467.
+	 *
+	 * NOTE: to cover cases where getTopIndex() is called without setTopIndex()
+	 * being called, we fetch the current GtkAdjustment value and cache it for
+	 * comparison. In getTopIndex() we compare the current value with the cached
+	 * one to see if the user has scrolled/moved the viewport using the GUI.
+	 * If so, we use the old method of fetching the top index.
+	 */
+	if (OS.GTK3) {
+		long /*int*/ vAdjustment = OS.gtk_scrollable_get_vadjustment (handle);
+		currentAdjustment = OS.gtk_adjustment_get_value (vAdjustment);
+		if (cachedAdjustment == currentAdjustment) {
+			OS.gtk_text_buffer_get_iter_at_mark (bufferHandle, position, indexMark);
+			return OS.gtk_text_iter_get_line (position);
+		}
+	}
 	GdkRectangle rect = new GdkRectangle ();
 	OS.gtk_text_view_get_visible_rect (handle, rect);
 	OS.gtk_text_view_get_line_at_y (handle, position, rect.y, null);
@@ -2710,7 +2733,23 @@ public void setTopIndex (int index) {
 	if ((style & SWT.SINGLE) != 0) return;
 	byte [] position = new byte [ITER_SIZEOF];
 	OS.gtk_text_buffer_get_iter_at_line (bufferHandle, position, index);
-	OS.gtk_text_view_scroll_to_iter (handle, position, 0, true, 0, 0);
+	if (OS.GTK3) {
+		/*
+		 * Feature in GTK: create a new GtkTextMark for the purposes of
+		 * keeping track of the top index. In getTopIndex() we can use this
+		 * without worrying about line validation. See bug 487467.
+		 *
+		 * We also cache the current GtkAdjustment value for future comparison
+		 * in getTopIndex().
+		 */
+		byte [] buffer = Converter.wcsToMbcs (null, "index_mark", true);
+		indexMark = OS.gtk_text_buffer_create_mark (bufferHandle, buffer, position, true);
+		OS.gtk_text_view_scroll_to_mark (handle, indexMark, 0, true, 0, 0);
+		long /*int*/ vAdjustment = OS.gtk_scrollable_get_vadjustment (handle);
+		cachedAdjustment = OS.gtk_adjustment_get_value (vAdjustment);
+	} else {
+		OS.gtk_text_view_scroll_to_iter (handle, position, 0, true, 0, 0);
+	}
 }
 
 /**