Skip to content

Commit bd7cd75

Browse files
committed
Edge callback listeners execute asyncly eclipse-platform#1466
This commit cotributes to executing the registered listeners of Edge browser on Browser callbacks in a asynchronous fashion making sure no WebView related task is executed while a WebView callback is already executing making sure there's no deadlock. contributes to eclipse-platform#1771 and eclipse-platform#1919
1 parent 0fe4a76 commit bd7cd75

File tree

2 files changed

+28
-10
lines changed

2 files changed

+28
-10
lines changed

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

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ public WebViewEnvironment(ICoreWebView2Environment environment) {
8181

8282
static boolean inCallback;
8383
boolean inNewWindow;
84+
private boolean inEvaluate = false;
8485
HashMap<Long, LocationEvent> navigations = new HashMap<>();
8586
private boolean ignoreGotFocus;
8687
private boolean ignoreFocusIn;
@@ -906,10 +907,19 @@ public Object evaluate(String script) throws SWTException {
906907
// Feature in WebView2. ExecuteScript works regardless of IsScriptEnabled setting.
907908
// Disallow programmatic execution manually.
908909
if (!jsEnabled) return null;
909-
910+
if(inCallback) {
911+
execute(script);
912+
return null;
913+
}
914+
inEvaluate = true;
910915
String script2 = "(function() {try { " + script + " } catch (e) { return '" + ERROR_ID + "' + e.message; } })();\0";
911916
String[] pJson = new String[1];
912-
int hr = callAndWait(pJson, completion -> webViewProvider.getWebView(true).ExecuteScript(script2.toCharArray(), completion));
917+
int hr;
918+
try {
919+
hr = callAndWait(pJson, completion -> webViewProvider.getWebView(true).ExecuteScript(script2.toCharArray(), completion));
920+
} finally {
921+
inEvaluate = false;
922+
}
913923
if (hr != COM.S_OK) error(SWT.ERROR_FAILED_EVALUATE, hr);
914924

915925
Object data = JSON.parse(pJson[0]);
@@ -1319,7 +1329,7 @@ int handleNewWindowRequested(long pView, long pArgs) {
13191329
args.GetDeferral(ppv);
13201330
ICoreWebView2Deferral deferral = new ICoreWebView2Deferral(ppv[0]);
13211331
inNewWindow = true;
1322-
browser.getDisplay().asyncExec(() -> {
1332+
Runnable openWindowHandler = () -> {
13231333
try {
13241334
if (browser.isDisposed()) return;
13251335
WindowEvent openEvent = new WindowEvent(browser);
@@ -1355,7 +1365,21 @@ int handleNewWindowRequested(long pView, long pArgs) {
13551365
args.Release();
13561366
inNewWindow = false;
13571367
}
1358-
});
1368+
};
1369+
1370+
// In case a new window is opened using evaluate with scripts like
1371+
// 'window.open()', there can be a deadlock if we execute the handler
1372+
// asynchronously. And in case a new browser is initialized in the same
1373+
// environment inside an OpenWindowListener of another browser, there can also
1374+
// be a deadlock. Hence, generally it should be run asynchronously. However, a
1375+
// combination of opening a new window from evaluate and starting a new instance
1376+
// of browser inside the OpenWindowListener should be avoided.
1377+
if (inEvaluate) {
1378+
openWindowHandler.run();
1379+
} else {
1380+
browser.getDisplay().asyncExec(openWindowHandler);
1381+
}
1382+
13591383
return COM.S_OK;
13601384
}
13611385

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -745,7 +745,6 @@ public void changed(LocationEvent event) {
745745

746746
@Test
747747
public void test_LocationListener_LocationListener_ordered_changing () {
748-
assumeFalse("Currently broken for Edge", isEdge);
749748
List<String> locations = Collections.synchronizedList(new ArrayList<>());
750749
browser.addLocationListener(changingAdapter(event -> {
751750
locations.add(event.location);
@@ -1578,8 +1577,6 @@ public void completed(ProgressEvent event) {
15781577
*/
15791578
@Test
15801579
public void test_LocationListener_evaluateInCallback() {
1581-
assumeFalse("behavior is not (yet) supported by Edge browser", isEdge);
1582-
15831580
AtomicBoolean changingFinished = new AtomicBoolean(false);
15841581
AtomicBoolean changedFinished = new AtomicBoolean(false);
15851582
browser.addLocationListener(new LocationListener() {
@@ -1628,8 +1625,6 @@ public void changed(LocationEvent event) {
16281625
/** Verify that evaluation works inside an OpenWindowListener */
16291626
@Test
16301627
public void test_OpenWindowListener_evaluateInCallback() {
1631-
assumeFalse("behavior is not (yet) supported by Edge browser", isEdge);
1632-
16331628
AtomicBoolean eventFired = new AtomicBoolean(false);
16341629
browser.addOpenWindowListener(event -> {
16351630
browser.evaluate("SWTopenListener = true");
@@ -2189,7 +2184,6 @@ public void test_evaluate_array_mixedTypes () {
21892184
*/
21902185
@Test
21912186
public void test_BrowserFunction_callback () {
2192-
assumeFalse("Currently broken for Edge", isEdge);
21932187
AtomicBoolean javaCallbackExecuted = new AtomicBoolean(false);
21942188

21952189
class JavascriptCallback extends BrowserFunction { // Note: Local class defined inside method.

0 commit comments

Comments
 (0)