From caf599345f6037e66ded8449cb1f579847cda0a6 Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Fri, 31 Jul 2026 17:18:31 +0200 Subject: [PATCH] [Win32] Fix BrowserFunction missing on first page A BrowserFunction created while an Edge/WebView2 browser is still initializing (with a navigation queued via setUrl/setText) could be missing on the first loaded page: AddScriptToExecuteOnDocumentCreated was issued only after the queued navigation and raced with document creation, so the function was sometimes not yet registered when the page's document was created. With the change, we register the document-created scripts of all pending BrowserFunctions inside WebViewProvider.initializeWebView(), before completing the initialization future. Completing the future synchronously runs the queued navigation, so registering beforehand guarantees the functions are in place before the first document is created. This is the only point that is reliably "after init, before the first navigation", since a CompletableFuture navigation chain cannot be preempted retroactively. Registration is now issued asynchronously (fire-and-forget). What makes a function land on a page is issuing the registration before the navigation is issued to WebView2, not waiting for its completion. As a result the previous inCallback>0 deferral is no longer needed (an async call cannot deadlock inside a WebView2 callback) and the separate synchronous registration method is removed, leaving a single registration method and a single branch in createFunction. Since a function's script may be registered either by createFunction() or by initializeWebView() - note that createFunction() executes the function's script and thus may pump the event loop and complete the initialization itself - registration is made idempotent: an entry is put into functionScriptIds as soon as the registration is issued, with a null value until the asynchronous completion delivers the script ID. Thus a contained key denotes an already registered function, which makes the registration happen exactly once, independent of the order in which initialization and function creation interleave. deregisterFunction stays correct for a registration whose script ID has not arrived yet: the async completion callback detects the removed function via the functions map and removes the just-registered script itself. Fixes https://github.com/eclipse-platform/eclipse.platform.swt/issues/3370 Co-Authored-By: Claude Opus 4.8 --- .../win32/org/eclipse/swt/browser/Edge.java | 104 ++++++++++---- .../Test_org_eclipse_swt_browser_Browser.java | 127 ++++++++++++++++++ 2 files changed, 207 insertions(+), 24 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 226607eb2a6..3518bc5a662 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 @@ -85,7 +85,13 @@ public WebViewEnvironment(ICoreWebView2Environment environment) { boolean inNewWindow; private boolean inEvaluate; HashMap navigations = new HashMap<>(); - /** Maps BrowserFunction index to the script ID from AddScriptToExecuteOnDocumentCreated. */ + /** + * Maps BrowserFunction index to the script ID from AddScriptToExecuteOnDocumentCreated. An entry + * is added synchronously when the (asynchronous) registration is issued, with a {@code null} + * value until the registration completes and provides the script ID. Therefore a contained key + * denotes a function whose script is already registered, which ensures that a function's script + * is registered exactly once. + */ private final Map functionScriptIds = new HashMap<>(); private int ignoreGotFocus; private boolean ignoreFocusIn; @@ -386,6 +392,11 @@ ICoreWebView2 initializeWebView(ICoreWebView2Controller controller) { webViewWrapper.webView_11 = initializeWebView_11(webView); webViewWrapper.webView_12 = initializeWebView_12(webView); webViewWrapper.webView_13 = initializeWebView_13(webView); + // Register the scripts of all BrowserFunctions created during initialization + // before completing the future below, as that synchronously runs queued + // navigation tasks (e.g. from setUrl()/setText()) and thus may already create + // the first document + registerPendingFunctionScripts(webView); boolean success = webViewWrapperFuture.complete(webViewWrapper); // Release the webViews if the webViewWrapperFuture has already timed out and completed exceptionally if(!success && webViewWrapperFuture.isCompletedExceptionally()) { @@ -399,6 +410,10 @@ private void abortInitialization() { webViewWrapperFuture.cancel(true); } + boolean isInitialized() { + return webViewWrapperFuture.isDone(); + } + void releaseWebView() { getWebViewWrapper().releaseWebViews(); } @@ -1839,40 +1854,79 @@ public boolean setUrl(String url, String postData, String[] headers) { } /** - * Registers the function script persistently via AddScriptToExecuteOnDocumentCreated so it is - * injected on every future document creation before any page scripts run, avoiding the race - * condition between async function injection and navigation completion. - * If called while inside a WebView2 callback, the persistent registration is deferred via - * {@link Display#asyncExec(Runnable)} so it completes once the callback returns. + * Registers a BrowserFunction persistently via AddScriptToExecuteOnDocumentCreated so it is + * injected on every future document creation before any page scripts run. + *

+ * The registration is issued immediately, but without waiting for its (asynchronous) completion: + * what makes a function available on a page is issuing the registration before the + * navigation that creates the document is issued to WebView2 - not waiting for the registration to + * complete. Since nothing is blocked on the completion, this can also safely be issued from within + * a WebView2 callback without risking a deadlock. + *

+ * If the browser is not yet initialized when the function is created, the registration is issued by + * {@link WebViewProvider#initializeWebView(ICoreWebView2Controller)} (via + * {@link #registerPendingFunctionScripts(ICoreWebView2)}) before the first navigation, so functions + * created concurrently with initialization are available on the first loaded page. Note that + * {@code super.createFunction(function)} below may itself complete the initialization, since it + * executes the function's script and thus pumps the event loop until the browser is initialized. + * Independent of which of the two registers the script, {@link #registerFunctionScript(ICoreWebView2, int, String)} + * ensures it is registered exactly once. * See issue #20. */ @Override public void createFunction(BrowserFunction function) { super.createFunction(function); - int functionIndex = function.index; - String functionString = function.functionString; - if (inCallback > 0) { - // Cannot wait for a callback result while already inside a WebView2 callback; - // defer the persistent registration to after the callback completes. - browser.getDisplay().asyncExec(() -> { - if (browser.isDisposed() || !functions.containsKey(functionIndex)) return; - registerFunctionScript(functionIndex, functionString); - }); - return; + if (webViewProvider.isInitialized()) { + registerFunctionScript(webViewProvider.getWebView(false), function.index, function.functionString); } - registerFunctionScript(functionIndex, functionString); } -private void registerFunctionScript(int functionIndex, String functionString) { - String[] scriptId = new String[1]; - callAndWait(scriptId, completion -> - webViewProvider.getWebView(false).AddScriptToExecuteOnDocumentCreated( - stringToWstr(functionString), completion.getAddress())); - if (scriptId[0] != null) { - functionScriptIds.put(functionIndex, scriptId[0]); +private void registerPendingFunctionScripts(ICoreWebView2 webView) { + for (Map.Entry entry : functions.entrySet()) { + BrowserFunction function = entry.getValue(); + if (function.functionString != null) { + registerFunctionScript(webView, entry.getKey(), function.functionString); + } } } +/** + * Issues the registration of a function's document-created script on the given WebView without + * blocking for the asynchronous completion. Functions whose script is already registered are + * skipped, so it does not matter whether the registration is issued by + * {@link #createFunction(BrowserFunction)} or {@link #registerPendingFunctionScripts(ICoreWebView2)}. + *

+ * The resulting script ID is stored in {@link #functionScriptIds} once the completion callback + * fires; if the function was deregistered again in the meantime, the script is removed right away + * instead, so an immediately following deregistration does not leak the script. + */ +private void registerFunctionScript(ICoreWebView2 webView, int functionIndex, String functionString) { + if (functionScriptIds.containsKey(functionIndex)) { + // The script of this function has already been registered + return; + } + functionScriptIds.put(functionIndex, null); + IUnknown completion = newCallback((result, scriptIdPointer) -> { + if ((int) result == COM.S_OK) { + String scriptId = wstrToString(scriptIdPointer, false); + if (functions.containsKey(functionIndex)) { + functionScriptIds.put(functionIndex, scriptId); + } else if (!browser.isDisposed()) { + webView.RemoveScriptToExecuteOnDocumentCreated(stringToWstr(scriptId)); + } + } else { + functionScriptIds.remove(functionIndex); + } + return COM.S_OK; + }); + int hr = webView.AddScriptToExecuteOnDocumentCreated(stringToWstr(functionString), completion.getAddress()); + if (hr != OS.S_OK) { + functionScriptIds.remove(functionIndex); + System.err.println("Registering browser function failed with result " + hr + " for function: " + functionString); + } + completion.Release(); +} + @Override void deregisterFunction(BrowserFunction function) { super.deregisterFunction(function); @@ -1881,6 +1935,8 @@ void deregisterFunction(BrowserFunction function) { webViewProvider.getWebView(true).RemoveScriptToExecuteOnDocumentCreated( stringToWstr(scriptId)); } + // If scriptId == null, an asynchronous registration has not stored its ID yet; its completion + // callback detects the now-removed function (via the functions map) and removes the script itself. } } diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java index 651f6d0fd2d..ea8bcfd4383 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java +++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java @@ -3045,6 +3045,133 @@ public void test_BrowserFunction_availableOnLoad_concurrentInstances_issue20() { assertTrue(browser2FuncAvailable.get(), "BrowserFunction for second browser missing when page load completed"); } +/** + * Regression test: a BrowserFunction created from inside another BrowserFunction's + * callback must be registered and available on a page that is navigated to from within that same + * callback. + *

+ * On the Edge/WebView2 backend this exercises function creation while a WebView2 callback is on the + * stack. Registration must be issued (before the navigation queued in the same callback) without + * blocking, since blocking inside a callback would deadlock. + */ +@Test +public void test_BrowserFunction_createFunctionInsideCallback() { + assumeTrue(isEdge, "BrowserFunction availability before the page's inline scripts is specific to the Edge/WebView2 implementation"); + AtomicBoolean innerCalled = new AtomicBoolean(false); + + // 'inner' is only created when 'outer' is invoked from JavaScript, i.e. inside a callback. + class Inner extends BrowserFunction { + Inner() { + super(browser, "inner"); + } + @Override + public Object function(Object[] arguments) { + innerCalled.set(true); + return null; + } + } + class Outer extends BrowserFunction { + Outer() { + super(browser, "outer"); + } + @Override + public Object function(Object[] arguments) { + new Inner(); // create a new BrowserFunction from inside a callback + // Navigate to a page whose inline script calls the just-created function. + browser.setText(""); + return null; + } + } + new Outer(); + + // Trigger outer() once, after the first page has loaded. + AtomicBoolean outerTriggered = new AtomicBoolean(false); + browser.addProgressListener(completedAdapter(e -> { + if (outerTriggered.compareAndSet(false, true)) { + browser.execute("outer();"); + } + })); + browser.setText("first page"); + + shell.open(); + assertTrue(waitForPassCondition(innerCalled::get), + "BrowserFunction created inside a callback was not available on the page navigated to from that callback"); +} + +/** + * Regression test for issue #20: a BrowserFunction created while the browser is still initializing + * must be available before the first loaded page's own inline scripts run - not merely + * after the page finished loading. This combines concurrent initialization (the browser is not + * awaited) with a page whose inline script immediately calls the function. + */ +@Test +public void test_BrowserFunction_availableBeforePageScripts_concurrentInit_issue20() { + assumeTrue(isEdge, "BrowserFunction availability before the page's inline scripts is specific to the Edge/WebView2 implementation"); + AtomicBoolean functionCalled = new AtomicBoolean(false); + + // Use new Browser() directly (not the createBrowser() helper that waits for initialization) so + // the browser is still initializing while we navigate and register the function. + Browser b = new Browser(shell, SWT.NONE); + createdBroswers.add(b); + // Mirror the bug's order: request the navigation first, then create the function - both before + // initialization completes. + b.setText(""); + new BrowserFunction(b, "options") { + @Override + public Object function(Object[] arguments) { + functionCalled.set(true); + return null; + } + }; + + shell.open(); + assertTrue(waitForPassCondition(functionCalled::get), + "BrowserFunction 'options' was not available before the first page's inline script ran during concurrent initialization"); +} + +/** + * Regression test for issue #20: when multiple BrowserFunctions are created while the browser is + * still initializing, all of them must be available on the first loaded page. + */ +@Test +public void test_BrowserFunction_multipleFunctionsDuringConcurrentInit_issue20() { + assumeFalse(SwtTestUtil.isCocoa, "BrowserFunction availability during concurrent initialization is not reliable on Cocoa"); + AtomicReference result = new AtomicReference<>(); + AtomicReference failure = new AtomicReference<>(); + + Browser b = new Browser(shell, SWT.NONE); + createdBroswers.add(b); + b.setUrl("about:blank"); + new BrowserFunction(b, "f1") { + @Override + public Object function(Object[] arguments) { + return 1; + } + }; + new BrowserFunction(b, "f2") { + @Override + public Object function(Object[] arguments) { + return 2; + } + }; + b.addProgressListener(completedAdapter(e -> { + try { + result.set(b.evaluate("return f1() + f2();")); + } catch (SWTException ex) { + failure.set(ex); + } + })); + + shell.open(); + waitForPassCondition(() -> result.get() != null || failure.get() != null); + if (failure.get() != null) { + throw failure.get(); + } + assertNotNull(result.get(), "Neither BrowserFunction was available on the first loaded page"); + assertEquals(3.0, ((Number) result.get()).doubleValue(), + "Both BrowserFunctions created during concurrent initialization must be available on the first page"); +} + /** * Regression test: a disposed BrowserFunction must no longer be available (re-injected) after a * subsequent navigation. This verifies that deregistration removes the persistent document-created