Bug 511228: [Webkit2] Ensure proper settings are used for webkit2.

1) Wrote fix so that setJavascriptEnabled() api now works on webkit2.

2) Note, added "assert" to a few methods. These do not run in production
code or in snippets (unless enabled manually), but they trigger when you
run jUnit tests.

Change-Id: Iac12c1ed9084f0720ff8441203dbc111cebfa6b1
Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=511228
Signed-off-by: Leo Ufimtsev <lufimtse@redhat.com>

2 files changed, 44 insertions, 8 deletions

diff --git a/bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/browser/WebKit.java b/bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/browser/WebKit.java
index 7ead7fedb5..faade5e38e 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/browser/WebKit.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/browser/WebKit.java
@@ -1287,6 +1287,13 @@ private static class Webkit2JavascriptEvaluator {
 @Override
 public Object evaluate (String script) throws SWTException {
 	if (WEBKIT2){
+        if (webkit_settings_get(WebKitGTK.enable_javascript) == 0) {
+        	// If you try to run javascript while scripts are turned off, then:
+        	// - on Webkit1: nothing happens.
+        	// - on Webkit2: an exception is thrown.
+        	// To ensure consistent behavior, do not even try to execute js on webkit2 if it's off.
+        	return null;
+        }
 		boolean doNotBlock = nonBlockingEvaluate > 0 ? true : false;
 		return Webkit2JavascriptEvaluator.evaluate(script, this.browser, webView, doNotBlock);
 	} else {
@@ -2323,6 +2330,7 @@ long /*int*/ webkit_mime_type_policy_decision_requested (long /*int*/ web_view,
 
 /** Webkit1 only */
 long /*int*/ webkit_navigation_policy_decision_requested (long /*int*/ web_view, long /*int*/ frame, long /*int*/ request, long /*int*/ navigation_action, long /*int*/ policy_decision) {
+	assert !WEBKIT2 : "Webkit1 only code was ran by webkit2";
 	if (loadingText) {
 		/*
 		 * WebKit is auto-navigating to about:blank in response to a
@@ -2366,8 +2374,7 @@ long /*int*/ webkit_navigation_policy_decision_requested (long /*int*/ web_view,
 		if (jsEnabled != jsEnabledOnNextPage) {
 			jsEnabled = jsEnabledOnNextPage;
 			DisabledJSCount += !jsEnabled ? 1 : -1;
-			long /*int*/ settings = WebKitGTK.webkit_web_view_get_settings (webView);
-			OS.g_object_set (settings, WebKitGTK.enable_scripts, jsEnabled ? 1 : 0, 0);
+			webkit_settings_set(WebKitGTK.enable_scripts, jsEnabled ? 1 : 0);
 		}
 
 		/* hook status change signal if frame is a newly-created sub-frame */
@@ -2396,7 +2403,8 @@ long /*int*/ webkit_navigation_policy_decision_requested (long /*int*/ web_view,
 
 /** Webkit2 only */
 long /*int*/ webkit_decide_policy (long /*int*/ web_view, long /*int*/ decision, int decision_type, long /*int*/ user_data) {
-    switch (decision_type) {
+	assert WEBKIT2 : "Webkit2 only code was ran by webkit1";
+	switch (decision_type) {
     case WebKitGTK.WEBKIT_POLICY_DECISION_TYPE_NAVIGATION_ACTION:
        long /*int*/ request = WebKitGTK. webkit_navigation_policy_decision_get_request(decision);
        if (request == 0){
@@ -2440,8 +2448,7 @@ long /*int*/ webkit_decide_policy (long /*int*/ web_view, long /*int*/ decision,
           if (jsEnabled != jsEnabledOnNextPage) {
              jsEnabled = jsEnabledOnNextPage;
              DisabledJSCount += !jsEnabled ? 1 : -1;
-             long /*int*/ settings = WebKitGTK.webkit_web_view_get_settings (webView);
-             OS.g_object_set (settings, WebKitGTK.enable_scripts, jsEnabled ? 1 : 0, 0);
+             webkit_settings_set(WebKitGTK.enable_javascript, jsEnabled ? 1 : 0);
           }
        }
        if(!newEvent.doit){
@@ -2812,6 +2819,21 @@ long /*int*/ webkit_window_object_cleared (long /*int*/ web_view, long /*int*/ f
 	return 0;
 }
 
+/** Webkit1 & Webkit2
+ * @return An integer value for the property is returned. For boolean settings, 0 indicates false, 1 indicates true */
+private int webkit_settings_get(byte [] property) {
+	long /*int*/ settings = WebKitGTK.webkit_web_view_get_settings (webView);
+	int[] result = new int[1];
+	OS.g_object_get (settings, property, result, 0);
+	return result[0];
+}
+
+/** Webkit1 & Webkit2 */
+private void webkit_settings_set(byte [] property, int value) {
+	long /*int*/ settings = WebKitGTK.webkit_web_view_get_settings (webView);
+	OS.g_object_set(settings, property, value, 0);
+}
+
 private void registerBrowserFunctions() {
 	Iterator<BrowserFunction> elements = functions.values().iterator ();
 	while (elements.hasNext ()) {
diff --git a/bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/internal/webkit/WebKitGTK.java b/bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/internal/webkit/WebKitGTK.java
index 73bb773ccf..471201ed86 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/internal/webkit/WebKitGTK.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/internal/webkit/WebKitGTK.java
@@ -87,10 +87,24 @@ public class WebKitGTK extends C {
 
 
 
-	/** Properties */
+	// Properties:
+	// Webkit1: https://webkitgtk.org/reference/webkitgtk/unstable/WebKitWebSettings.html#WebKitWebSettings.properties
+	// Webkit2: https://webkitgtk.org/reference/webkit2gtk/unstable/WebKitSettings.html#WebKitSettings.properties
+	//
+	// Developer Note:
+	// - Webkit1 documentation suggested to use g_object_(set|get) to modify properties.
+	// - Webkit2 documentation doesn't explicitly say if g_object_(set|get) is safe to use, but
+	//   I've confirmed with webkitgtk+ developers on IRC (freenode#webkitgtk+ <mcatanzaro>) that it is in fact still
+	//   safe to use g_object_(set|get) for updating properties.
+	//   Note:
+	//    - Some settings in Webkit2 have changed. It's not safe to use webkit1 settings on webkit2.
+	//    - On webkit2 you can also use the newly introduced functions for getting/setting settings as well as g_object_set().
 	public static final byte[] default_encoding = ascii ("default-encoding"); // $NON-NLS-1$
 	public static final byte[] default_charset = ascii ("default-charset"); // $NON-NLS-1$
-	public static final byte[] enable_scripts = ascii ("enable-scripts"); // $NON-NLS-1$
+
+	public static final byte[] enable_scripts = ascii ("enable-scripts"); // $NON-NLS-1$		// Webkit1 only.
+	public static final byte[] enable_javascript = ascii ("enable-javascript"); // $NON-NLS-1$	// Webkit2 only
+
 	public static final byte[] enable_plugins = ascii("enable-plugins"); // $NON-NLS-1$
 	public static final byte[] enable_webgl = ascii("enable-webgl"); // $NON-NLS-1$
 	public static final byte[] enable_universal_access_from_file_uris = ascii ("enable-universal-access-from-file-uris"); // $NON-NLS-1$
@@ -1403,7 +1417,7 @@ public static final double webkit_web_view_get_progress (long /*int*/ web_view)
 
 /** @method flags=dynamic */
 public static final native long /*int*/ _webkit_web_view_get_settings (long /*int*/ web_view);
-public static final long /*int*/ webkit_web_view_get_settings (long /*int*/ web_view) {
+public static final long /*int*/ webkit_web_view_get_settings (long /*int*/ web_view) {	// Webkit1 & Webkit2
 	lock.lock();
 	try {
 		return _webkit_web_view_get_settings (web_view);