[BlazorWebView] Add opt-in static content caching - #35706
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35706Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35706" |
|
Hey there @@Kebechet! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
Hey there @Kebechet! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
github-actions
Bot
added
area-blazor
|
/review -b feature/enhanced-reviewer -p android |
MauiBot
added
the
s/agent-reviewed
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
kubaflo
left a comment
kubaflo
left a comment
There was a problem hiding this comment.
Could you please check the ai's suggestions?
|
Thanks for the review and for the automated
On the
That said, the two approaches aren't mutually exclusive: if maintainers prefer to also relax the default (opt-out), this same callback is the natural escape hatch for apps that want to force |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an app-extensibility point to override Cache-Control for static assets served by BlazorWebView, while keeping the historical default of disabling caching to ensure user scripts re-execute consistently.
Changes:
- Introduces
StaticContentCacheControlProviderandBlazorWebViewStaticContentRequestpublic API surface. - Applies platform-specific
Cache-Controloverride logic across iOS, Windows (WinUI), Android, and Tizen handlers. - Adds device tests validating override behavior, default preservation, and that resolved content-type is passed to the provider.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BlazorWebView/tests/DeviceTests/Elements/BlazorWebViewTests.StaticContentCaching.cs | Adds device tests for cache-control override behavior and content-type propagation. |
| src/BlazorWebView/src/Maui/iOS/BlazorWebViewHandler.iOS.cs | Applies override-or-default cache-control header for iOS served static content. |
| src/BlazorWebView/src/Maui/Windows/WinUIWebViewManager.cs | Applies cache-control override for WinUI static content responses. |
| src/BlazorWebView/src/Maui/Tizen/BlazorWebViewHandler.Tizen.cs | Applies cache-control override for Tizen static content served via interceptor. |
| src/BlazorWebView/src/Maui/StaticContentCacheControl.cs | Centralizes default cache-control value and override resolution helper. |
| src/BlazorWebView/src/Maui/PublicAPI/net/PublicAPI.Unshipped.txt | Tracks new public APIs for net target. |
| src/BlazorWebView/src/Maui/PublicAPI/net-windows/PublicAPI.Unshipped.txt | Tracks new public APIs for net-windows target. |
| src/BlazorWebView/src/Maui/PublicAPI/net-tizen/PublicAPI.Unshipped.txt | Tracks new public APIs for net-tizen target. |
| src/BlazorWebView/src/Maui/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt | Tracks new public APIs for net-maccatalyst target. |
| src/BlazorWebView/src/Maui/PublicAPI/net-ios/PublicAPI.Unshipped.txt | Tracks new public APIs for net-ios target. |
| src/BlazorWebView/src/Maui/PublicAPI/net-android/PublicAPI.Unshipped.txt | Tracks new public APIs for net-android target. |
| src/BlazorWebView/src/Maui/IBlazorWebView.cs | Adds interface surface for the provider (default implementation returning null). |
| src/BlazorWebView/src/Maui/BlazorWebViewStaticContentRequest.cs | Adds the request model passed to the provider. |
| src/BlazorWebView/src/Maui/BlazorWebView.cs | Adds the settable provider property with documentation and default behavior description. |
| src/BlazorWebView/src/Maui/Android/WebKitWebViewClient.cs | Applies cache-control override for Android responses returned from the handler. |
| /// Initializes a new instance of the <see cref="BlazorWebViewStaticContentRequest"/> struct. | ||
| /// </summary> |
| // re-executed. It is applied unless the application opts a resource in to caching via | ||
| // BlazorWebView.StaticContentCacheControlProvider. See https://github.com/dotnet/maui/issues/8279 |
|
/review -b feature/enhanced-reviewer |
|
/review rerun |
github-actions
Bot
added
the
s/agent-ready-for-rerun
|
/review -b feature/enhanced-reviewer -p android |
github-actions
Bot
added
the
s/agent-review-in-progress
kubaflo
left a comment
kubaflo
left a comment
There was a problem hiding this comment.
Could you please check the suggestions?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 8 findings
See inline comments for details.
| // opt specific resources into caching via BlazorWebView.StaticContentCacheControlProvider. | ||
| // The original (unstripped) URI is passed so the provider can act on query strings (e.g. img.png?v=2). | ||
| // See https://github.com/dotnet/maui/issues/8279 | ||
| var cacheControlOverride = StaticContentCacheControl.ResolveOverride(_webViewHandler?.VirtualView, originalRequestUri, contentType, logger); |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[major] Cross-Platform Behavioral Consistency — Android WebView does not use its HTTP cache for responses returned from shouldInterceptRequest; the headers on a WebResourceResponse are surfaced to JS (fetch/XHR) but are not interpreted by the WebView for caching. Concrete scenario: an app opts img.png into max-age=3600 via StaticContentCacheControlProvider, the new device test passes because the fetch response echoes max-age=3600, but every subsequent page navigation still calls ShouldInterceptRequest and re-reads the asset from the file provider — so the flicker reported in #8279 is not actually fixed on Android. Please verify on-device that a second load of a cacheable asset does not re-enter this method (see the companion comment on the test file); if the WebView does not honor it, either document the platform limitation on the public API or serve intercepted responses from an app-level cache.
| // The nonce makes this a guaranteed cache miss, so a passing header assertion cannot come from a stale cached | ||
| // response: the provider must have run for the requested resource. | ||
| Assert.True(providerInvokedForTarget, "The provider was not invoked for the requested resource - the response was likely served from the WebView cache."); | ||
| Assert.Equal("max-age=3600", cacheControl); |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[major] Regression Prevention and Test Coverage — Every test here asserts only the value of the header echoed back to JavaScript; none asserts the user-visible behavior from the linked issue (the resource is actually served from the WebView cache, so it stops being re-fetched/flickering). That makes the whole suite pass even if the platform ignores Cache-Control on handler-served responses entirely — which is exactly the risk on Android (shouldInterceptRequest) and iOS (WKURLSchemeHandler), where custom responses are widely reported not to enter the WebView HTTP cache. The discriminating test is cheap and already almost written: fetch the same cacheable URL twice and assert the provider (or a request counter incremented in the handler) was invoked only once. Note the file comment on lines 16-19 already assumes this caching happens across BlazorWebView instances — that assumption is load-bearing for the fix but is never verified.
| } | ||
| return null; | ||
| }, | ||
| fetchPath: "_framework/blazor.modules.json", |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Regression Prevention and Test Coverage — This test cannot discriminate, so the new originalRequestUri parameter added to WinUIWebViewManager.TryServeFromFolderAsync is effectively untested. The comment states it covers the WinUI folder-serving path, but that path only produces a response when the file physically exists under $(BaseDirectory)/wwwroot/_framework/blazor.modules.json. The device-test app has no wwwroot on disk (content comes from BlazorWebViewWithCustomFiles' in-memory provider, and blazor.modules.json does not exist anywhere in the repo), so on Windows TryServeFromFolderAsync finds no stream, returns false, and the request falls through to the TryGetResponseContent branch — the same branch every other platform uses. The assertion therefore passes identically whether or not TryServeFromFolderAsync forwards the unstripped URI. Add the file to the test app's content root (or add a folder-served asset the test controls) so the folder path is genuinely exercised.
|
|
||
| /// <summary> | ||
| /// Gets or sets a callback that determines the <c>Cache-Control</c> header value used for static content | ||
| /// (such as images, fonts, or stylesheets) served from the app's content root. |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Public API Surface Design — The documented scope is narrower than the implemented scope, in a way that can break apps. The doc says the callback determines the header "for static content (such as images, fonts, or stylesheets) served from the app's content root", but the callback is invoked for every app-origin response on all platforms: the host page (index.html), _framework/blazor.webview.js, _framework/blazor.modules.json, and framework assets — none of which come from the content root. The line this PR replaces on iOS said "Disable local caching. This will prevent user scripts from executing correctly", i.e. caching those framework scripts is precisely what the no-store default was guarding against. Concrete scenario: an app writes req => "max-age=86400" (or matches on a broad path prefix), the Blazor startup script and host page get cached, and after an app update the WebView serves stale scripts and Blazor fails to start. Please document that the callback fires for framework files and the host page, and recommend opting in per-resource.
| /// back to the default header. | ||
| /// </para> | ||
| /// </summary> | ||
| public Func<BlazorWebViewStaticContentRequest, string?>? StaticContentCacheControlProvider { get; set; } |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Public API Surface Design — API shape is inconsistent with the existing extensibility points on this control and is not forward-compatible. Every other request/navigation hook on BlazorWebView is an event (UrlLoading, BlazorWebViewInitializing, WebResourceRequested), which (a) can be wired directly in XAML — the primary way BlazorWebView is declared — while a Func<> property cannot, forcing x:Name + code-behind, and (b) can gain new information or new outputs on its EventArgs without a new API. This Func<BlazorWebViewStaticContentRequest, string?> locks the contract to exactly one input object and one Cache-Control string; the next request (ETag/Expires, per-request status code, async resolution) needs a second public API because neither the delegate signature nor the sealed BlazorWebViewStaticContentRequest can change once shipped. Consider a StaticContentRequested event whose args expose the URI/content type and a settable CacheControl, matching WebViewWebResourceRequestedEventArgs.
|
|
||
| // Values containing CR/LF are also rejected: some platforms concatenate the value into a raw response | ||
| // header block, so a stray newline would produce a malformed response or allow header injection. | ||
| if (cacheControl.Contains('\r', StringComparison.Ordinal) || cacheControl.Contains('\n', StringComparison.Ordinal)) |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Null Safety and Defensive Coding — Two of the three rejection paths are silent, which leaves developers with no way to diagnose an ignored value. If an app returns a value containing CR/LF (e.g. built with Environment.NewLine or read from a config file with a trailing newline), the value is discarded and the default no-store is sent with no log entry at any level — the app author sees "my caching setting does nothing" and has nothing to grep for. Same for the Uri.TryCreate failure on line 24. The throwing path (line 39) correctly logs, so the infrastructure is already here: add a warning-level log for the rejected-value and unparsable-URI cases. Secondary: new BlazorWebViewStaticContentRequest(uri, contentType) on line 32 is constructed inside the try, so a framework-side null contentType surfaces as ArgumentNullException and is logged as "The StaticContentCacheControlProvider threw an exception", misattributing a framework bug to app code — construct the request before the try.
| // layer of quotes. Peel any remaining JSON-string layers so callers get the raw value. | ||
| if (value is string str) | ||
| { | ||
| while (TryDeserialize<string>(str, out var peeled) && peeled is not null && peeled != str) |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[minor] Regression Prevention and Test Coverage — This unbounded peel loop changes behavior for all future T = string callers of this shared helper, not just the new cache-control tests (which are currently the only <string> callers, so no existing test regresses today). Any string result that is itself a valid JSON string literal will be over-peeled and silently returned unquoted, which is impossible to distinguish from a correct result. Also, when the header is absent the page stores null, TryDeserialize short-circuits on the literal "null" (line 102), and the helper returns the 4-character string null — so Assert.Contains("no-store", cacheControl) in the new tests fails with Assert.Contains() Failure ... "null" instead of a clear "no Cache-Control header was served". Consider peeling exactly one layer (the documented double-encoding) rather than looping, and returning null for the JS null case.
| private TizenWebViewManager? _webviewManager; | ||
|
|
||
| private ILogger? _logger; | ||
| internal ILogger Logger => _logger ??= Services!.GetService<ILogger<BlazorWebViewHandler>>() ?? NullLogger<BlazorWebViewHandler>.Instance; |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[minor] Architectural Layer Placement — This is now the third byte-identical copy of the same Logger property (Android/BlazorWebViewHandler.Android.cs:28, iOS/BlazorWebViewHandler.iOS.cs:56, and here). Since BlazorWebViewHandler is a partial class with a shared BlazorWebViewHandler.cs, this lazily-resolved logger (and its _logger backing field) belongs there once rather than being copy-pasted per platform — the next platform-specific logging change now has to be made in three places and will silently drift.
This comment has been minimized.
This comment has been minimized.
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 4 findings
See inline comments for details.
| /// <summary> | ||
| /// Gets the absolute URI of the requested static content. | ||
| /// </summary> | ||
| public Uri Uri { get; } |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Cross-Platform Behavioral Consistency — Uri is documented only as "the absolute URI of the requested static content", but its scheme and origin differ per platform because each handler passes its own AppOrigin-based request URL:
- iOS / MacCatalyst:
app://0.0.0.0/...(BlazorWebViewHandler.iOS.cs:AppOrigin = "app://" + BlazorWebView.AppHostAddress + "/", andStartUrlSchemeTaskpassesurlSchemeTask.Request.Url.AbsoluteString) - Android:
https://0.0.0.0/...(WebKitWebViewClient.AppOrigin) - Windows:
https://0.0.0.0/...(eventArgs.Request.Uri) - Tizen:
http://0.0.0.0/...(BlazorWebViewHandler.Tizen.cs:AppOrigin = "http://0.0.0.0/")
Concrete failure: an app writing the natural check request.Uri.AbsoluteUri.StartsWith("https://0.0.0.0/images/") (or request.Uri.Scheme == "https") works on Android/Windows and silently never matches on iOS/MacCatalyst, so images keep flickering on exactly the platform the callback was added for. The PR's own tests only ever inspect AbsolutePath/Query, so they cannot surface this.
Either document on this property that only path/query are portable and the scheme/host are platform-specific, or normalize what is handed to the callback so all platforms agree.
|
|
||
| /// <summary> | ||
| /// Gets or sets a callback that determines the <c>Cache-Control</c> header value used for static content | ||
| /// (such as images, fonts, or stylesheets) served from the app's content root. |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Public API Surface Design — The doc scopes the callback to "static content (such as images, fonts, or stylesheets)" and the argument type is named BlazorWebViewStaticContentRequest, but every serving path wired up in this PR invokes it for all content served from the content root, including the host page and framework assets: WebKitWebViewClient.GetResponse, BlazorWebViewHandler.iOS.SchemeHandler.StartUrlSchemeTask, WinUIWebViewManager.HandleWebResourceRequest/TryServeFromFolderAsync (which explicitly covers _framework/blazor.modules.json), and the Tizen interceptor.
Concrete failure: a developer follows the documented intent and returns "max-age=86400" for a broad prefix (e.g. anything under /, or anything without a .png check). index.html and _framework/blazor.webview.js are then cached by the WebView for a day, so a shipped app update serves the stale host page/startup script — the exact scenario the pre-existing no-store default (and the "user scripts are always re-executed" comment repeated in each platform file) was protecting against.
Document here that the callback also receives the host page and _framework/* requests and that returning a cacheable value for them is unsafe, or exclude those requests from the callback.
|
|
||
| // Values containing CR/LF are also rejected: some platforms concatenate the value into a raw response | ||
| // header block, so a stray newline would produce a malformed response or allow header injection. | ||
| if (cacheControl.Contains('\r', StringComparison.Ordinal) || cacheControl.Contains('\n', StringComparison.Ordinal)) |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Logic and Correctness Verification — The CR/LF rejection (and the whitespace rejection above it) silently returns null, while the only other failure path — the provider throwing — logs via StaticContentCacheControlProviderFailed. The two silent branches are the ones a developer is most likely to hit by accident.
Concrete scenario: a provider builds its value from configuration and returns "max-age=3600\n" (trailing newline from a file read) or $"max-age={ReadSetting()}" where the setting is empty. The request is served with no-cache, ..., no-store, the app still flickers, and there is nothing in the log to distinguish "my provider was never called" from "my value was rejected". Since the CR/LF branch is a security guard against header injection, dropping it without a trace also hides a genuine attack signal.
Add a log call (e.g. a LogWarning/new LoggerMessage carrying requestUri and the reason) before each return null; in these two validation branches.
| // a raw response header block, where a newline would produce a malformed response or allow header injection. | ||
| var cacheControl = await GetServedCacheControlHeaderAsync(_ => "max-age=3600\r\nX-Injected: 1", fetchQueryString: "?test=newline-provider"); | ||
|
|
||
| Assert.Contains("no-store", cacheControl, StringComparison.Ordinal); |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Regression Prevention and Test Coverage — This test and its four siblings (...ReturningNullKeepsDefaultNoStore L51, ...ReturningEmptyStringKeepsDefaultNoStore L61, ...ReturningWhitespaceKeepsDefaultNoStore L71, ...ThrowingKeepsDefaultNoStore L93) assert only Assert.Contains("no-store", cacheControl). no-store is also exactly what is served when the provider is never consulted at all, so the fixture conflates two signals and the assertion cannot discriminate.
Concrete consequence: delete the ResolveOverride call from WebKitWebViewClient.GetResponse / BlazorWebViewHandler.iOS.cs / WinUIWebViewManager — i.e. remove the feature on a platform — and all five of these tests still pass green. Only StaticContentCacheControlProviderCanOverrideCacheControlHeader (L41) guards against that, and only for the override case; the validation guards this PR adds (whitespace, CR/LF, exception swallowing) therefore have no test that can fail if the guard, or the call into it, regresses.
Add the same providerInvoked flag pattern used at L25/L41 to each of these tests (set it inside the provider lambda, Assert.True(providerInvoked, ...) before the header assertion) so a passing run proves the provider actually ran and its value was rejected.
AI Review Summary
🗂️ Review Sessions — click to expand🚦 Gate — Test Before & After FixGate Result:
|
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
📱 BlazorWebViewTests (StaticContentCacheControlProviderCanOverrideCacheControlHeader, StaticContentCacheControlProviderReturningNullKeepsDefaultNoStore, StaticContentCacheControlProviderReturningEmptyStringKeepsDefaultNoStore, StaticContentCacheControlProviderReturningWhitespaceKeepsDefaultNoStore, StaticContentCacheControlProviderReturningValueWithNewlinesKeepsDefaultNoStore, StaticContentCacheControlProviderThrowingKeepsDefaultNoStore, StaticContentCacheControlProviderReceivesResolvedContentType, StaticContentCacheControlProviderReceivesQueryString, StaticContentCacheControlProviderReceivesQueryStringForFolderServedContent) Category=BlazorWebView |
🛠️ BUILD ERROR |
🔴 Without fix — 📱 BlazorWebViewTests (StaticContentCacheControlProviderCanOverrideCacheControlHeader, StaticContentCacheControlProviderReturningNullKeepsDefaultNoStore, StaticContentCacheControlProviderReturningEmptyStringKeepsDefaultNoStore, StaticContentCacheControlProviderReturningWhitespaceKeepsDefaultNoStore, StaticContentCacheControlProviderReturningValueWithNewlinesKeepsDefaultNoStore, StaticContentCacheControlProviderThrowingKeepsDefaultNoStore, StaticContentCacheControlProviderReceivesResolvedContentType, StaticContentCacheControlProviderReceivesQueryString, StaticContentCacheControlProviderReceivesQueryStringForFolderServedContent): 🛠️ BUILD ERROR · 93s
Error-relevant lines (filtered from the build log):
/home/vsts/work/1/s/src/BlazorWebView/tests/DeviceTests/Elements/BlazorWebViewTests.StaticContentCaching.cs(164,67): error CS0246: The type or namespace name 'BlazorWebViewStaticContentRequest' could not be found (are you missing a using directive or an assembly reference?) [/home/vsts/work/1/s/src/BlazorWebView/tests/DeviceTests/MauiBlazorWebView.DeviceTests.csproj::TargetFramework=net10.0-android]
Build FAILED.
🟢 With fix — 📱 BlazorWebViewTests (StaticContentCacheControlProviderCanOverrideCacheControlHeader, StaticContentCacheControlProviderReturningNullKeepsDefaultNoStore, StaticContentCacheControlProviderReturningEmptyStringKeepsDefaultNoStore, StaticContentCacheControlProviderReturningWhitespaceKeepsDefaultNoStore, StaticContentCacheControlProviderReturningValueWithNewlinesKeepsDefaultNoStore, StaticContentCacheControlProviderThrowingKeepsDefaultNoStore, StaticContentCacheControlProviderReceivesResolvedContentType, StaticContentCacheControlProviderReceivesQueryString, StaticContentCacheControlProviderReceivesQueryStringForFolderServedContent): ⚠️ ENV ERROR · 671s
(no coded error found; showing last 1200 chars)
k/1/s/artifacts/log/adb-logcat-com.microsoft.maui.mauiblazorwebview.devicetests-default.log
info: ADB log contained no DOTNET-tagged entries (see full log file for details)
info: Wrote ADB bugreport to /home/vsts/work/1/s/artifacts/log/adb-bugreport-com.microsoft.maui.mauiblazorwebview.devicetests.zip
info: <<XHARNESS_RESULT_START>>
{
"version": 1,
"machineName": "runnervm1jb2f",
"exitCode": 80,
"exitCodeName": "APP_CRASH",
"platform": "android",
"device": "emulator-5554",
"deviceOsVersion": "API 30",
"architecture": "x86_64",
"files": [
{
"name": "adb-logcat-com.microsoft.maui.mauiblazorwebview.devicetests-default.log",
"type": "logcat"
},
{
"name": "adb-bugreport-com.microsoft.maui.mauiblazorwebview.devicetests.zip",
"type": "bugreport"
}
]
}
<<XHARNESS_RESULT_END>>
info: Attempting to remove apk 'com.microsoft.maui.mauiblazorwebview.devicetests'..
info: Successfully uninstalled com.microsoft.maui.mauiblazorwebview.devicetests
XHarness exit code: 80 (APP_CRASH)
Tests completed with exit code: 80
⚠️ Failure Details
- 🛠️ BlazorWebViewTests (StaticContentCacheControlProviderCanOverrideCacheControlHeader, StaticContentCacheControlProviderReturningNullKeepsDefaultNoStore, StaticContentCacheControlProviderReturningEmptyStringKeepsDefaultNoStore, StaticContentCacheControlProviderReturningWhitespaceKeepsDefaultNoStore, StaticContentCacheControlProviderReturningValueWithNewlinesKeepsDefaultNoStore, StaticContentCacheControlProviderThrowingKeepsDefaultNoStore, StaticContentCacheControlProviderReceivesResolvedContentType, StaticContentCacheControlProviderReceivesQueryString, StaticContentCacheControlProviderReceivesQueryStringForFolderServedContent) without fix: build failed before tests could run
/home/vsts/work/1/s/src/BlazorWebView/tests/DeviceTests/Elements/BlazorWebViewTests.StaticContentCaching.cs(164,67): error CS0246: The type or namespace name 'BlazorWebViewStaticContentRequest' could ...
⚠️ BlazorWebViewTests (StaticContentCacheControlProviderCanOverrideCacheControlHeader, StaticContentCacheControlProviderReturningNullKeepsDefaultNoStore, StaticContentCacheControlProviderReturningEmptyStringKeepsDefaultNoStore, StaticContentCacheControlProviderReturningWhitespaceKeepsDefaultNoStore, StaticContentCacheControlProviderReturningValueWithNewlinesKeepsDefaultNoStore, StaticContentCacheControlProviderThrowingKeepsDefaultNoStore, StaticContentCacheControlProviderReceivesResolvedContentType, StaticContentCacheControlProviderReceivesQueryString, StaticContentCacheControlProviderReceivesQueryStringForFolderServedContent) with fix:App crashed during test run (XHarness exit 80 APP_CRASH)
📁 Fix files reverted (14 files)
src/BlazorWebView/src/Maui/Android/WebKitWebViewClient.cssrc/BlazorWebView/src/Maui/BlazorWebView.cssrc/BlazorWebView/src/Maui/IBlazorWebView.cssrc/BlazorWebView/src/Maui/PublicAPI/net-android/PublicAPI.Unshipped.txtsrc/BlazorWebView/src/Maui/PublicAPI/net-ios/PublicAPI.Unshipped.txtsrc/BlazorWebView/src/Maui/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txtsrc/BlazorWebView/src/Maui/PublicAPI/net-tizen/PublicAPI.Unshipped.txtsrc/BlazorWebView/src/Maui/PublicAPI/net-windows/PublicAPI.Unshipped.txtsrc/BlazorWebView/src/Maui/PublicAPI/net/PublicAPI.Unshipped.txtsrc/BlazorWebView/src/Maui/Tizen/BlazorWebViewHandler.Tizen.cssrc/BlazorWebView/src/Maui/Windows/StaticContentProvider.cssrc/BlazorWebView/src/Maui/Windows/WinUIWebViewManager.cssrc/BlazorWebView/src/Maui/iOS/BlazorWebViewHandler.iOS.cssrc/BlazorWebView/src/SharedSource/Log.cs
New files (not reverted):
src/BlazorWebView/src/Maui/BlazorWebViewStaticContentRequest.cssrc/BlazorWebView/src/Maui/StaticContentCacheControl.cs
📋 Pre-Flight — Context & Validation
Issue: #8279 - Maui Blazor image cache is not working
PR: #35706 - [BlazorWebView] Add StaticContentCacheControlProvider for static content caching
Platforms Affected: Android, iOS/MacCatalyst, Windows, Tizen
Files Changed: 16 implementation, 2 test
Key Findings
- The Android reproduction keeps the same
BlazorWebViewalive while local images blink on repeated navigation; served assets currently carryCache-Control: no-cache, max-age=0, must-revalidate, no-store. - The PR adds an opt-in public per-request callback and preserves the historical default when the callback is absent or declines an override.
- Prior maintainer feedback remains unresolved around the public API shape, shared cache semantics, caching of 404 responses, and tests that assert response headers without proving a second request is served from cache.
- The supplied gate result is inconclusive because the tests could not be built or run; this is an environment/build blocker, not evidence that the PR fix failed.
Code Review Summary
Verdict: NEEDS_CHANGES
Confidence: low
Errors: 1 | Warnings: 3 | Suggestions: 2
Key code review findings:
- ✗
handler.VirtualViewis a throwing typed getter after disconnect, so every platform's new request path can crash before the nullable provider guard runs (Android/WebKitWebViewClient.cs:135, plus iOS, Windows, and Tizen call sites). - ⚠ The provider is invoked for missing resources on Android/iOS/Tizen, allowing cacheable 404 placeholders that the callback cannot identify (
Android/WebKitWebViewClient.cs:135). - ⚠ Device tests fetch each resource only once and prove header emission, not actual WebView caching or resolution of the image-flicker reproduction (
BlazorWebViewTests.StaticContentCaching.cs:21-203). - ⚠ CR/LF-only validation still permits other control characters that can corrupt raw platform header blocks (
StaticContentCacheControl.cs:53). - ℹ Add a provider-less exact-default test and explicitly scope or support WPF/WinForms BlazorWebView.
- Failure-mode probe: an Android request callback can race
DisconnectHandler;_webViewHandler?.VirtualViewonly null-checks the handler and still throws when its typedVirtualViewhas been cleared. - Blast radius: the new request logic runs for every app-origin static request, including startup resources and apps that never opt into the feature; no mutable static state was added.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35706 | Add a public per-request cache-control provider and apply its result in each platform request handler | 18 files | Original PR |
🔬 Code Review — Deep Analysis
Code Review — PR #35706
[BlazorWebView] Add StaticContentCacheControlProvider for static content caching · head 6f1987c5 · base inflight/current · 18 files, +442/−4
Independent Assessment
What this changes: Adds an opt-in Func<BlazorWebViewStaticContentRequest, string?>? on BlazorWebView (+ a default-interface getter on IBlazorWebView, + a public BlazorWebViewStaticContentRequest POCO). A new internal StaticContentCacheControl.ResolveOverride(...) is called from all four platform request handlers (Android WebKitWebViewClient.GetResponse, iOS SchemeHandler.StartUrlSchemeTask, WinUIWebViewManager × 2 paths, Tizen OnRequestInterceptCallback) to replace the historical Cache-Control: no-cache, max-age=0, must-revalidate, no-store when the app returns a value. Null/empty/whitespace/CR-LF values, unparseable URIs, and provider exceptions all fall back to the default.
Inferred motivation: no-store on every served asset forces images/fonts/CSS to be re-fetched and re-decoded on every render — the flicker in #8279. Default is preserved, so it is additive.
Is the approach sound? The shape (per-request callback, default preserved, centralized helper) is reasonable. The mechanics are where it breaks down: the helper's IBlazorWebView? parameter promises null-tolerance that the call sites cannot deliver.
External Output Contract
| Consumer token/pattern | Producer location | Producer emission condition | Consumer assumption | Ordinary negative case | Downstream effect |
|---|---|---|---|---|---|
headers["Content-Type"] / TryGetValue("Content-Type") |
aspnetcore StaticContentProvider.GetResponseHeaders (verified in Microsoft.AspNetCore.Components.WebView.dll 10.0.0) |
Always emitted with the resolved MIME type, including for 404 placeholders (text/plain) |
"A resolved content type for a real asset" | Missing asset → text/plain 404 placeholder |
Provider is invoked and its cacheable header stamped on a 404 (see |
headers["Cache-Control"] default |
same producer; literal no-cache, max-age=0, must-revalidate, no-store |
Every TryGetResponseContent* success |
PR assumes replacing the dict entry is byte-identical when no provider | No provider set | ✅ Correct — Android/Windows/Tizen keep the dict entry; iOS re-emits the same literal via ?? Default |
| Provider return string | arbitrary app code | Unconstrained | "Safe to concatenate into a raw header block" | Value with \0 or other CTL chars |
Passes the CR/LF-only guard → Tizen raw block truncation / WinUI CreateWebResourceResponse throw ( |
handler.VirtualView |
ViewHandler<TVirtual,TPlatform> |
Throws InvalidOperationException when null (ViewHandlerOfT.cs:40-44) |
ResolveOverride(IBlazorWebView? …) + blazorWebView?. implies it can be null |
Handler disconnected while a request is in flight | ❌ #1 below |
Reconciliation with PR Narrative
Author claims: purely additive; default byte-identical; per-resource control including query strings; device tests cover override / null-default / content-type; device tests never run locally.
Agreement: The non-breaking-default claim checks out on all four platforms (verified against the aspnetcore assembly's header literal and iOS's ?? Default). PublicAPI entries are complete and correctly annotated across all six TFM folders.
Disagreement: "existing apps are unaffected" is not fully true — ResolveOverride's argument (handler.VirtualView) is evaluated on every app-origin request before the provider is null early-out, so apps that never touch the new API inherit a new throwing path. The "device tests would be the real validation" caveat is load-bearing: the tests assert only the header echoed to JS, never that the resource is actually cached.
Prior Review Reconciliation
| Prior finding (severity) | Source | Status | Evidence |
|---|---|---|---|
| [major] Android passes query-stripped URI to provider | MauiBot 06-11 | ✅ Fixed | WebKitWebViewClient.cs:119,135 passes originalRequestUri |
| [major] WinUI folder path passes stripped URI | MauiBot 06-13 | ✅ Fixed | new originalRequestUri param, WinUIWebViewManager.cs:165,208 |
[moderate] Tizen headers["Content-Type"] indexer can throw |
MauiBot 06-13 | ✅ Fixed | BlazorWebViewHandler.Tizen.cs:137 uses TryGetValue |
[moderate] new Uri(...) unguarded on background thread |
MauiBot 06-13 | ✅ Fixed | StaticContentCacheControl.cs:25 Uri.TryCreate |
| [major] Provider invocation not exception-guarded | MauiBot 07-18 | ✅ Fixed | StaticContentCacheControl.cs:29-40 try/catch + Log.cs EventId 19 |
| [moderate] Tests share a cacheable URL / persisted cache false-pass | MauiBot 07-14→07-18 | ✅ Fixed | per-test query + Guid nonce + providerInvokedForTarget assert |
| [moderate] Empty string treated as null | MauiBot 07-13 | 🔄 Declined, documented + tested | XML docs on BlazorWebView.cs/IBlazorWebView.cs; whitespace test |
[minor] Test helper Func<…, string> vs string? |
MauiBot/Copilot | 🔄 Declined with valid reason | DeviceTests project has NRT disabled; string? → CS8632-as-error |
[major] Android/iOS may not honor Cache-Control on intercepted responses → #8279 possibly unfixed |
MauiBot 08-05 | ❌ Unresolved | Head commit is 08-01; no code or doc change since |
| [major] Tests prove header emission, not caching | MauiBot 08-05 | ❌ Unresolved | StaticContentCaching.cs — single fetch per test, no fetch-twice/hit assertion |
| [moderate] Folder-served WinUI test cannot discriminate | MauiBot 08-05 | ❌ Unresolved | No wwwroot/_framework/blazor.modules.json on disk → TryServeFromFolderAsync returns false, request falls through to the shared branch |
| [blocking ×3] API scope vs shared cache; 404s get cacheable headers; tests don't prove caching | kubaflo (maintainer) 08-02 | ❌ Unresolved | Explicit "I would require…" list; no commits since 08-01 |
Rule #5 applies on its own: unresolved prior ❌/[major] findings force NEEDS_CHANGES.
Blast Radius Assessment
- Runs for all instances: yes.
ResolveOverrideis on the static-content path of every BlazorWebView request on every platform, and theVirtualViewargument is evaluated before theprovider is nullcheck — so no-provider apps execute the new code too. - Startup impact: indirect — index.html and
_framework/*are served through these same paths during startup. - Static/shared state: only the new
const string Default(now shared by iOS and WindowsStaticContentProvider.cs:30); no mutable statics.
CI Status
- Required-check result:
gh pr checks --requiredunavailable (ghunauthenticated in this environment, exit 4 — tool-unavailable fallback applied). Collected via public REST + AzDO instead: requiredmaui-pris queued/in_progress;maui-pr-devicetestsfailure; failing legs include Build Windows (Debug), Helix Unit Tests, several Integration Tests, Blazor macOS, RunOniOS_Blazor*. - Classification: pending + pre-existing infra. AzDO timeline (build
1541627) showsProvision JDK Task: PowerShell exited with code '1',MSB4024 … process cannot access the file … .buildtasks\*.dll(file lock), missing.trx/log paths, and a Helix failure inControls.Core.UnitTests— a project this PR does not touch. No CS-level compile error attributable to the diff. The maintainer independently states themaui-prfailures reproduce oninflight/current. - Action taken: inspected AzDO timeline directly (gh/
azdo-build-investigatorauth unavailable); recorded the tooling gap; confidence capped at low (CI pending and red, plus no CI evidence for the new BlazorWebView device tests on Android/iOS).
Findings
❌ Error — handler.VirtualView throws instead of returning null; new unconditional exception path on every static-content request
Windows/WinUIWebViewManager.cs:154 · Android/WebKitWebViewClient.cs:135 · iOS/BlazorWebViewHandler.iOS.cs:299 · Tizen/BlazorWebViewHandler.Tizen.cs:139
ResolveOverride(IBlazorWebView? blazorWebView, …) with blazorWebView?.StaticContentCacheControlProvider (StaticContentCacheControl.cs:16) advertises null-tolerance. None of the four call sites can provide null: all handlers derive from ViewHandler<IBlazorWebView, T>, whose typed getter is (TVirtualView?)base.VirtualView ?? throw new InvalidOperationException(...) (src/Core/src/Handlers/View/ViewHandlerOfT.cs:40-44), and ElementHandler.DisconnectHandler sets VirtualView = null (ElementHandler.cs:137). Android's _webViewHandler?.VirtualView reads as null-safe but only guards the handler reference.
Verified state transition on Windows (worst case): WebResourceRequested += async (s, eventArgs) => … (SharedSource/WebView2WebViewManager.cs:300) is an async void lambda that is never unsubscribed, and DisconnectHandler (BlazorWebViewHandler.Windows.cs:30-55) only fire-and-forgets DisposeAsync() and nulls its own _webviewManager — the WinUIWebViewManager and its subscription outlive it. Navigate away / close the window while an image or stylesheet is in flight → base VirtualView is already null → InvalidOperationException inside async void → unhandled on the UI thread. Android/iOS raise it on a JNI / ObjC callback thread. Pre-PR, none of these request paths dereferenced VirtualView (confirmed via git show <base>:<file>).
The repo already has the correct pattern for exactly this context: WebRequestInterceptingWebView.TryInterceptResponseStream(IViewHandler? handler, …) reads handler.VirtualView through the explicit-interface (nullable, non-throwing) implementation on all three platforms. Fix: use ((IElementHandler?)handler)?.VirtualView as IBlazorWebView, and ideally short-circuit on provider is null before touching the handler at all.
⚠️ Warning — Provider is invoked for missing resources on iOS/Android/Tizen, so 404 placeholders can be cached
iOS/BlazorWebViewHandler.iOS.cs:299 (status forced to 200 at :329) · Android/WebKitWebViewClient.cs:135 · Tizen/…:139
aspnetcore's TryGetResponseContent returns true for any app-origin URI, emitting a 404 + text/plain placeholder body. iOS rewrites the status to 200 and then stamps the app's Cache-Control on it; Android and Tizen forward it unchanged. Windows deliberately excludes this (WinUIWebViewManager.cs:114-115, statusCode != 404). Following the PR's own usage example with max-age=86400 on image/*, a typo'd or not-yet-shipped asset gets its "not found" placeholder pinned in the WebView cache for a day on three platforms and self-heals only on Windows. BlazorWebViewStaticContentRequest exposes no status code, so the app cannot defend itself. This is maintainer blocking concern #2 and is unresolved.
⚠️ Warning — The tests prove header emission, not caching; the linked bug may remain unfixed on Android/iOS
tests/DeviceTests/Elements/BlazorWebViewTests.StaticContentCaching.cs (all 9 tests)
Every test performs one fetch and asserts the header value echoed back to JS. That passes even if the platform ignores Cache-Control entirely on handler-served responses — the live risk for Android shouldInterceptRequest and iOS WKURLSchemeHandler, where custom responses are widely reported not to enter the WebView HTTP cache (my independent read: moderate confidence they don't enter the disk/HTTP cache; Blink/WebCore's per-document memory cache may still honor it, which is what the #8279 SPA flicker actually depends on — not resolvable from source). The file's own comment at lines 16-19 assumes cross-instance caching happens, and that assumption is load-bearing for the fix yet never verified. The discriminating test is cheap: fetch the same cacheable URL twice and assert the provider ran only once. Prior [major], unresolved.
⚠️ Warning — CR/LF-only sanitization is insufficient for the raw-header platforms
StaticContentCacheControl.cs:53
The value is (correctly) treated as untrusted, but only \r/\n are rejected. Tizen builds a literal wire block — "HTTP/1.0 200 OK\r\n" + "{key}:{value}\r\n" + "\r\n" — and hands it to interceptor.SetResponse (BlazorWebViewHandler.Tizen.cs:145-151); a value containing \0 passes the guard and truncates the marshalled native string, dropping every subsequent header and the terminating blank line. On Windows the same value reaches GetHeaderString → CreateWebResourceResponse, which can throw for an invalid header string — again inside async void. Prefer one RFC 9110 field-value pass (reject < 0x20 except HTAB, and 0x7F) plus a trim; it replaces both Contains scans.
💡 Suggestion — Nothing covers the "no provider set" path, and the fallback assertions don't pin the default
All nine tests assign a provider, so the provider is null early-out — the PR's headline non-breaking guarantee — is never executed. Assert.Contains("no-store", …) also passes for any string containing no-store, so dropping must-revalidate would stay green. Since StaticContentCacheControl.Default is now a single const shared with Windows/StaticContentProvider.cs:30, editing it silently changes every platform with no failing test. Add a provider-less test asserting the exact historical literal, and switch the four fallback assertions to Assert.Equal on the literal (not the const).
💡 Suggestion — WPF/WinForms BlazorWebView diverge
Windows/StaticContentProvider.cs:30 — good de-dup, but WPF/WinForms serve static content through SharedSource/WebView2WebViewManager under #if WEBVIEW2_WINFORMS || WEBVIEW2_WPF and still emit aspnetcore's hardcoded literal with no hook. Consider moving StaticContentCacheControl to src/BlazorWebView/src/SharedSource/, or scoping them out explicitly in the description.
(Not re-raised: the folder-served blazor.modules.json test can't discriminate — prior [moderate] 08-05, still unresolved; and the per-instance-API-vs-shared-cache design question — maintainer blocking concern #1.)
Failure-Mode Probing
- Runs for pages/apps that don't use the feature? Yes, and that is the problem:
handler.VirtualViewis evaluated as an argument on every app-origin request, beforeprovider is nullis ever checked. The feature's blast radius is 100% of BlazorWebView apps, not just adopters. - Handler disconnect/reconnect (navigation, Shell tab switch, window close)? ❌ [Draft] Readme WIP #1. Windows is the sharpest: the
WebResourceRequestedsubscription is never removed andDisposeAsyncis fire-and-forget, so requests genuinely arrive afterVirtualViewis nulled. Android'sWebviewManager != nullguard narrows it to a check-then-use race across threads (ShouldInterceptRequest= background,DisconnectHandler= UI). Even the new device tests can hit it —AttachAndRundisconnects immediately after the fetch. - Null
PlatformView/BindingContext/Parent? Not touched; the provider takes no element state. - Accumulating subscriptions across lifecycle? No new subscriptions; the provider is a plain property read per request, snapshotted into a local first (avoids a TOCTOU tear — good).
- Provider mutated concurrently / re-entrant? Reference read is atomic and the local snapshot means a mid-flight swap can't null-deref. Fine.
- Provider throws or blocks? Throwing is handled (try/catch + EventId 19 log). A blocking provider is not — it stalls the WebView2 deferral and the Android/iOS request threads. The XML docs warn about UI state but not about blocking; worth a doc sentence.
- Stale static state? Only
const string Default; no runtime state survives disposal. - Does the caching actually happen? Un-disproven. Cannot be settled from source; needs an on-device fetch-twice check on Android and iOS.
Verdict: NEEDS_CHANGES
Confidence: low — blast radius is platform handler/request plumbing (caps at medium), then capped again by CI being simultaneously red and pending with no device-test evidence for the new tests on Android/iOS, and by gh being unauthenticated for the required-check query.
Summary: The API shape, default preservation, and PublicAPI bookkeeping are solid, and the author has resolved a long list of earlier review findings. But the new code dereferences the throwing typed VirtualView on four native request-handling paths — unconditionally, for apps that never opt in — where the repo's own WebRequestInterceptingWebView shows the null-safe pattern; on Windows the never-unsubscribed async void handler makes that a real post-disconnect crash, not a theoretical one. On top of that, three maintainer blocking concerns and three prior [major]/[moderate] findings from the 2026-08-05 round remain unaddressed (head commit predates them), and the suite still cannot show that the header changes any actual caching behavior — so the #8279 fix is unproven on exactly the two platforms where it is most doubtful.
Not posted to GitHub (gh unauthenticated; Rule #7 — never --approve/--request-changes regardless). Expert-reviewer inline findings written to CustomAgentLogsTmp/PRState/35706/PRAgent/inline-findings.json (6 entries, valid Review-API shape).
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | Internal Android ETag revalidation with conditional 304 Not Modified responses |
6 files | Built/deployed; candidate and control both hit the same WebView-83 Blazor startup timeout | |
| 2 | try-fix | Internal per-manager bounded memoization of static-content bytes | 5 files | Built/deployed; same startup timeout, and self-review found it may not prevent Chromium decoding/blink | |
| PR | PR #35706 | Public per-request cache-control provider wired into four platform handlers | 18 files | Original PR; gate could not build/run tests |
Candidate Narrative
Candidate 1 — HTTP validator revalidation
Candidate 1 replaces application-selected freshness policy with internal Android strong ETags and matching If-None-Match / 304 Not Modified responses. It is the only alternative that directly gives Chromium a reusable entity without a freshness window, avoids public API and handler VirtualView access, and explicitly excludes 404s. Its candidate and unchanged-control runs both built and deployed, then failed identically before Blazor startup (3 passed / 21 failed / 4 skipped). The intercepted-304 behavior, Android-only scope, and new disk-storage permission remain unverified. Full narrative: ../try-fix-1/content.md.
Candidate 2 — managed asset memoization
Candidate 2 caches encoded response bytes per AndroidWebKitWebViewManager, eliminating repeated managed asset opens without changing HTTP semantics. It also built and deployed, then matched the unchanged control's startup failure shape (3 passed / 20 failed / 4 skipped; one fewer total test due to the candidate test replacement). Expert review found that returning the full no-store response can still make Chromium decode and paint the image again, so even a green memoization assertion would not demonstrate resolution of #8279. Full narrative: ../try-fix-2/content.md.
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
claude-opus-5 |
2 | No | Chromium cannot reuse decoded content while no-store remains; service-worker, DOM-retention, and data/blob rewriting alternatives are not framework-safe |
gpt-5.6-sol |
2 | No | MAUI can affect decoded-image reuse only through cache/revalidation semantics or user-visible DOM mutation; native byte caching does not solve decode/paint |
Exhausted: Yes — the remaining technically credible mechanisms collapse into the already explored cache-header/provider or HTTP-validator families. Native byte caching does not address browser decoding, and browser-content rewriting would alter application semantics.
Selected Fix: None — no candidate passed all tests or was demonstrably better than the PR. Candidate 1 is the strongest conceptual alternative, but its decisive Android cache behavior is unverified; Candidate 2 does not adequately target the visible blink. The PR itself remains unverified by its inconclusive gate and has unresolved code-review findings.
📝 Recommended PR Title & Description
Assessment: ✏️ Recommend updating — the title remains accurate, but the winning reviewer-hardened fix adds lifecycle, missing-response, validation, documentation, and repeat-request test behavior not reflected in the current description.
Recommended title
[BlazorWebView] Add StaticContentCacheControlProvider for static content caching
Recommended description
### Description
`BlazorWebView` serves every asset from the app content root with this response header:
```
Cache-Control: no-cache, max-age=0, must-revalidate, no-store
```
The `no-store` directive prevents the WebView from caching anything, so static images, fonts, and CSS are re-fetched and re-decoded every time a page re-renders them. That is the cause of the image flicker reported in https://github.com/dotnet/maui/issues/8279 when navigating between pages.
This header was introduced with the original BlazorWebView port (#654) and mirrors the upstream behavior. The intent — preserved in the clearer `HybridWebView` sibling comment ("Disable local caching which would otherwise prevent user scripts from executing correctly") — was to ensure user scripts are always re-executed and never served stale from the WebView cache. That concern applies to executable/dynamic content, but the header was applied to **all** responses, including inert static assets.
### What this change does
Adds an **opt-in, additive** hook so applications can choose the `Cache-Control` value per successfully served resource:
```csharp
public Func<BlazorWebViewStaticContentRequest, string?>? StaticContentCacheControlProvider { get; set; }
```
- The default is `null`, so behavior is **unchanged** (`no-store` everywhere). This is intentionally **not** a behavioral change — existing apps are unaffected.
- When set, the callback is invoked for successfully resolved app-origin content with the request `Uri` and resolved `ContentType`. Return a `Cache-Control` value to use, or `null` to keep the default for that request.
- Missing/non-success responses keep the historical default and do not invoke the callback, preventing cacheable not-found placeholders.
- The callback covers everything served from the content root, including the host page and `_framework/*` assets, not only images/fonts/stylesheets. Host pages and executable scripts should remain uncached unless their URLs are versioned.
- The request URI scheme and host are platform-specific (`https`, `app`, or `http` depending on the handler). Portable callbacks should match `Uri.AbsolutePath` and `Uri.Query`.
#### Usage
```csharp
blazorWebView.StaticContentCacheControlProvider = request =>
request.ContentType.StartsWith("image/", StringComparison.Ordinal)
? "max-age=86400"
: null; // null => existing no-store behavior
```
This lets apps stop static images from flickering (the scenario in issue 8279) while keeping framework scripts uncached, and gives full per-resource control — e.g. marking a specific asset `no-store`, or cache-busting via a versioned query string (the handler strips the query only when resolving the file, so `img.png?v=2` is a fresh cache key that still maps to `img.png`).
`null` keeps the default without a diagnostic. Empty/whitespace-only values and values containing invalid HTTP header control characters also fall back to the default and are logged; provider exceptions are caught, logged, and fall back as well. The callback can run on a platform request thread, so it must not access UI state or perform blocking work.
### Why opt-in rather than changing the default
Changing the default to allow caching would be a behavioral breaking change: the WebView cache can be shared by multiple `BlazorWebView` instances and persist across app runs/updates (Android cache dir, iOS `WKWebsiteDataStore`, Windows WebView2 user-data folder), so a bundled asset changed in an app update could be served stale until eviction. Keeping the default and adding an opt-in avoids that while still resolving the issue for apps that want it. Apps opting in should use versioned request URIs for content that can change.
If maintainers prefer to also flip the default (opt-out), this same hook works as the escape hatch.
### Platforms
Implemented for Android, iOS/MacCatalyst, Windows, and Tizen. The override is applied only to a successfully resolved response when the callback returns a valid non-null value; otherwise the historical default header is preserved exactly.
A shared `StaticContentCacheControl` helper holds the default constant, resolves the callback, validates returned header values, and logs rejected values or callback exceptions. Platform handlers read the virtual view through the nullable `IElementHandler` contract so an in-flight request after handler disconnect falls back safely instead of hitting the throwing typed `VirtualView` getter.
### Testing
Adds device tests in `BlazorWebViewTests.StaticContentCaching.cs` that:
- fetch a unique cacheable URL twice and assert the target provider is invoked only once, distinguishing actual WebView cache reuse from header emission,
- verify a provider-less request preserves the exact historical default,
- verify `null`, empty, whitespace-only, invalid-control-character, and throwing providers were invoked and then preserve the exact default,
- verify the callback receives the resolved content type and original query string, including the Windows folder-serving path.
The original PR was built across the `net`, Android, iOS, and Windows library targets (including the PublicAPI analyzer). The reviewer-hardened Android library and Android BlazorWebView device-test project also compile with zero warnings and errors.
Runtime device verification remains inconclusive in the available environment: the supplied Android gate exited with an app/environment crash before producing a real pass/fail, and the alternative-candidate control runs could not start Blazor under the installed Android System WebView. The repeated-request assertion still needs to run on a current Android WebView (and ideally the other supported platform engines) before the caching behavior is considered verified.
### Alternatives evaluated
- An internal Android ETag/`304 Not Modified` implementation avoids public API, but is Android-only, changes `no-store` persistence semantics, and could not verify that a synthesized `304` from `ShouldInterceptRequest` is accepted by Chromium.
- Managed per-manager byte memoization avoids a second `IFileProvider` open, but still returns a complete body with `no-store`, so it does not directly prevent browser re-decode/repaint and does not prove the image flicker is fixed.
### Related
Addresses https://github.com/dotnet/maui/issues/8279 by providing an opt-in mechanism (does not change default behavior).
🏁 Report — Final Recommendation
Comparative Fix Report — PR #35706
Decision
Winner: pr-plus-reviewer
It retains the PR's cross-platform, opt-in cache policy while removing the raw PR's concrete handler-lifecycle and missing-response defects, applying every expert-review finding, and adding the first regression assertion that can distinguish a real cache reuse from mere header emission.
Regression evidence
No candidate has a passing runtime regression result, so no candidate receives a test-pass preference:
| Candidate | Runtime result | Classification |
|---|---|---|
pr |
Gate ended in Android APP_CRASH; without-fix build was also invalid because the baseline process left PR-added files behind |
|
pr-plus-reviewer |
Runtime test deliberately not rerun; Android library and device-test project compile cleanly | |
try-fix-1 |
3 passed / 21 failed / 4 skipped; unchanged control produced the identical Blazor-startup timeout set | |
try-fix-2 |
3 passed / 20 failed / 4 skipped; common failures matched the unchanged control before candidate assertions |
The required ordering rule is therefore satisfied without demoting any candidate for a candidate-caused regression failure: none passed the runtime regression test, and none reached a candidate assertion that failed.
Ranking
| Rank | Candidate | Assessment |
|---|---|---|
| 1 | pr-plus-reviewer |
Best coverage and safety. Cross-platform, opt-in, default-preserving, disconnect-safe, excludes missing responses, validates/logs unsafe values, and contains a discriminating repeat-request test. Runtime caching remains unverified. |
| 2 | try-fix-1 |
Strongest alternative design. Android ETag/304 Not Modified revalidation avoids public API, scopes validators per manager, and excludes 404s. It is Android-only, changes storage/privacy semantics from no-store to no-cache, and its central synthesized-304 behavior never ran. |
| 3 | pr |
Plausible cross-platform mechanism with a carefully preserved default, but it has a concrete post-disconnect VirtualView exception path, can cache missing-content placeholders on three platforms, under-validates header values, and lacks proof of cache reuse. |
| 4 | try-fix-2 |
Bounded per-manager byte memoization is internally scoped and buildable, but it still returns a full no-store response. It can reduce managed file opens without preventing Chromium/WebKit from reprocessing and decoding the image, so its test does not establish a fix for #8279. |
Candidate comparison
pr
The public callback gives applications precise per-resource policy and preserves existing behavior when unset. It also covers all MAUI platforms in one design. However, its platform wiring widened the request-path blast radius to every BlazorWebView instance and introduced a concrete throwing getter after disconnect. Its tests verify headers rather than the reported navigation/cache behavior.
pr-plus-reviewer
This candidate preserves the useful API and cross-platform implementation while:
- making handler access null-safe after disconnect,
- preventing overrides on missing content,
- documenting platform-specific request URI and full callback scope,
- logging rejected values and validating native header input,
- strengthening fallback tests, and
- requiring actual reuse for a repeated cacheable request.
The remaining uncertainty is empirical rather than hidden: if intercepted responses are not cacheable on Android or another platform, the new regression test should fail and force the design back to discussion instead of allowing a false-green header test.
try-fix-1
The ETag approach is architecturally attractive for Android because it avoids a new public contract and forces freshness through revalidation. It also handles 404s and handler disposal more safely than the raw PR. It ranks below pr-plus-reviewer because it fixes only Android, permits disk persistence that no-store prohibited, and depends on an unverified 304 returned from ShouldInterceptRequest.
try-fix-2
The managed memory cache has sensible bounds and lifecycle scope, but it targets the wrong layer. Avoiding a second IFileProvider open does not let the browser reuse its decoded image while no-store remains, so it is not a credible end-to-end fix for the reported flicker.
Final rationale
pr-plus-reviewer wins because it has the broadest intended behavior, the fewest known correctness defects, and the strongest regression oracle. It is the best candidate to advance, but the winner remains unverified until the repeated-request test executes on a current Android WebView (and ideally the other supported platform engines).
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
kubaflo
left a comment
kubaflo
left a comment
There was a problem hiding this comment.
Could you please check the ai's suggestions?
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9083ee52-6f8d-4298-8307-8813c24d0aed
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9083ee52-6f8d-4298-8307-8813c24d0aed
Add bounded managed response caching for intercepted static content on Android, Apple, Windows, and Tizen, with request-policy handling and expanded device coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9083ee52-6f8d-4298-8307-8813c24d0aed
Preserve the original PR history as a second parent while keeping the independently rebuilt and validated implementation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9083ee52-6f8d-4298-8307-8813c24d0aed
Empirical Android validationI reproduced the original reload cost under decoded-image-cache pressure, then reran the identical scenario with the bounded managed response cache. Before — uncached reload (~510 ms) pr35706-before.mp4
After — cached reload (~11 ms) pr35706-after.mp4
The regression test retains 64 unique 800×500 canvas PNGs to force realistic decoded-image pressure. It also counts actual |
|
/review rerun |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
Final validation and CI dispositionValidated commit
No product change is warranted for those UI-pipeline failures. The implementation-specific local suites, full PR build, and device-test pipeline all pass. Before/after recordings, screenshots, and measured reload timings are in the empirical validation comment. |
Resolve PublicAPI.Unshipped.txt conflicts by retaining both the static content caching API and the new platform handler API entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9083ee52-6f8d-4298-8307-8813c24d0aed
kubaflo
left a comment
kubaflo
left a comment
There was a problem hiding this comment.
Approved, but it will go to net11 not the next SR release
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/BlazorWebView/src/Maui/BlazorWebView.cs:78
- The XML docs mention that returning null/empty keeps the default behavior, but the implementation also treats whitespace-only values and values containing CR/LF as invalid and falls back to the default. Documenting these cases helps avoid confusing “cache enabled but ignored” situations and makes the documented contract match the runtime behavior.
/// Return <see langword="null"/> or an empty string from the callback to keep the default behavior for a
/// given request. Cache entries remain subject to platform limits, expiration, and eviction.
src/BlazorWebView/src/Maui/StaticContentCacheControl.cs:41
- When the provider throws, the error log currently includes the full request URI as provided by the platform. Because the provider is explicitly given the original URI (including query string), this log can inadvertently include query parameters (which may contain sensitive data). Consider logging the URI without query/fragment (e.g., path-only) for consistency with other request logs and to reduce accidental data exposure.
// The provider is arbitrary application code invoked from the native request-handling path. On Windows
// it runs inside an async void handler, where an escaped exception would also skip deferral.Complete()
// and hang the request. A faulty provider must not take down static asset serving, so keep the default.
logger?.StaticContentCacheControlProviderFailed(requestUri, ex);
return null;
src/BlazorWebView/src/Maui/IBlazorWebView.cs:38
- The XML docs describe null/empty as falling back to the default Cache-Control, but the implementation also falls back for whitespace-only values and values containing CR/LF. Updating the docs to reflect these additional fallback cases keeps the public contract accurate.
/// served by the <see cref="BlazorWebView"/>, or <see langword="null"/> to use the default (which disables
/// caching). Returning <see langword="null"/> or an empty string from the callback for a given request also
/// falls back to the default for that request. The callback may be invoked on a background thread, so it
/// must not access UI state directly. If the callback throws, the exception is logged and the request falls
/// back to the default header. Cache entries remain subject to platform limits, expiration, and eviction.
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 of Change
BlazorWebViewhistorically serves local static assets with:Cache-Control: no-cache, max-age=0, must-revalidate, no-storeThat safe default ensures scripts are not reused unexpectedly, but it also forces images, fonts, and stylesheets to be read and decoded again. This contributes to the image flicker reported in #8279.
This change adds an opt-in callback:
Returning
null, an empty value, or whitespace preserves the historicalno-storebehavior, so existing applications are unchanged.Implementation
Synthetic responses returned by Android
ShouldInterceptRequest, Apple'sWKURLSchemeHandler, WebView2 interception, and Tizen's request interceptor cannot be assumed to enter the browser HTTP cache merely because they carry cache headers. The implementation therefore adds a bounded, per-WebView managed response cache on Android, iOS/MacCatalyst, Windows, and Tizen:GETresponses with a positivemax-ageRange,Authorization,no-store, non-GET, and other non-cacheable requestsThe callback receives the original absolute URI and resolved content type. Exceptions and malformed/header-injection values fall back to the existing default and are logged.
What NOT to Do
Cache-Controlheaders alone for synthetic/intercepted WebView responses; Android and Apple validation proved those responses were read again without the managed cache.IFileInfo.CreateReadStream()calls.GET, or responseno-store/no-cachetraffic.Validation
The device coverage now verifies the served header and actual file reads, plus repeated requests, expiration,
no-store,no-cache, request refresh, refresh-to-no-store, query strings, content types, invalid callback values, callback failures, and request-header precedence.net10.0library buildMakePri.exeandXamlCompiler.execannot run on macOSThe Android decoded-image-pressure regression test retains 64 unique canvas images, evicts the decoded image, and proves the second load comes from the managed response cache. In the captured run, the uncached reload took approximately 510 ms and the cached reload approximately 11 ms.
Issues Fixed
Addresses #8279 without changing default caching behavior.