[Edge] Use getWebView(false) in registerFunctionScript to avoid pumping past NavigationCompleted#3256
Draft
HeikoKlare wants to merge 1 commit intoeclipse-platform:masterfrom
Conversation
Contributor
Test Results (win32) 29 files 29 suites 4m 31s ⏱️ For more details on these errors, see this check. Results for commit a07954a. ♻️ This comment has been updated with latest results. |
…ng past NavigationCompleted Previously, registerFunctionScript called getWebView(true), which pumps the event loop until all pending WebView2 tasks in lastWebViewTask are done — including any Navigate call scheduled by a preceding setUrl(). This caused NavigationCompleted to fire during the pump, before the caller of createFunction() had a chance to register a ProgressListener. The NavigationCompleted event was therefore lost and the ProgressListener never fired, causing tests that register the listener after creating a BrowserFunction to time out. Using getWebView(false) limits the pump to waiting only for WebView2 initialization (webViewWrapperFuture::isDone). The Navigate call is chained on the ForkJoinPool after initialization completes and cannot have run yet when getWebView(false) returns, so NavigationCompleted cannot fire during registerFunctionScript. AddScriptToExecuteOnDocumentCreated is still called on a fully initialized WebView2 instance. The currently loaded document is not affected: super.createFunction() already injects the function script into it via nonBlockingExecute() before registerFunctionScript is entered. See eclipse-platform#3254 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
HeikoKlare
force-pushed
the
fix/edge-registerFunctionScript-getWebView
branch
from
59af2fc to
a07954a
Compare
Contributor
Author
|
This change (though probably reasonable) does not fix the actual issue. The latest CI run shows the test failure that was supposed to be fixed: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
registerFunctionScriptcalledgetWebView(true), which pumps the event loop viaprocessOSMessagesUntiluntil all pending WebView2 tasks inlastWebViewTaskare done. WhencreateFunction()is called aftersetUrl(), the pending tasks include theNavigatecall scheduled bysetUrl. Pumping until that task completes gives WebView2 enough time to also fireNavigationCompleted— before the caller ofcreateFunction()has had a chance to register aProgressListener.As a result, the
NavigationCompletedevent is dispatched and lost insideregisterFunctionScript, theProgressListenernever fires, and any test that registers aProgressListenerafter creating aBrowserFunctionwill time out. This manifested as a flaky failure oftest_BrowserFunction_availableOnLoad_concurrentInstances_issue20in CI builds (see issue #3254).The
asyncExecdeferral increateFunctiondoes not help in this scenario because it only triggers whencreateFunctionis called from inside a WebView2 callback (inCallback > 0). When called from regular Java code,inCallback == 0andregisterFunctionScriptis entered synchronously.Fix
Change
getWebView(true)togetWebView(false)inregisterFunctionScript.getWebView(false)limits the event loop pump to waiting only for WebView2 initialization (webViewWrapperFuture::isDone). TheNavigatecall is chained on the ForkJoinPool after initialization completes and cannot have executed yet whengetWebView(false)returns, soNavigationCompletedcannot fire duringregisterFunctionScript.AddScriptToExecuteOnDocumentCreatedis still called on a fully initialized WebView2 instance, so the behaviour is otherwise identical.The currently loaded document is not affected by this change:
super.createFunction()already injects the function script into the current document vianonBlockingExecute()beforeregisterFunctionScriptis entered.Related