From 86c452d48622356efffcb04c4d860733947774d0 Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Wed, 25 Sep 2024 14:06:24 +0200 Subject: [PATCH] Edge: properly handle initialization failures and abortion There is currently only few handling for failures during initialization of an Edge browser / WebView instance. The Microsoft documentation provides further information on this, in particular: - ERROR_INVALID_STATE indicates that multiple Edge instances with the same data folder but different environment options exist - E_ABORT indicates an active abortion of the initialization process - On any other non-OK return value than the above ones, the app should retry initialization of the instance This change adds appropriate handling for these scenarios, consisting of uniform rollback logic when the initialization fails or is aborted and retry logic to make repeated creation attempts. --- .../win32/org/eclipse/swt/browser/Edge.java | 58 +++++++++++++------ .../eclipse/swt/internal/ole/win32/COM.java | 1 + .../org/eclipse/swt/internal/win32/OS.java | 1 + .../Test_org_eclipse_swt_browser_Browser.java | 2 +- 4 files changed, 44 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 709da2cafb..93bcb6ca11 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 @@ -309,6 +309,10 @@ ICoreWebView2 initializeWebView(ICoreWebView2Controller controller) { return webView; } + private void abortInitialization() { + webViewFuture.cancel(true); + } + private void initializeWebView_2(ICoreWebView2 webView) { long[] ppv = new long[1]; int hr = webView.QueryInterface(COM.IID_ICoreWebView2_2, ppv); @@ -567,35 +571,55 @@ private String getDataDir(Display display) { @Override public void create(Composite parent, int style) { + createInstance(); +} + +private void createInstance() { containingEnvironment = createEnvironment(); long[] ppv = new long[1]; int hr = containingEnvironment.environment().QueryInterface(COM.IID_ICoreWebView2Environment2, ppv); if (hr == COM.S_OK) environment2 = new ICoreWebView2Environment2(ppv[0]); // The webview calls are queued to be executed when it is done executing the current task. - IUnknown setupBrowserCallback = newCallback((result, pv) -> { - if ((int)result == COM.S_OK) { + containingEnvironment.environment().CreateCoreWebView2Controller(browser.handle, controllerInitializedCallback()); +} + +private IUnknown controllerInitializedCallback() { + return newCallback((result, pv) -> { + Runnable rollbackInitialization = () -> { + webViewProvider.abortInitialization(); + if (environment2 != null) { + environment2.Release(); + } + }; + if (browser.isDisposed()) { + rollbackInitialization.run(); + } + if (result == OS.HRESULT_FROM_WIN32(OS.ERROR_INVALID_STATE)) { + SWT.error(SWT.ERROR_INVALID_ARGUMENT, null, + " Edge instance with same data folder but different environment options already exists"); + } + switch ((int) result) { + case COM.S_OK: new IUnknown(pv).AddRef(); + setupBrowser((int) result, pv); + break; + case COM.E_WRONG_THREAD: + error(SWT.ERROR_THREAD_INVALID_ACCESS, (int) result); + break; + case COM.E_ABORT: + rollbackInitialization.run(); + break; + default: + System.err.println("Edge initialization failed, retrying"); + rollbackInitialization.run(); + createInstance(); + break; } - setupBrowser((int)result, pv); return COM.S_OK; }); - containingEnvironment.environment().CreateCoreWebView2Controller(browser.handle, setupBrowserCallback); } void setupBrowser(int hr, long pv) { - if(browser.isDisposed()) { - browserDispose(new Event()); - return; - } - switch (hr) { - case COM.S_OK: - break; - case COM.E_WRONG_THREAD: - error(SWT.ERROR_THREAD_INVALID_ACCESS, hr); - break; - default: - error(SWT.ERROR_NO_HANDLES, hr); - } long[] ppv = new long[] {pv}; controller = new ICoreWebView2Controller(ppv[0]); final ICoreWebView2 webView = webViewProvider.initializeWebView(controller); diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/ole/win32/COM.java b/bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/ole/win32/COM.java index d2cbc3c7a5..a4497a739d 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/ole/win32/COM.java +++ b/bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/ole/win32/COM.java @@ -183,6 +183,7 @@ public class COM extends OS { public static final int DV_E_STGMEDIUM = -2147221402; public static final int DV_E_TYMED = -2147221399; public static final int DVASPECT_CONTENT = 1; + public static final int E_ABORT = 0x80004004; public static final int E_ACCESSDENIED = 0x80070005; public static final int E_FAIL = -2147467259; public static final int E_INVALIDARG = -2147024809; diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/win32/OS.java b/bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/win32/OS.java index 6d085694b3..d503fc21a1 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/win32/OS.java +++ b/bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/win32/OS.java @@ -456,6 +456,7 @@ public class OS extends C { public static final int EN_CHANGE = 0x300; public static final int EP_EDITTEXT = 1; public static final int ERROR_FILE_NOT_FOUND = 0x2; + public static final int ERROR_INVALID_STATE = 0x139F; public static final int ERROR_NO_MORE_ITEMS = 0x103; public static final int ERROR_CANCELED = 0x4C7; public static final int ESB_DISABLE_BOTH = 0x3; 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 9097284461..50bdecfa80 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 @@ -145,7 +145,7 @@ public static Collection browserFlagsToTest() { if (SwtTestUtil.isWindows) { // NOTE: This is currently disabled due to test issues in the CI // Execute Edge tests first, because IE starts some OS timer that conflicts with Edge event handling - // browserFlags.add(0, new Object[] {SWT.EDGE}); + browserFlags.add(0, new Object[] {SWT.EDGE}); } browserFlags.add(new Object[] {SWT.NONE}); return browserFlags;