Support Edge instances in different displays #1013

The current implementation of the Edge browser creates a
WebView2Environment on first creation of an WebView2 / Edge browser
instance. On every further creation of a WebView2 instance, this
environment is used. In case the WebView2 instance is created for a
different display, i.e., within a different thread, the instantiation
fails as the WebView2Environment has been created in a different thread.

To support the creation of WebView2 instances in different displays,
this change replaces the static WebView2Environment with one environment
for every display. The WebView2 instances are created for the
environment belonging to the display for which the current instantiation
is requested. An according regression test is added.

Contributes to
#1013

Author: HeikoKlare

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

@@ -44,9 +44,13 @@ class Edge extends WebBrowser {
 	static final String LANGUAGE_PROP = "org.eclipse.swt.browser.EdgeLanguage";
 	static final String VERSIONT_PROP = "org.eclipse.swt.browser.EdgeVersion";
 
-	static String DataDir;
-	static ICoreWebView2Environment Environment;
-	static ArrayList<Edge> Instances = new ArrayList<>();
+	private record WebViewEnvironment(ICoreWebView2Environment environment, ArrayList<Edge> instances) {
+		public WebViewEnvironment(ICoreWebView2Environment environment) {
+			this (environment, new ArrayList<>());
+		}
+	}
+
+	private static Map<Display, WebViewEnvironment> webViewEnvironments = new HashMap<>();
 
 	ICoreWebView2 webView;
 	ICoreWebView2_2 webView_2;
@@ -272,10 +276,14 @@ private static void processNextOSMessage() {
 }
 
 static ICoreWebView2CookieManager getCookieManager() {
-	if (Instances.isEmpty()) {
+	WebViewEnvironment environmentWrapper = webViewEnvironments.get(Display.getCurrent());
+	if (environmentWrapper == null) {
+		SWT.error(SWT.ERROR_INVALID_ARGUMENT, null, " [WebView2: environment not initialized for current display]");
+	}
+	if (environmentWrapper.instances().isEmpty()) {
 		SWT.error(SWT.ERROR_NOT_IMPLEMENTED, null, " [WebView2: cookie access requires a Browser instance]");
 	}
-	Edge instance = Instances.get(0);
+	Edge instance = environmentWrapper.instances.get(0);
 	if (instance.webView_2 == null) {
 		SWT.error(SWT.ERROR_NOT_IMPLEMENTED, null, " [WebView2 version 88+ is required to access cookies]");
 	}
@@ -296,9 +304,10 @@ void checkDeadlock() {
 	}
 }
 
-ICoreWebView2Environment createEnvironment() {
-	if (Environment != null) return Environment;
+WebViewEnvironment createEnvironment() {
 	Display display = Display.getCurrent();
+	WebViewEnvironment existingEnvironment = webViewEnvironments.get(display);
+	if (existingEnvironment != null) return existingEnvironment;
 
 	// Gather customization properties
 	String browserDir = System.getProperty(BROWSER_DIR_PROP);
@@ -336,33 +345,38 @@ ICoreWebView2Environment createEnvironment() {
 		SWT.error(SWT.ERROR_NOT_IMPLEMENTED, null, " [WebView2 runtime not found]");
 	}
 	if (hr != COM.S_OK) error(SWT.ERROR_NO_HANDLES, hr);
-	Environment = new ICoreWebView2Environment(ppv[0]);
-	DataDir = dataDir;
+	ICoreWebView2Environment environment = new ICoreWebView2Environment(ppv[0]);
+	WebViewEnvironment environmentWrapper = new WebViewEnvironment(environment);
 
 	// Save Edge version for reporting
 	long[] ppVersion = new long[1];
-	Environment.get_BrowserVersionString(ppVersion);
+	environment.get_BrowserVersionString(ppVersion);
 	String version = wstrToString(ppVersion[0], true);
 	System.setProperty(VERSIONT_PROP, version);
 
 	// Destroy the environment on app exit.
 	display.disposeExec(() -> {
-		Environment.Release();
-		Environment = null;
+		for (Edge instance : environmentWrapper.instances()) {
+			instance.browserDispose(null);
+		}
+		environment.Release();
+		webViewEnvironments.remove(display);
 	});
-	return Environment;
+
+	webViewEnvironments.put(display, environmentWrapper);
+	return environmentWrapper;
 }
 
 @Override
 public void create(Composite parent, int style) {
 	checkDeadlock();
-	ICoreWebView2Environment environment = createEnvironment();
+	WebViewEnvironment environmentWrapper = createEnvironment();
 
 	long[] ppv = new long[1];
-	int hr = environment.QueryInterface(COM.IID_ICoreWebView2Environment2, ppv);
+	int hr = environmentWrapper.environment().QueryInterface(COM.IID_ICoreWebView2Environment2, ppv);
 	if (hr == COM.S_OK) environment2 = new ICoreWebView2Environment2(ppv[0]);
 
-	hr = callAndWait(ppv, completion -> environment.CreateCoreWebView2Controller(browser.handle, completion));
+	hr = callAndWait(ppv, completion -> environmentWrapper.environment().CreateCoreWebView2Controller(browser.handle, completion));
 	switch (hr) {
 	case COM.S_OK:
 		break;
@@ -426,11 +440,11 @@ public void create(Composite parent, int style) {
 	browser.addListener(SWT.Resize, this::browserResize);
 	browser.addListener(SWT.Move, this::browserMove);
 
-	Instances.add(this);
+	environmentWrapper.instances().add(this);
 }
 
 void browserDispose(Event event) {
-	Instances.remove(this);
+	webViewEnvironments.get(Display.getCurrent()).instances().remove(this);
 
 	if (webView_2 != null) webView_2.Release();
 	if (environment2 != null) environment2.Release();

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

@@ -294,6 +294,70 @@ public void test_Constructor_multipleInstantiationsInDifferentShells() {
 	}
 }
 
+private class EdgeBrowserApplication extends Thread {
+	private volatile boolean shouldClose;
+	private volatile boolean isRunning;
+	private volatile boolean isDisposed;
+
+	@Override
+	public void run() {
+		Display threadDisplay = new Display();
+		Shell browserShell = new Shell(threadDisplay);
+		Browser browser = createBrowser(browserShell, SWT.EDGE);
+		browserShell.setVisible(true);
+		isDisposed = browser.isDisposed();
+		isRunning = true;
+
+		while (!shouldClose) {
+			synchronized (this) {
+				try {
+					wait();
+				} catch (InterruptedException e) {
+					shouldClose = true;
+				}
+			}
+		}
+
+		browser.dispose();
+		browserShell.dispose();
+		isDisposed = browser.isDisposed();
+		isRunning = false;
+	}
+
+	public void close() {
+		shouldClose = true;
+		synchronized (this) {
+			notifyAll();
+		}
+		while (isRunning) {
+			Thread.yield();
+		}
+	}
+}
+
+@Test
+public void test_Constructor_multipleInstantiationsInDifferentThreads() {
+	assumeTrue("test case is only relevant on Windows", SwtTestUtil.isWindows);
+
+	int numberOfApplication = 5;
+	List<EdgeBrowserApplication> browserApplications = new ArrayList<>();
+	for (int i = 0; i < numberOfApplication; i++) {
+		EdgeBrowserApplication application = new EdgeBrowserApplication();
+		browserApplications.add(application);
+		application.start();
+	}
+	for (EdgeBrowserApplication application : browserApplications) {
+		while (!application.isRunning) {
+			Thread.yield();
+		}
+		assertFalse(application.isDisposed);
+	}
+	for (EdgeBrowserApplication application : browserApplications) {
+		application.close();
+		assertTrue(application.isDisposed);
+	}
+}
+
 @Test
 public void test_evalute_Cookies () {
 	final AtomicBoolean loaded = new AtomicBoolean(false);