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

Before recent improvements of the Edge implementation, capabilities of
to deal with asynchronous disposals of the browser were missing, leading
to unhandled errors inside the WebView initialization. They have been
worked around by not processing all kinds of asynchronously scheduled
events (like a disposal) but only those being processes by the OS event
queue. This was still necessary to process the OS callbacks for WebView
initialization and other operations.

In some cases, this may lead to an Edge browser instance blocking the UI
thread, as some asynchronously scheduled tasks need to be processed but
are not. In addition, enhancements of the Edge implementation made it
capable of processing asynchronous browser display disposals.

This change simplifies the event processing inside Edge to not just
process the next OS message but to just ordinarily spin the event loop.

Author: HeikoKlare

bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java

@@ -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];
@@ -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];
@@ -311,8 +311,8 @@ ICoreWebView2 initializeWebView(ICoreWebView2Controller controller) {
 		return webView;
 	}
 
-	private void abortInitialization() {
-		webViewFuture.cancel(true);
+	private void abortInitialization(String abortMessage) {
+		webViewFuture.completeExceptionally(new SWTError(abortMessage));
 	}
 
 	private void initializeWebView_2(ICoreWebView2 webView) {
@@ -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() {
@@ -587,8 +573,8 @@ private void createInstance(int previousAttempts) {
 }
 
 private IUnknown createControllerInitializationCallback(int previousAttempts) {
-	Runnable initializationRollback = () -> {
-		webViewProvider.abortInitialization();
+	Consumer<String> initializationRollback = abortMessage -> {
+		webViewProvider.abortInitialization(abortMessage);
 		if (environment2 != null) {
 			environment2.Release();
 			environment2 = null;
@@ -597,28 +583,27 @@ private IUnknown createControllerInitializationCallback(int previousAttempts) {
 	};
 	return newCallback((result, pv) -> {
 		if (browser.isDisposed()) {
-			initializationRollback.run();
+			initializationRollback.accept("Edge initialization was aborted");
 			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");
+			initializationRollback.accept("Edge instance with same data folder but different environment options already exists");
+			return COM.S_OK;
 		}
 		switch ((int) result) {
 		case COM.S_OK:
 			new IUnknown(pv).AddRef();
 			setupBrowser((int) result, pv);
 			break;
 		case COM.E_WRONG_THREAD:
-			initializationRollback.run();
+			initializationRollback.accept("Wrong thread access");
 			error(SWT.ERROR_THREAD_INVALID_ACCESS, (int) result);
 			break;
 		case COM.E_ABORT:
-			initializationRollback.run();
+			initializationRollback.accept("Edge initialization was aborted");
 			break;
 		default:
-			initializationRollback.run();
+			initializationRollback.accept("Edge initialization failed, retrying");
 			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);

tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java

@@ -302,11 +302,17 @@ private int reportOpenedDescriptors() {
 }
 
 private Browser createBrowser(Shell s, int flags) {
-	long maximumBrowserCreationMilliseconds = 90_000;
+	return createBrowser(s, flags, true);
+}
+
+private Browser createBrowser(Shell s, int flags, boolean expectSuccess) {
+	long maximumBrowserCreationMilliseconds = 5_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);
@@ -334,7 +340,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());
 }