Edge:evaluate deadlock fix eclipse-platform#1466

This commit cotributes to replicating the behaviour of Edge:evaluate as
it is in WebKit - it must not wait for the execution of script to obtain
the result, in case evaluate is called inside a WebView callback. This
commit also makes sure that OpenWindowListeners are execute
synchronously or asynchronously depending on if the
handleNewWindowRequested is called from the evaluate script. Moreover,
this enables all the tests which were failing because of Edge:evaluate
limitations.

contributes to
eclipse-platform#1771 and
eclipse-platform#1919

Author: amartya4256

bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java

@@ -81,6 +81,7 @@ public WebViewEnvironment(ICoreWebView2Environment environment) {
 
 	static boolean inCallback;
 	boolean inNewWindow;
+	private boolean inEvaluate;
 	HashMap<Long, LocationEvent> navigations = new HashMap<>();
 	private boolean ignoreGotFocus;
 	private boolean ignoreFocusIn;
@@ -906,11 +907,19 @@ public Object evaluate(String script) throws SWTException {
 	// Feature in WebView2. ExecuteScript works regardless of IsScriptEnabled setting.
 	// Disallow programmatic execution manually.
 	if (!jsEnabled) return null;
-
+	if(inCallback) {
+		execute(script);
+		return null;
+	}
 	String script2 = "(function() {try { " + script + " } catch (e) { return '" + ERROR_ID + "' + e.message; } })();\0";
 	String[] pJson = new String[1];
-	int hr = callAndWait(pJson, completion -> webViewProvider.getWebView(true).ExecuteScript(script2.toCharArray(), completion));
-	if (hr != COM.S_OK) error(SWT.ERROR_FAILED_EVALUATE, hr);
+	inEvaluate = true;
+	try {
+		int hr = callAndWait(pJson, completion -> webViewProvider.getWebView(true).ExecuteScript(script2.toCharArray(), completion));
+		if (hr != COM.S_OK) error(SWT.ERROR_FAILED_EVALUATE, hr);
+	} finally {
+		inEvaluate = false;
+	}
 
 	Object data = JSON.parse(pJson[0]);
 	if (data instanceof String && ((String) data).startsWith(ERROR_ID)) {
@@ -1319,7 +1328,7 @@ int handleNewWindowRequested(long pView, long pArgs) {
 	args.GetDeferral(ppv);
 	ICoreWebView2Deferral deferral = new ICoreWebView2Deferral(ppv[0]);
 	inNewWindow = true;
-	browser.getDisplay().asyncExec(() -> {
+	Runnable openWindowHandler = () -> {
 		try {
 			if (browser.isDisposed()) return;
 			WindowEvent openEvent = new WindowEvent(browser);
@@ -1355,7 +1364,21 @@ int handleNewWindowRequested(long pView, long pArgs) {
 			args.Release();
 			inNewWindow = false;
 		}
-	});
+	};
+
+	// In case a new window is opened using evaluate with scripts like
+	// 'window.open()', there can be a deadlock if we execute the handler
+	// asynchronously. And in case a new browser is initialized in the same
+	// environment inside an OpenWindowListener of another browser, there can also
+	// be a deadlock. Hence, generally it should be run asynchronously. However, a
+	// combination of opening a new window from evaluate and starting a new instance
+	// of browser inside the OpenWindowListener should be avoided.
+	if (inEvaluate) {
+		openWindowHandler.run();
+	} else {
+		browser.getDisplay().asyncExec(openWindowHandler);
+	}
+
 	return COM.S_OK;
 }
 

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

@@ -745,7 +745,6 @@ public void changed(LocationEvent event) {
 
 @Test
 public void test_LocationListener_LocationListener_ordered_changing () {
-	assumeFalse("Currently broken for Edge", isEdge);
 	List<String> locations = Collections.synchronizedList(new ArrayList<>());
 	browser.addLocationListener(changingAdapter(event -> {
 		locations.add(event.location);
@@ -1578,8 +1577,6 @@ public void completed(ProgressEvent event) {
 */
 @Test
 public void test_LocationListener_evaluateInCallback() {
-	assumeFalse("behavior is not (yet) supported by Edge browser", isEdge);
-
 	AtomicBoolean changingFinished = new AtomicBoolean(false);
 	AtomicBoolean changedFinished = new AtomicBoolean(false);
 	browser.addLocationListener(new LocationListener() {
@@ -1628,8 +1625,6 @@ public void changed(LocationEvent event) {
 /** Verify that evaluation works inside an OpenWindowListener */
 @Test
 public void test_OpenWindowListener_evaluateInCallback() {
-	assumeFalse("behavior is not (yet) supported by Edge browser", isEdge);
-
 	AtomicBoolean eventFired = new AtomicBoolean(false);
 	browser.addOpenWindowListener(event -> {
 		browser.evaluate("SWTopenListener = true");
@@ -2189,7 +2184,6 @@ public void test_evaluate_array_mixedTypes () {
  */
 @Test
 public void test_BrowserFunction_callback () {
-	assumeFalse("Currently broken for Edge", isEdge);
 	AtomicBoolean javaCallbackExecuted = new AtomicBoolean(false);
 
 	class JavascriptCallback extends BrowserFunction { // Note: Local class defined inside method.