From 82d080b2514b2f56be80d67659623478d72716d3 Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Wed, 31 Jan 2024 11:34:15 +0100 Subject: [PATCH] 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 https://github.com/eclipse-platform/eclipse.platform.swt/issues/1013 --- .../win32/org/eclipse/swt/browser/Edge.java | 52 ++++++++++------ .../Test_org_eclipse_swt_browser_Browser.java | 61 +++++++++++++++++++ 2 files changed, 95 insertions(+), 18 deletions(-) diff --git a/bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java b/bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java index 477726227bf..deec1ba155e 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java +++ b/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 Instances = new ArrayList<>(); + private record WebViewEnvironment(ICoreWebView2Environment environment, ArrayList instances) { + public WebViewEnvironment(ICoreWebView2Environment environment) { + this (environment, new ArrayList<>()); + } + } + + private static Map webViewEnvironments = new HashMap<>(); ICoreWebView2 webView; ICoreWebView2_2 webView_2; @@ -54,6 +58,8 @@ class Edge extends WebBrowser { ICoreWebView2Settings settings; ICoreWebView2Environment2 environment2; + WebViewEnvironment containingEnvironment; + static boolean inCallback; boolean inNewWindow; HashMap navigations = new HashMap<>(); @@ -272,10 +278,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 +306,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 +347,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(); + containingEnvironment = createEnvironment(); long[] ppv = new long[1]; - int hr = environment.QueryInterface(COM.IID_ICoreWebView2Environment2, ppv); + int hr = containingEnvironment.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 -> containingEnvironment.environment().CreateCoreWebView2Controller(browser.handle, completion)); switch (hr) { case COM.S_OK: break; @@ -426,11 +442,11 @@ public void create(Composite parent, int style) { browser.addListener(SWT.Resize, this::browserResize); browser.addListener(SWT.Move, this::browserMove); - Instances.add(this); + containingEnvironment.instances().add(this); } void browserDispose(Event event) { - Instances.remove(this); + containingEnvironment.instances.remove(this); if (webView_2 != null) webView_2.Release(); if (environment2 != null) environment2.Release(); diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java index 6b501d2fdfc..349da277952 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java +++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java @@ -294,6 +294,67 @@ 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(); + threadDisplay.dispose(); + isDisposed = browser.isDisposed(); + isRunning = false; + } + + public void close() { + shouldClose = true; + synchronized (this) { + notifyAll(); + } + waitForPassCondition(() -> !isRunning); + } +} + +@Test +public void test_Constructor_multipleInstantiationsInDifferentThreads() { + assumeTrue("test case is only relevant on Windows", SwtTestUtil.isWindows); + + int numberOfApplication = 5; + List browserApplications = new ArrayList<>(); + for (int i = 0; i < numberOfApplication; i++) { + EdgeBrowserApplication application = new EdgeBrowserApplication(); + browserApplications.add(application); + application.start(); + } + for (EdgeBrowserApplication application : browserApplications) { + waitForPassCondition(() -> application.isRunning); + assertFalse(application.isDisposed); + } + for (EdgeBrowserApplication application : browserApplications) { + application.close(); + assertTrue(application.isDisposed); + } +} + @Test public void test_evalute_Cookies () { final AtomicBoolean loaded = new AtomicBoolean(false);