From d7febc4997cad0afc6b683cefe06fd5114168f36 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; to avoid endless waiting, the number of retries is currently limited to 5 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. Contributes to https://github.com/eclipse-platform/eclipse.platform.swt/issues/1664 --- .../win32/org/eclipse/swt/browser/Edge.java | 73 ++++++++++++++----- .../eclipse/swt/internal/ole/win32/COM.java | 1 + .../org/eclipse/swt/internal/win32/OS.java | 1 + 3 files changed, 55 insertions(+), 20 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 2e4d7055b71..d5fbdc78961 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 @@ -57,6 +57,8 @@ class Edge extends WebBrowser { private static final URI URI_FOR_CUSTOM_TEXT_PAGE = setupAndGetLocationForCustomTextPage(); private static final String ABOUT_BLANK = "about:blank"; + private static final int MAXIMUM_CREATION_RETRIES = 5; + private record WebViewEnvironment(ICoreWebView2Environment environment, ArrayList instances) { public WebViewEnvironment(ICoreWebView2Environment environment) { this (environment, new ArrayList<>()); @@ -309,6 +311,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,39 +573,66 @@ private String getDataDir(Display display) { @Override public void create(Composite parent, int style) { + createInstance(0); +} + +private void createInstance(int previousAttempts) { containingEnvironment = createEnvironment(); containingEnvironment.instances().add(this); 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, createControllerInitializationCallback(previousAttempts)); +} + +private IUnknown createControllerInitializationCallback(int previousAttempts) { + Runnable initializationRollback = () -> { + webViewProvider.abortInitialization(); + if (environment2 != null) { + environment2.Release(); + environment2 = null; + } + containingEnvironment.instances().remove(this); + }; + return newCallback((result, pv) -> { + if (browser.isDisposed()) { + initializationRollback.run(); + return COM.S_OK; + } + if (result == OS.HRESULT_FROM_WIN32(OS.ERROR_INVALID_STATE)) { + initializationRollback.run(); + 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: + initializationRollback.run(); + error(SWT.ERROR_THREAD_INVALID_ACCESS, (int) result); + break; + case COM.E_ABORT: + initializationRollback.run(); + break; + default: + initializationRollback.run(); + if (previousAttempts < MAXIMUM_CREATION_RETRIES) { + System.err.println(String.format("Edge initialization failed, retrying (attempt %d / %d)", previousAttempts + 1, MAXIMUM_CREATION_RETRIES)); + createInstance(previousAttempts + 1); + } else { + SWT.error(SWT.ERROR_UNSPECIFIED, null, + String.format(" Aborting Edge initialiation after %d retries", MAXIMUM_CREATION_RETRIES)); + } + 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: - containingEnvironment.instances().remove(this); - error(SWT.ERROR_THREAD_INVALID_ACCESS, hr); - break; - default: - System.err.println("WebView instantiation failed with result: " + hr); - containingEnvironment.instances().remove(this); - 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 d2cbc3c7a5c..a4497a739db 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 d2482709a38..9cd86e1f607 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 @@ -457,6 +457,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;