[Android] BlazorWebView: Simplify Blazor startup scripts - #35053
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35053Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35053" |
There was a problem hiding this comment.
Pull request overview
Refactors Android BlazorWebView startup initialization to reduce nested EvaluateJavascript callback chains, and adds device tests/helpers to validate startup invariants and messaging behavior.
Changes:
- Replaces the nested Android startup script sequence with a single
BlazorInitScriptexecuted via oneEvaluateJavascriptcall. - Adds new BlazorWebView device tests focused on startup flags/port capture/idempotency and message dispatch filtering.
- Introduces
WebViewHelpers.WaitForCondition()to poll arbitrary JS conditions using existing retry infrastructure.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/BlazorWebView/src/Maui/Android/WebKitWebViewClient.cs | Consolidates Android Blazor startup JS into a single init script and simplifies native callback flow. |
| src/BlazorWebView/tests/DeviceTests/Elements/BlazorWebViewTests.Startup.cs | Adds startup-focused device tests for Android/all platforms. |
| src/BlazorWebView/tests/DeviceTests/WebViewHelpers.Shared.cs | Adds a generic “wait for JS condition” helper used by the new tests. |
ea7944d to
5ed2bbf
Compare
5ed2bbf to
551c803
Compare
551c803 to
ca96b1d
Compare
|
/review |
|
✅ Expert Code Review completed successfully! |
There was a problem hiding this comment.
Expert Code Review — PR #35053
Methodology: 3 independent reviewers with adversarial consensus (initial review + targeted follow-ups for disputed findings).
Summary
This is a well-executed simplification that collapses a 3-level nested EvaluateJavascript callback chain into a single self-contained init script. The elimination of the JS-side MessageChannel relay is a genuine architectural improvement, and the new capturePort-gated Blazor.start() fixes a latent ordering issue. The 5 new device tests provide thorough coverage of the key behaviors (bridge setup, startup flags, port capture, idempotency, message source filtering).
Findings
| # | Severity | Finding | Consensus | File |
|---|---|---|---|---|
| 1 | 🟡 MODERATE | Missing .catch() on Promise.resolve(Blazor.start()) — unhandled rejection leaves startup permanently blocked |
3/3 reviewers | WebKitWebViewClient.cs:57 |
| 2 | 🟡 MODERATE | BlazorStartupScriptsFinished log semantics changed — fires unconditionally and before actual Blazor.start() completion |
3/3 reviewers | WebKitWebViewClient.cs:214 |
| 3 | 🟢 MINOR | Test setup boilerplate (~15 lines) repeated across all 5 test methods | 2/3 reviewers | BlazorWebViewTests.Startup.cs |
| 4 | 🟢 MINOR | receiveMessage single-callback overwrites (Android) vs multi-callback array (iOS) — pre-existing cross-platform inconsistency |
2/3 reviewers | WebKitWebViewClient.cs:44 |
| 5 | 🟢 MINOR | event.source === null assumption for native PostWebMessage could use documentation note |
2/3 reviewers | WebKitWebViewClient.cs:52 |
Discarded Findings (single reviewer, not corroborated)
sendMessagesilently drops messages before port capture — both follow-up reviewers independently concluded this is pre-existing behavior (not a regression) and the internal bridge is not a public API- Idempotency test validates JS guard pattern rather than re-injecting the actual init script
startedValueassertion inBlazorStartupSetsStartingAndStartedFlagsis tautological afterWaitForWebViewReady
Test Coverage Assessment
✅ Good coverage: 5 new device tests covering startup bridge, flags, port capture, idempotency, and message source filtering. The WaitForCondition helper is a useful reusable addition. The cross-platform BlazorStartupSetsUpWindowExternal test validates the bridge on all platforms, while the Android-specific #if ANDROID tests appropriately scope platform-specific behavior.
Blazor.start() rejection path (aligns with Finding #1). If .catch() is added, a corresponding test would strengthen confidence.
Verdict
LGTM with suggestions. The two 🟡 MODERATE findings (missing .catch() and log semantics) are non-blocking but worth addressing — especially the Promise rejection path, which would make startup failures much harder to diagnose without it. The 🟢 MINOR findings are all non-blocking improvements or pre-existing observations.
Generated by Expert Code Review for issue #35053 · ● 16.1M
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description
Refactors the Android
RunBlazorStartupScriptsmethod inWebKitWebViewClient.csfrom a three-level nestedEvaluateJavascriptcallback chain into oneBlazorInitScriptconstant with one callback.What changed
Before: Three nested
EvaluateJavascriptcalls with threeJavaScriptValueCallbackinstances:window.__BlazorStarted.MessageChannel, sets upwindow.external, and configures port listeners.SetUpMessageChannel()posts the port, injectBlazor.start()and set__BlazorStarted.After: One
BlazorInitScriptconstant and oneEvaluateJavascriptcallback:MessageChannel.window.__nativePortfor JavaScript-to-native messaging.messageevent towindow.external.__callback.Blazor.start()from thecapturePorthandler so the native port is captured first.Startup flags
Introduces two flags:
window.__BlazorStartingis set when the initialization script runs and guards duplicateOnPageFinishedcalls.window.__BlazorStartedis set after theBlazor.start()promise resolves and is used byWaitForWebViewReady.Message-source filtering
The message listener processes only events for which
event.source === null; events from JavaScript contexts with a non-null source are skipped.Preserved behavior
WebMessageChannelsetup throughSetUpMessageChannel().PostWebMessagecalls throughSendMessage().BlazorWebMessageCallbackfor JavaScript messages received on the native port.autostart="false"in templates; no template changes.New device tests
Adds
BlazorWebViewTests.Startup.cswith six tests:BlazorStartupSetsUpWindowExternalsendMessageandreceiveMessageare functionsBlazorStartupSetsStartingAndStartedFlagstrueBlazorStartupCapturesNativePortwindow.__nativePortis set after handoffBlazorStartupScriptIsIdempotentBlazorStartupRejectionPreservesLiveBridgeBlazorMessageDispatchOnlyProcessesNativeSourceMessagesAlso adds
WebViewHelpers.WaitForCondition(), which polls a JavaScript boolean expression through the existing retry infrastructure.Files changed
src/BlazorWebView/src/Maui/Android/WebKitWebViewClient.cssrc/BlazorWebView/src/SharedSource/Log.csBlazorStartupScriptsSubmittedlog eventsrc/BlazorWebView/tests/DeviceTests/Elements/BlazorWebViewTests.Startup.cssrc/BlazorWebView/tests/DeviceTests/WebViewHelpers.Shared.csWaitForConditionValidation status
The trusted Android BlazorWebView Gate currently fails: 24 of 29 tests time out waiting for
window.Blazorandwindow.__BlazorStarted. The implementation requires correction before merge; no passing test result is claimed.