Skip to content

Edge: properly handle initialization failures and abortion #1578

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 7, 2025
Merged

Edge: properly handle initialization failures and abortion #1578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 53 additions & 20 deletions bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Edge> instances) {
public WebViewEnvironment(ICoreWebView2Environment environment) {
this (environment, new ArrayList<>());
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/ole/win32/COM.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/win32/OS.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor

@amartya4256 amartya4256 Jan 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't 0x8007139F also a COMException and hence should be put in COM.java?
Refer these: MicrosoftEdge/WebView2Feedback#2495, MicrosoftEdge/WebView2Feedback#3008

Copy link
Contributor Author

@HeikoKlare HeikoKlare Jan 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ERROR_INVALID_STATE == 0x139f is an ordinary Windows error code (see https://learn.microsoft.com/en-us/windows/win32/debug/system-error-codes--4000-5999-), just like the other ordinary Windows error codes we have in OS (like ERROR_NO_MORE_ITEMS). So I would say it is correctly placed in OS.

Copy link
Contributor

@amartya4256 amartya4256 Jan 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh alright then its all good

public static final int ERROR_NO_MORE_ITEMS = 0x103;
public static final int ERROR_CANCELED = 0x4C7;
public static final int ESB_DISABLE_BOTH = 0x3;
Expand Down
Loading