Handle CreateCoreWebView2EnvironmentWithOptions ERROR_NOT_SUPPORTED - #3500
Handle CreateCoreWebView2EnvironmentWithOptions ERROR_NOT_SUPPORTED#3500r-mennig wants to merge 1 commit into
Conversation
| if (hr == OS.HRESULT_FROM_WIN32(OS.ERROR_NOT_SUPPORTED)) { | ||
| SWT.error(SWT.ERROR_INVALID_ARGUMENT, null, String.format(" [Invalid WebView2 directory: %s. Please ensure that '%s' points to a WebView2 application directory (which usually ends with \\EdgeWebView\\Application\\<Version>)]", browserDir, BROWSER_DIR_PROP)); | ||
| } |
There was a problem hiding this comment.
I've found no reasonable way to write a unit test for this error code.
The documentation states that this error should be thrown if the given browserExecutableFolder contains the string \Edge\Application
I could not verify this in testing, it seems like the folder path doesn't actually matter here.
Instead, it seems like some other criteria is used to tell the Edge binary folder apart from the WebView2 folder (likely the different .exe files msedge.exe / msedgewebview2.exe but I could not find any further information on this).
The only way I found to reproduce this error is by setting the system property to an actual Edge binary installation (which would add way too much overhead for such a small change)
| 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_NOT_SUPPORTED = 0x32; |
There was a problem hiding this comment.
1aad45d to
7d8dcc5
Compare
| return phr[0]; | ||
| } | ||
|
|
||
| int callAndWait(String[] pstr, ToIntFunction<IUnknown> callable) { |
There was a problem hiding this comment.
Although this method was not involved in causing this bug, it still has the same underlying problem as the other callAndWait method (overwriting the return value if the callback exits before the event loop spinning happens). That's why I also changed the code here
There was a problem hiding this comment.
I can reproduce the error in #3499 by passing this VM argument in my Launch Configuration (I launched the application org.eclipse.ui.ide.workbench) and opening the internal web browser:
-Dorg.eclipse.swt.browser.EdgeDir="C:\\Program Files (x86)\\Microsoft\\Edge\\Application\\151.0.4129.72"And double-checked the correct usage by passing this instead (no freeze occurred):
-Dorg.eclipse.swt.browser.EdgeDir="C:\\Program Files (x86)\\Microsoft\\EdgeWebView\\Application\\151.0.4129.72"With this PR there is no freeze with either parameter ✔️
I only have some minor comments regarding unnecessary formatting and about wording in the documentation.
Once those are done, I approve 👍
Are we delaying this until M1 or do we push it for M3? It looks pretty harmless to me and getting rid of UI freezes is always a plus so I'd merge it right away.
Thanks for the review! I would propose delaying this until M1. After all, the problem only occurs if a system property is misconfigured (although this is hard to tell for a user with the current implementation) |
When passing a Edge binary directory to 'org.eclipse.swt.browser.EdgeDir', CreateCoreWebView2EnvironmentWithOptions returns HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED) instead of HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND) which is thrown for other invalid directories. In this case, the passed completion callback exits immediately before the processOSMessagesUntil(..) loop starts spinning. This previously caused the return value being overwritten in callAndWait(long[], ToIntFunction<IUnknown>), leading to a permanent UI freeze. Fixes eclipse-platform#3499
7d8dcc5 to
b5a92d6
Compare
2 participants
When passing a Edge binary directory to
'org.eclipse.swt.browser.EdgeDir',
CreateCoreWebView2EnvironmentWithOptions
returns HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED) instead of HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND) which is thrown for other invalid directories.
In this case, the passed completion callback exits immediately before the processOSMessagesUntil(..) loop starts spinning. This previously caused the return value being overwritten in callAndWait(long[], ToIntFunction), leading to a permanent UI freeze.
Fixes #3499