Skip to content

Commit 762e863

Browse files
HeikoKlareclaude
andcommitted
[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. 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 #3370 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 9b8609b commit 762e863

2 files changed

Lines changed: 191 additions & 23 deletions

File tree

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

Lines changed: 67 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,13 @@ ICoreWebView2 initializeWebView(ICoreWebView2Controller controller) {
386386
webViewWrapper.webView_11 = initializeWebView_11(webView);
387387
webViewWrapper.webView_12 = initializeWebView_12(webView);
388388
webViewWrapper.webView_13 = initializeWebView_13(webView);
389+
// Register the document-created scripts of all BrowserFunctions that were created while the
390+
// browser was still initializing, *before* completing the initialization future. Completing
391+
// the future synchronously runs any queued navigation task (e.g. from a preceding
392+
// setUrl()/setText()), so registering here guarantees the functions are injected before the
393+
// first document is created, avoiding a race with document creation.
394+
// See https://github.com/eclipse-platform/eclipse.platform.swt/issues/20
395+
registerPendingFunctionScripts(webView);
389396
boolean success = webViewWrapperFuture.complete(webViewWrapper);
390397
// Release the webViews if the webViewWrapperFuture has already timed out and completed exceptionally
391398
if(!success && webViewWrapperFuture.isCompletedExceptionally()) {
@@ -399,6 +406,14 @@ private void abortInitialization() {
399406
webViewWrapperFuture.cancel(true);
400407
}
401408

409+
/**
410+
* @return whether the WebView has finished (asynchronous) initialization, i.e. whether the
411+
* initialization future is completed. This check does <em>not</em> pump the event loop.
412+
*/
413+
boolean isInitialized() {
414+
return webViewWrapperFuture.isDone();
415+
}
416+
402417
void releaseWebView() {
403418
getWebViewWrapper().releaseWebViews();
404419
}
@@ -1839,40 +1854,67 @@ public boolean setUrl(String url, String postData, String[] headers) {
18391854
}
18401855

18411856
/**
1842-
* Registers the function script persistently via AddScriptToExecuteOnDocumentCreated so it is
1843-
* injected on every future document creation before any page scripts run, avoiding the race
1844-
* condition between async function injection and navigation completion.
1845-
* If called while inside a WebView2 callback, the persistent registration is deferred via
1846-
* {@link Display#asyncExec(Runnable)} so it completes once the callback returns.
1857+
* Registers a BrowserFunction persistently via AddScriptToExecuteOnDocumentCreated so it is
1858+
* injected on every future document creation before any page scripts run.
1859+
* <p>
1860+
* Registration is issued asynchronously (fire-and-forget): what makes a function available on a
1861+
* page is <em>issuing</em> the registration before the navigation that creates the document is
1862+
* issued to WebView2 - not waiting for the registration to complete. Being asynchronous, this can
1863+
* also safely be issued from within a WebView2 callback without risking a deadlock.
1864+
* <p>
1865+
* If the browser is not yet initialized when the function is created, the registration is issued by
1866+
* {@link WebViewProvider#initializeWebView(ICoreWebView2Controller)} (via
1867+
* {@link #registerPendingFunctionScripts(ICoreWebView2)}) before the first navigation, so functions
1868+
* created concurrently with initialization are available on the first loaded page.
18471869
* See <a href="https://github.com/eclipse-platform/eclipse.platform.swt/issues/20">issue #20</a>.
18481870
*/
18491871
@Override
18501872
public void createFunction(BrowserFunction function) {
1873+
// Capture the initialization state *before* super.createFunction(): it triggers browser
1874+
// initialization if not yet done, during which initializeWebView() registers all pending
1875+
// functions (before the first navigation). Registering again below would create a duplicate.
1876+
boolean alreadyInitialized = webViewProvider.isInitialized();
18511877
super.createFunction(function);
1852-
int functionIndex = function.index;
1853-
String functionString = function.functionString;
1854-
if (inCallback > 0) {
1855-
// Cannot wait for a callback result while already inside a WebView2 callback;
1856-
// defer the persistent registration to after the callback completes.
1857-
browser.getDisplay().asyncExec(() -> {
1858-
if (browser.isDisposed() || !functions.containsKey(functionIndex)) return;
1859-
registerFunctionScript(functionIndex, functionString);
1860-
});
1861-
return;
1878+
if (alreadyInitialized) {
1879+
registerFunctionScript(webViewProvider.getWebView(false), function.index, function.functionString);
18621880
}
1863-
registerFunctionScript(functionIndex, functionString);
18641881
}
18651882

1866-
private void registerFunctionScript(int functionIndex, String functionString) {
1867-
String[] scriptId = new String[1];
1868-
callAndWait(scriptId, completion ->
1869-
webViewProvider.getWebView(false).AddScriptToExecuteOnDocumentCreated(
1870-
stringToWstr(functionString), completion.getAddress()));
1871-
if (scriptId[0] != null) {
1872-
functionScriptIds.put(functionIndex, scriptId[0]);
1883+
/**
1884+
* Registers the document-created scripts of all already-created BrowserFunctions on the given,
1885+
* already-available WebView. Called during initialization before the first navigation is issued.
1886+
*/
1887+
private void registerPendingFunctionScripts(ICoreWebView2 webView) {
1888+
for (Map.Entry<Integer, BrowserFunction> entry : functions.entrySet()) {
1889+
BrowserFunction function = entry.getValue();
1890+
if (function.functionString != null) {
1891+
registerFunctionScript(webView, entry.getKey(), function.functionString);
1892+
}
18731893
}
18741894
}
18751895

1896+
/**
1897+
* Issues the registration of a function's document-created script on the given WebView without
1898+
* blocking for the asynchronous completion. The resulting script ID is stored once the completion
1899+
* callback fires; if the function was deregistered again in the meantime, the script is removed
1900+
* right away instead, so an immediately following deregistration does not leak the script.
1901+
*/
1902+
private void registerFunctionScript(ICoreWebView2 webView, int functionIndex, String functionString) {
1903+
IUnknown completion = newCallback((result, scriptIdPointer) -> {
1904+
if ((int) result == COM.S_OK) {
1905+
String scriptId = wstrToString(scriptIdPointer, false);
1906+
if (functions.containsKey(functionIndex)) {
1907+
functionScriptIds.put(functionIndex, scriptId);
1908+
} else if (!browser.isDisposed()) {
1909+
webView.RemoveScriptToExecuteOnDocumentCreated(stringToWstr(scriptId));
1910+
}
1911+
}
1912+
return COM.S_OK;
1913+
});
1914+
webView.AddScriptToExecuteOnDocumentCreated(stringToWstr(functionString), completion.getAddress());
1915+
completion.Release();
1916+
}
1917+
18761918
@Override
18771919
void deregisterFunction(BrowserFunction function) {
18781920
super.deregisterFunction(function);
@@ -1881,6 +1923,8 @@ void deregisterFunction(BrowserFunction function) {
18811923
webViewProvider.getWebView(true).RemoveScriptToExecuteOnDocumentCreated(
18821924
stringToWstr(scriptId));
18831925
}
1926+
// If scriptId == null, an asynchronous registration has not stored its ID yet; its completion
1927+
// callback detects the now-removed function (via the functions map) and removes the script itself.
18841928
}
18851929

18861930
}

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

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3045,6 +3045,130 @@ public void test_BrowserFunction_availableOnLoad_concurrentInstances_issue20() {
30453045
assertTrue(browser2FuncAvailable.get(), "BrowserFunction for second browser missing when page load completed");
30463046
}
30473047

3048+
/**
3049+
* Regression test: a BrowserFunction created from <em>inside</em> another BrowserFunction's
3050+
* callback must be registered and available on a page that is navigated to from within that same
3051+
* callback.
3052+
* <p>
3053+
* On the Edge/WebView2 backend this exercises function creation while a WebView2 callback is on the
3054+
* stack. Registration must be issued (before the navigation queued in the same callback) without
3055+
* blocking, since blocking inside a callback would deadlock.
3056+
*/
3057+
@Test
3058+
public void test_BrowserFunction_createFunctionInsideCallback() {
3059+
AtomicBoolean innerCalled = new AtomicBoolean(false);
3060+
3061+
// 'inner' is only created when 'outer' is invoked from JavaScript, i.e. inside a callback.
3062+
class Inner extends BrowserFunction {
3063+
Inner() {
3064+
super(browser, "inner");
3065+
}
3066+
@Override
3067+
public Object function(Object[] arguments) {
3068+
innerCalled.set(true);
3069+
return null;
3070+
}
3071+
}
3072+
class Outer extends BrowserFunction {
3073+
Outer() {
3074+
super(browser, "outer");
3075+
}
3076+
@Override
3077+
public Object function(Object[] arguments) {
3078+
new Inner(); // create a new BrowserFunction from inside a callback
3079+
// Navigate to a page whose inline script calls the just-created function.
3080+
browser.setText("<html><body><script>inner();</script></body></html>");
3081+
return null;
3082+
}
3083+
}
3084+
new Outer();
3085+
3086+
// Trigger outer() once, after the first page has loaded.
3087+
AtomicBoolean outerTriggered = new AtomicBoolean(false);
3088+
browser.addProgressListener(completedAdapter(e -> {
3089+
if (outerTriggered.compareAndSet(false, true)) {
3090+
browser.execute("outer();");
3091+
}
3092+
}));
3093+
browser.setText("<html><body>first page</body></html>");
3094+
3095+
shell.open();
3096+
assertTrue(waitForPassCondition(innerCalled::get),
3097+
"BrowserFunction created inside a callback was not available on the page navigated to from that callback");
3098+
}
3099+
3100+
/**
3101+
* Regression test for issue #20: a BrowserFunction created while the browser is still initializing
3102+
* must be available <em>before</em> the first loaded page's own inline scripts run - not merely
3103+
* after the page finished loading. This combines concurrent initialization (the browser is not
3104+
* awaited) with a page whose inline script immediately calls the function.
3105+
*/
3106+
@Test
3107+
public void test_BrowserFunction_availableBeforePageScripts_concurrentInit_issue20() {
3108+
AtomicBoolean functionCalled = new AtomicBoolean(false);
3109+
3110+
// Use new Browser() directly (not the createBrowser() helper that waits for initialization) so
3111+
// the browser is still initializing while we navigate and register the function.
3112+
Browser b = new Browser(shell, SWT.NONE);
3113+
createdBroswers.add(b);
3114+
// Mirror the bug's order: request the navigation first, then create the function - both before
3115+
// initialization completes.
3116+
b.setText("<html><body><script>options();</script></body></html>");
3117+
new BrowserFunction(b, "options") {
3118+
@Override
3119+
public Object function(Object[] arguments) {
3120+
functionCalled.set(true);
3121+
return null;
3122+
}
3123+
};
3124+
3125+
shell.open();
3126+
assertTrue(waitForPassCondition(functionCalled::get),
3127+
"BrowserFunction 'options' was not available before the first page's inline script ran during concurrent initialization");
3128+
}
3129+
3130+
/**
3131+
* Regression test for issue #20: when multiple BrowserFunctions are created while the browser is
3132+
* still initializing, all of them must be available on the first loaded page.
3133+
*/
3134+
@Test
3135+
public void test_BrowserFunction_multipleFunctionsDuringConcurrentInit_issue20() {
3136+
AtomicReference<Object> result = new AtomicReference<>();
3137+
AtomicReference<SWTException> failure = new AtomicReference<>();
3138+
3139+
Browser b = new Browser(shell, SWT.NONE);
3140+
createdBroswers.add(b);
3141+
b.setUrl("about:blank");
3142+
new BrowserFunction(b, "f1") {
3143+
@Override
3144+
public Object function(Object[] arguments) {
3145+
return 1;
3146+
}
3147+
};
3148+
new BrowserFunction(b, "f2") {
3149+
@Override
3150+
public Object function(Object[] arguments) {
3151+
return 2;
3152+
}
3153+
};
3154+
b.addProgressListener(completedAdapter(e -> {
3155+
try {
3156+
result.set(b.evaluate("return f1() + f2();"));
3157+
} catch (SWTException ex) {
3158+
failure.set(ex);
3159+
}
3160+
}));
3161+
3162+
shell.open();
3163+
waitForPassCondition(() -> result.get() != null || failure.get() != null);
3164+
if (failure.get() != null) {
3165+
throw failure.get();
3166+
}
3167+
assertNotNull(result.get(), "Neither BrowserFunction was available on the first loaded page");
3168+
assertEquals(3.0, ((Number) result.get()).doubleValue(),
3169+
"Both BrowserFunctions created during concurrent initialization must be available on the first page");
3170+
}
3171+
30483172
/**
30493173
* Regression test: a disposed BrowserFunction must no longer be available (re-injected) after a
30503174
* subsequent navigation. This verifies that deregistration removes the persistent document-created

0 commit comments

Comments
 (0)