[Win32][Edge] Fix BrowserFunction race condition using AddScriptToExecuteOnDocumentCreated#3166
Open
HeikoKlare wants to merge 1 commit intoeclipse-platform:masterfrom
Open
[Win32][Edge] Fix BrowserFunction race condition using AddScriptToExecuteOnDocumentCreated#3166HeikoKlare wants to merge 1 commit intoeclipse-platform:masterfrom
HeikoKlare wants to merge 1 commit intoeclipse-platform:masterfrom
Conversation
Contributor
AddScriptToExecuteOnDocumentCreated In Edge/WebView2, execute() is asynchronous. When new BrowserFunction() is called, the injection script is queued via ExecuteScript(), but if a page navigation completes before WebView2 processes that queued script, the function is unavailable in the new document. This race is most easily triggered when two Browser instances are created in quick succession. Fix: override createFunction() to also register the function script via AddScriptToExecuteOnDocumentCreated. This API guarantees that the script runs on every future document creation, before any page scripts — eliminating the race condition. The script ID returned by the async registration is stored so it can be cleaned up via RemoveScriptToExecuteOnDocumentCreated when the BrowserFunction is disposed. When createFunction() is called from within a WebView2 callback (inCallback>0), blocking on the registration callback would deadlock, so the persistent registration is skipped and the existing NavigationStarting re-injection remains as a fallback. Fixes eclipse-platform#20
Contributor
Author
|
Test plan is done. The automated tests are run via CI anyway. I also successfully executed the reproduction snippet: #2449 (comment) @sratz can you please review this Edge enhancement? |
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
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.
Summary
Fixes #20 (see also #2449 for a reproduction snippet).
In Edge/WebView2,
execute()is asynchronous. Whennew BrowserFunction()is called, the injection script is queued viaExecuteScript(), but if a page navigation completes before WebView2 processes that queued script, the function is unavailable in the new document. This race is most easily triggered when twoBrowserinstances are created in quick succession, as the second browser's initialization forces event-loop processing that can advance the first browser's navigation past the injection window.Fix: Override
createFunction()inEdgeto additionally register the function script viaAddScriptToExecuteOnDocumentCreated. This WebView2 API guarantees the script runs on every future document creation, before any page scripts — eliminating the race condition. The script ID returned by the async registration is stored so it can be cleaned up viaRemoveScriptToExecuteOnDocumentCreatedwhen theBrowserFunctionis disposed.When
createFunction()is called from within a WebView2 callback (inCallback > 0), blocking on the registration would deadlock, so the persistent registration is skipped and the existingNavigationStartingre-injection remains as a fallback for that case.Changes
ICoreWebView2.java: Add missingRemoveScriptToExecuteOnDocumentCreatedvtbl binding (index 28, betweenAddScriptToExecuteOnDocumentCreatedat 27 andExecuteScriptat 29)Edge.java: OverridecreateFunction()to persistently register viaAddScriptToExecuteOnDocumentCreated; overridederegisterFunction()to callRemoveScriptToExecuteOnDocumentCreatedon disposalTest_org_eclipse_swt_browser_Browser.java: Two regression tests (isEdge-only):test_BrowserFunction_availableBeforePageScripts_issue20: registers a function, navigates to a page whose inline<script>immediately calls it, asserts the function was invoked (tests theAddScriptToExecuteOnDocumentCreatedguarantee directly)test_BrowserFunction_availableOnLoad_concurrentInstances_issue20: creates two browsers concurrently without waiting for initialization, verifies both functions are callable when their pages complete loading (replicates the original bug report timing)Test plan
test_BrowserFunction_availableBeforePageScripts_issue20on Windows with Edgetest_BrowserFunction_availableOnLoad_concurrentInstances_issue20on Windows with Edgetest_BrowserFunction_*tests to verify no regressions🤖 Generated with Claude Code