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