Skip to content

fix(windows): register message bridge at document-start; support injectedJavaScriptBeforeContentLoaded - #3983

Open
FaithfulAudio wants to merge 2 commits into
react-native-webview:masterfrom
FacilitronWorks:wv3-document-start-message-bridge
Open

FaithfulAudio wants to merge 2 commits into
react-native-webview:masterfrom
FacilitronWorks:wv3-document-start-message-bridge

Conversation

@FaithfulAudio

Copy link
Copy Markdown

Part of the hardening series offered in #3980. This is WV-3: register the message bridge at document-start, and wire up injectedJavaScriptBeforeContentLoaded.

Problem

Two related timing bugs in the messaging path:

  • window.ReactNativeWebView (plus the alert() shim and postMessage forwarding) is injected via ExecuteScriptAsync from OnNavigationCompleted — i.e. after the page has already finished loading.
  • WebMessageReceived is unhooked and re-hooked on every OnNavigationStarting.

Real-world failure mode

Any page script that reads or calls window.ReactNativeWebView while the page is still loading — a synchronous inline <script> in <head>, or a framework's early bootstrap code — sees undefined and either throws or silently no-ops, because the bridge doesn't exist until after NavigationCompleted fires. This is a real, commonly-hit ordering bug in embedded-web-content integrations generally (it's the reason injectedJavaScriptBeforeContentLoaded exists as a concept in this library on other platforms) — Windows has no "before content loaded" injection point at all today.

Fix

  • kMessageBridgeScript (contents unchanged from the current NavigationCompleted injection — alert shim, postMessage forwarding, chrome.webview message-event dispatch, all preserved) is registered once via CoreWebView2.AddScriptToExecuteOnDocumentCreatedAsync, in OnCoreWebView2Initialized, instead of re-injected via ExecuteScriptAsync on every completed navigation. This makes it exist before any page script runs, on every subsequent navigation in that CoreWebView2.
  • WebMessageReceived is registered once at the same point instead of on every NavigationStarting.
  • The same AddScriptToExecuteOnDocumentCreatedAsync mechanism now applies injectedJavaScriptBeforeContentLoaded (the prop already exists in the codegen'd spec — RCTWebView2Props::injectedJavaScriptBeforeContentLoaded — but nothing on Windows ever read it).

Trade-off worth flagging

messagingEnabled and injectedJavaScriptBeforeContentLoaded are now captured once, at CoreWebView2 init, rather than re-checked on every navigation. Previously messagingEnabled was checked fresh in both NavigationStarting and NavigationCompleted, so toggling it after mount took effect on the very next navigation; with this change it takes effect on remount instead. WebView2 doesn't offer a clean way to swap a document-created script after the fact short of tracking script IDs and calling RemoveScriptToExecuteOnDocumentCreated — happy to add that if the dynamic-toggle behavior matters for your users; flagging as a deliberate scope cut rather than an oversight.

Validation status

Not compiled against this repo. No Windows build was run this pass — verified by hand against RCTWebView2ComponentView.cpp, not by building.

Document-start bridge registration via AddScriptToExecuteOnDocumentCreatedAsync, and injectedJavaScriptBeforeContentLoaded support built the same way, are both production-proven in a parallel, independently-built Windows new-arch WebView2 Fabric component running in Facilitron FIT (react-native-windows 0.83.2, WinAppSDK 1.8, ARM64) — see the production twin: FacilitronWorks/react-native-webview-windows. Renamed symbols, hand-ported to this repo's codegen/class shape — not a copy-paste.

Related: #3980

…ctedJavaScriptBeforeContentLoaded

Two related timing bugs in the messaging path:

- window.ReactNativeWebView was injected via ExecuteScriptAsync from
  NavigationCompleted -- after the page had already finished loading. Any
  page script that reads window.ReactNativeWebView while the page is still
  loading (a synchronous inline <script> in <head>, for example) never saw
  it.
- WebMessageReceived was unhooked and re-hooked on every NavigationStarting,
  which is wasteful and unnecessary.

This registers the (unchanged) bridge script once via
AddScriptToExecuteOnDocumentCreatedAsync when CoreWebView2 becomes
available, so it exists before any page script runs on every subsequent
navigation, and registers WebMessageReceived once at the same point.

Same mechanism now wires up injectedJavaScriptBeforeContentLoaded, which
already existed in the codegen'd prop spec (RCTWebView2Props) but was never
read on Windows.

Trade-off worth flagging: messagingEnabled / injectedJavaScriptBeforeContentLoaded
are now captured once at CoreWebView2 init rather than re-checked on every
navigation; changing either prop after first mount takes effect on remount
rather than immediately. Happy to add proper add/remove-script tracking
(RemoveScriptToExecuteOnDocumentCreated) if that dynamic-update behavior
matters to you.

Validation: this is the same document-start registration pattern (and the
same injectedJavaScriptBeforeContentLoaded semantics) running in production
in a parallel Windows new-arch WebView2 Fabric component (Facilitron FIT,
react-native-windows 0.83.2) -- see
FacilitronWorks/react-native-webview-windows. Not compiled against this
repo in this change; needs CI here to validate.
…jectedJavaScriptBeforeContentLoaded changes

messagingEnabled and injectedJavaScriptBeforeContentLoaded were captured once at
CoreWebView2 init, so toggling either after mount only took effect on remount.
iOS honors post-mount changes via -resetupScripts (removeAllUserScripts +
re-add) and Android re-runs callInjectedJavaScriptBeforeContentLoaded from
onPageStarted, so Windows was the odd one out.

Tracks the winrt::hstring IDs returned by
AddScriptToExecuteOnDocumentCreatedAsync and removes the previous scripts with
RemoveScriptToExecuteOnDocumentCreated before re-adding, so a prop change takes
effect on the next navigation. UpdateProps captures the old values before any
member is overwritten and compares afterwards, so the dirty check is
old-vs-new by construction rather than by statement order.

Also stops gating injectedJavaScriptBeforeContentLoaded on messagingEnabled, to
match iOS (adds atStartScript outside its messagingEnabled branch) and Android.

Ownership window closed: once AddScriptToExecuteOnDocumentCreatedAsync returns,
the script is registered, so every exit before the ID reaches its member now
removes it. Both the apartment hop (check_hresult'd, can throw) and the
generation-mismatch Remove are handled explicitly. Without that, a throw between
the Add and the member write would orphan the registration: the next call would
read an empty ID, remove nothing, and Add a second bridge - and since
kMessageBridgeScript is not IIFE-wrapped, the duplicate top-level
`const originalPostMessage` would kill the second copy with a SyntaxError.

Adds <cstdint> for uint32_t; no fixed-width integer type was used in these files
before, so transitive availability is not relied on.

UNCOMPILED: no Windows build, MSBuild, or lint was run against this repo.
@FaithfulAudio

Copy link
Copy Markdown
Author

Pushed the fix for the limitation I flagged in the PR body — messagingEnabled and injectedJavaScriptBeforeContentLoaded being captured once at CoreWebView2 init, so post-mount changes only took effect on remount.

What changed

Track the winrt::hstring IDs that AddScriptToExecuteOnDocumentCreatedAsync returns, and remove the previous scripts with RemoveScriptToExecuteOnDocumentCreated before re-adding. That matches iOS -resetupScripts: (removeAllUserScripts + re-add) and Android re-running callInjectedJavaScriptBeforeContentLoaded from onPageStarted.

UpdateProps now captures the old values of both inputs before any member is overwritten and compares afterwards, so the dirty check is old-vs-new by construction rather than by statement order — an earlier revision computed it inline per-assignment, which was correct as written but one careless edit away from comparing new-vs-new.

Also stopped gating injectedJavaScriptBeforeContentLoaded on messagingEnabled. Windows derives messagingEnabled from typeof onMessage === 'function', so gating meant "no onMessage handler" silently dropped the pre-content script — whereas iOS adds atStartScript outside its if(_messagingEnabled) branch and Android doesn't gate it either.

The part worth reviewing closely

Once AddScriptToExecuteOnDocumentCreatedAsync returns, the script is registered, so there's a window until its ID reaches the member where any throw orphans the registration. That matters more than it looks: the next call would read an empty ID, remove nothing, and Add a second bridge — and since kMessageBridgeScript isn't IIFE-wrapped, the duplicate top-level const originalPostMessage would kill the second copy with a SyntaxError. Self-inflicted breakage on the error path.

Both throwing calls in that window are now handled explicitly: co_await on the apartment_context (which is check_hresult'd and can throw) and the generation-mismatch Remove. Every exit before the member write removes what it added.

One residual I'd rather state than hide: if that undo-Remove itself fails, the script stays registered with no ID stored. I couldn't find a way to close that without writing members off the UI thread, which trades one hazard for another. Open to a better shape if you have one.

Added #include <cstdint> for uint32_t — no fixed-width integer type was used in these files before, so I didn't want to rely on it arriving transitively.

Honest limits

  • Uncompiled. No Windows build, MSBuild, or lint has run against this repo — there's no C++ toolchain or WebView2/RNW header set in the environment I wrote it in. It's written against the real sources at the PR head and the real WebView2 projection, but treat every C++ claim as "read from headers", not "built". The apartment-hop reasoning in particular is from cppwinrt/winrt/base.h, not from observed behaviour.
  • CI has never run on this PR. Five workflow runs have sat at action_required since 2026-07-11 awaiting maintainer approval, so nothing here has been built by CI either.
  • One assumption I could not check: clearing injectedJavaScriptBeforeContentLoaded now removes the script unconditionally, which is only right if RNW's codegen'd props deliver a complete snapshot per update rather than a delta. If it's a delta, an unrelated prop change would wipe the pre-content script and cause a spurious remove/re-add each render. I had no RNCWebViewCodegen headers available to confirm which it is. If you know, that's the one line to check.

Context

@manage-some is independently testing this series on RNW 0.84.1 Fabric and has confirmed #3981 fixes the black-screen-on-relaunch. This addresses the post-mount-toggle gap I warned them about, so messagingEnabled / injectedJavaScriptBeforeContentLoaded set dynamically should now behave.

To be precise about what this does and doesn't buy, since it's easy to overstate: the WebMessageReceived handler is now hooked unconditionally, so window.chrome.webview.postMessage works immediately. But a document-created script only applies to future navigations — so attaching onMessage after mount without navigating still won't give you window.ReactNativeWebView on the already-loaded document. The bridge lands on the next navigation, same as before. Matching iOS/Android exactly there would need a ExecuteScriptAsync on the current document as well, which I've deliberately left out of scope.

@FaithfulAudio

Copy link
Copy Markdown
Author

Resolving the caveat I left in my last comment — whether the props reaching UpdateProps are a complete snapshot or a delta, which determines if clearing injectedJavaScriptBeforeContentLoaded unconditionally is correct.

It's a complete snapshot, so the line is correct as written.

The reason the question felt ambiguous is that both halves are true: the payload JS sends is a delta, but the props object handed to UpdateProps is built by cloning the previous object and applying that delta on top. The clinching evidence is in Fabric core, ReactCommon/react/renderer/core/propsConversions.h (lines 175–184 in the copy vendored at RN 0.83.2):

const auto *rawValue = rawProps.at(name, namePrefix, nameSuffix);
if (rawValue == nullptr) [[likely]] {
  return sourceValue;
}

// Special case: `null` always means "the prop was removed, use default
// value".
if (!rawValue->hasValue()) [[unlikely]] {
  return defaultValue;
}

Two things follow directly:

  • When a prop is absent from the delta, convertRawProp returns sourceValue — the value from the previous props object. That carry-forward branch is annotated [[likely]], which would be strange-to-dead code if props objects were deltas; it's the common path precisely because most props are unchanged and inherited.
  • A cleared prop does not arrive as an omission. React writes an explicit null sentinel for removal, and that lands in the !rawValue->hasValue() branch, yielding the default. So "cleared" is distinguishable from "unchanged" at this layer.

Upstream of that, the props object is constructed by cloneProps(propsParserContext, shadowNode.getProps(), std::move(rawProps)) — the clone base being the node's previous props — so by the time a component sees newProps, every field is populated: either inherited or overridden.

Consequence for this PR: an unrelated prop change cannot wipe the pre-content script. newProps->injectedJavaScriptBeforeContentLoaded still carries its prior value on an unrelated update, so the unconditional
has_value() ? to_hstring(...) : winrt::hstring{} is right, and the dirty check won't fire spuriously. Clearing the prop from JS does produce !has_value(), which is the behaviour we want — the script gets removed.

I'm deliberately not extending this to the other props in that function. userAgent in particular should not move to the unconditional form: clearing it would call Settings().UserAgent(L""), setting an empty user agent rather than restoring the WebView2 default, and OnCoreWebView2Initialized already treats empty as "don't touch". Same reasoning may apply elsewhere, so I'd rather leave those alone than generalise a pattern past where I've checked it.

Still unverified, unchanged from before: this is uncompiled — no Windows build or lint has run against this repo, and CI here has never run on this PR (five workflow runs have sat at action_required since 2026-07-11). The snapshot-vs-delta question above is settled from source; the C++ around it is not.

@FaithfulAudio

Copy link
Copy Markdown
Author

Friendly bump — this has been open for a while without a decision. Is there anything you need from me to move it along (rebase onto current main, splitting it, extra tests)? Happy to update it either way.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants