Skip to content

[Win32] Spin event loop in Edge instead of only processing OS messages #1773

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

Closed
wants to merge 3 commits into from
Closed

[Win32] Spin event loop in Edge instead of only processing OS messages #1773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
632e54c
[Win32] Correct WebView initialization retry logic in Edge
HeikoKlare Jan 27, 2025
2772617
[Win32] Spin event loop in Edge instead of processing OS messages #1771
HeikoKlare Jan 27, 2025
7a44e53
Add browser regression test for calling evaluate requiring event loop
sratz Jan 28, 2025
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
37 changes: 13 additions & 24 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 @@ -261,7 +261,7 @@ static int callAndWait(long[] ppv, ToIntFunction<IUnknown> callable) {
// "completion" callback may be called asynchronously,
// so keep processing next OS message that may call it
while (phr[0] == COM.S_OK && ppv[0] == 0) {
processNextOSMessage();
spinEventLoop();
}
completion.Release();
return phr[0];
Expand All @@ -281,7 +281,7 @@ static int callAndWait(String[] pstr, ToIntFunction<IUnknown> callable) {
// "completion" callback may be called asynchronously,
// so keep processing next OS message that may call it
while (phr[0] == COM.S_OK && pstr[0] == null) {
processNextOSMessage();
spinEventLoop();
}
completion.Release();
return phr[0];
Expand Down Expand Up @@ -444,31 +444,17 @@ void scheduleWebViewTask(Runnable action) {

private <T> void waitForFutureToFinish(CompletableFuture<T> future) {
while(!future.isDone()) {
processNextOSMessage();
spinEventLoop();
}
}

}

/**
* Processes a single OS message using {@link Display#readAndDispatch()}. This
* is required for processing the OS events during browser initialization, since
* Edge browser initialization happens asynchronously.
* <p>
* {@link Display#readAndDispatch()} also processes events scheduled for
* asynchronous execution via {@link Display#asyncExec(Runnable)}. This may
* include events such as the disposal of the browser's parent composite, which
* leads to a failure in browser initialization if processed in between the OS
* events for initialization. Thus, this method does not implement an ordinary
* readAndDispatch loop, but waits for an OS event to be processed.
*/
private static void processNextOSMessage() {
private static void spinEventLoop() {
Display display = Display.getCurrent();
MSG msg = new MSG();
while (!OS.PeekMessage (msg, 0, 0, 0, OS.PM_NOREMOVE)) {
if (!display.readAndDispatch()) {
display.sleep();
}
display.readAndDispatch();
}

static ICoreWebView2CookieManager getCookieManager() {
Expand Down Expand Up @@ -588,20 +574,23 @@ private void createInstance(int previousAttempts) {

private IUnknown createControllerInitializationCallback(int previousAttempts) {
Runnable initializationRollback = () -> {
webViewProvider.abortInitialization();
if (environment2 != null) {
environment2.Release();
environment2 = null;
}
containingEnvironment.instances().remove(this);
};
Runnable initializationAbortion = () -> {
webViewProvider.abortInitialization();
initializationRollback.run();
};
return newCallback((result, pv) -> {
if (browser.isDisposed()) {
initializationRollback.run();
initializationAbortion.run();
return COM.S_OK;
}
if (result == OS.HRESULT_FROM_WIN32(OS.ERROR_INVALID_STATE)) {
initializationRollback.run();
initializationAbortion.run();
SWT.error(SWT.ERROR_INVALID_ARGUMENT, null,
" Edge instance with same data folder but different environment options already exists");
}
Expand All @@ -611,11 +600,11 @@ private IUnknown createControllerInitializationCallback(int previousAttempts) {
setupBrowser((int) result, pv);
break;
case COM.E_WRONG_THREAD:
initializationRollback.run();
initializationAbortion.run();
error(SWT.ERROR_THREAD_INVALID_ACCESS, (int) result);
break;
case COM.E_ABORT:
initializationRollback.run();
initializationAbortion.run();
break;
default:
initializationRollback.run();
Expand Down
42 changes: 29 additions & 13 deletions ...t.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swt.widgets.Text;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.FixMethodOrder;
import org.junit.Ignore;
import org.junit.Rule;
Expand Down Expand Up @@ -158,16 +157,6 @@ public Test_org_eclipse_swt_browser_Browser(int swtBrowserSettings) {
this.swtBrowserSettings = swtBrowserSettings;
}

@BeforeClass
public static void setupEdgeEnvironment() {
// initialize Edge environment before any test runs to isolate environment setup
if (SwtTestUtil.isWindows) {
Shell shell = new Shell();
new Browser(shell, SWT.EDGE);
shell.dispose();
}
}

@Override
@Before
public void setUp() {
Expand Down Expand Up @@ -302,11 +291,17 @@ private int reportOpenedDescriptors() {
}

private Browser createBrowser(Shell s, int flags) {
return createBrowser(s, flags, true);
}

private Browser createBrowser(Shell s, int flags, boolean expectSuccess) {
long maximumBrowserCreationMilliseconds = 90_000;
long createStartTime = System.currentTimeMillis();
Browser b = new Browser(s, flags);
// Wait for asynchronous initialization via getting URL
b.getUrl();
if (expectSuccess) {
b.getUrl();
}
createdBroswers.add(b);
long createDuration = System.currentTimeMillis() - createStartTime;
assertTrue("creating browser took too long: " + createDuration + "ms", createDuration < maximumBrowserCreationMilliseconds);
Expand Down Expand Up @@ -334,7 +329,7 @@ public void test_Constructor_asyncParentDisposal() {
Display.getCurrent().asyncExec(() -> {
shell.dispose();
});
Browser browser = createBrowser(shell, swtBrowserSettings);
Browser browser = createBrowser(shell, swtBrowserSettings, false);
assertFalse(browser.isDisposed());
}

Expand Down Expand Up @@ -434,6 +429,27 @@ public void test_evalute_Cookies () {
assertFalse(res.isEmpty());
}

@Test
public void test_evaluate_callingIntoSwt() throws Exception {
AtomicBoolean initialLoad = new AtomicBoolean();
AtomicBoolean openWindowListenerCalled = new AtomicBoolean();
browser.addProgressListener(ProgressListener.completedAdapter(e -> initialLoad.set(true)));
browser.addOpenWindowListener(event -> {
event.required = true; // block default
openWindowListenerCalled.set(true);
});
browser.setText("""
<button id="button" onClick="window.open('https://eclipse.org');">open eclipse.org</button>
""");
waitForPassCondition(initialLoad::get);

browser.evaluate("""
document.getElementById("button").click();
""");

waitForPassCondition(openWindowListenerCalled::get);
}

@Test
public void test_ClearAllSessionCookies () {
// clearSessions will only work for Webkit2 when >= 2.16
Expand Down
Loading