[BlazorWebView] Add StaticContentCacheControlProvider for static content caching - #35706
[BlazorWebView] Add StaticContentCacheControlProvider for static content caching#35706Kebechet wants to merge 12 commits into
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. |
|
/review -b feature/enhanced-reviewer -p android |
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 |
|
/review -b feature/enhanced-reviewer -p android |
This comment has been minimized.
This comment has been minimized.
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
kubaflo
left a comment
There was a problem hiding this comment.
Can we avoid new public methods and make them internal?
|
Thanks for the review! Happy to reduce the surface — could you point me at which member you'd like changed? To make sure we're looking at the same thing, the PR doesn't add any new public methods. The complete new public surface is: The resolver that does the actual work ( The two properties are the feature — they're how an app opts a resource into caching instead of the historical
var header = MyCachePolicy.Resolve(new BlazorWebViewStaticContentRequest(uri, "image/png"));With an internal constructor that policy can only be exercised through a live |
…ent caching BlazorWebView serves all static content with `Cache-Control: no-cache, max-age=0, must-revalidate, no-store`, which disables WebView caching and causes static images to flicker when navigating between pages (issue 8279). Add an opt-in `BlazorWebView.StaticContentCacheControlProvider` callback that lets apps choose the Cache-Control value per resource. The default is null, preserving the existing no-store behavior, so the change is additive and non-breaking. Wired for Android, iOS/MacCatalyst, and Windows; a shared StaticContentCacheControl helper holds the default and resolution logic.
Resolve the Android gate build failure in the new device tests (CA1307 on Assert.Contains and CS8632 nullable annotations in a nullable-disabled project) and wire StaticContentCacheControlProvider into the Tizen request handler so the public API is no longer a no-op there.
…lProvider The provider received a query-stripped URI on Android, Windows, and Tizen while iOS passed the original URI, so apps could not make cache decisions based on versioned/cache-busting query strings (e.g. img.png?v=2) on those platforms. Capture the original URI before query stripping and pass it to the provider, keeping the stripped URI only for file resolution. Also fix a device-test harness issue where bare string results were left double-encoded (a stray layer of quotes), which made the cache-control override assertion fail on-device, and correct doc wording (struct->class, "in to"->"into"). Adds a device test asserting the provider receives the query string. Verified green on an Android emulator (26 passed / 0 failed).
… path The folder-serving path (WinUIWebViewManager.TryServeFromFolderAsync) passed the query-stripped requestUri to the StaticContentCacheControlProvider, so apps could not make cache-busting decisions based on query strings (e.g. img.png?v=2) for content served from that path. The framework-file branch and the other platforms already pass the original URI. Thread the original unstripped URI into TryServeFromFolderAsync and use it for the cache-control provider call, keeping the stripped URI for file resolution, hot reload, and logging. Add a device test that exercises the WinUI folder path via _framework/blazor.modules.json with a query string. Verified red/green on a real WinUI desktop run.
Address expert review feedback on the cache-control provider: - Guard URI parsing in StaticContentCacheControl.ResolveOverride, which now takes the raw request string and uses Uri.TryCreate instead of `new Uri`. The request handlers run on background threads, so a malformed URI now falls back to the default header rather than throwing. Removes the unguarded `new Uri(...)` from the iOS/Android/Windows/Tizen call sites. - Read Content-Type defensively on Tizen via TryGetValue, matching the Windows path, so a missing header no longer throws KeyNotFoundException. Verified on Android (emulator, API 35) and Windows (unpackaged): all five StaticContentCacheControlProvider device tests pass on both platforms.
…tions The provider callback 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, so a faulty provider could crash or hang static asset serving instead of falling back to the default header. Wrap the provider invocation in StaticContentCacheControl.ResolveOverride in a try/catch that logs and returns null, threading an ILogger through the Android, iOS, Windows and Tizen call sites. Add a StaticContentCacheControlProviderFailed log message, document the fallback on the public API, and add a device test that asserts a throwing provider still serves the default no-store header.
…ed cache The override device test served a fixed URL with max-age=3600 and asserted only the returned header. Because the WebView HTTP cache is shared for the app origin and persists across runs on a device, a rerun within the max-age window could be satisfied from a previous run's cached response without invoking the provider, so a regression that stopped calling the provider could still pass. Add a per-run nonce to the fetch URL so it is always a cache miss, and assert the provider was actually invoked for the requested resource.
…Windows header helper The default Cache-Control string was written both as StaticContentCacheControl.Default and as a literal in the Windows StaticContentProvider.GetResponseHeaders. Reference the constant from the header helper so the default lives in one place and the two cannot drift apart.
…efault ResolveOverride collapsed only null and empty provider return values to the default header, so a whitespace-only value such as " " was served verbatim as a non-standard, meaningless Cache-Control header. Use string.IsNullOrWhiteSpace so whitespace-only values fall back to the default too, and add a device test.
This comment has been minimized.
This comment has been minimized.
AI Review Summary
🗂️ Review Sessions — click to expand🚦 Gate — Test Before & After FixGate Result: ✅ PASSEDPlatform: WINDOWS · Base: inflight/current · Merge base: ✅ Verified (new API / feature) — this PR adds new API and a test that references it in the same project, so reverting the fix un-compiles the test: there is no valid "fails without the fix" baseline to establish (a compile-coupled baseline). The gate instead verified the fix by a clean build + pass with the fix, so this is a real PASS rather than a non-committal INCONCLUSIVE.
🔴 Without fix — 📱 BlazorWebViewTests (StaticContentCacheControlProviderCanOverrideCacheControlHeader, StaticContentCacheControlProviderReturningNullKeepsDefaultNoStore, StaticContentCacheControlProviderReturningEmptyStringKeepsDefaultNoStore, StaticContentCacheControlProviderReturningWhitespaceKeepsDefaultNoStore, StaticContentCacheControlProviderReturningValueWithNewlinesKeepsDefaultNoStore, StaticContentCacheControlProviderThrowingKeepsDefaultNoStore, StaticContentCacheControlProviderReceivesResolvedContentType, StaticContentCacheControlProviderReceivesQueryString, StaticContentCacheControlProviderReceivesQueryStringForFolderServedContent): 🛠️ BUILD ERROR · 286sError-relevant lines (filtered from the build log): 🟢 With fix — 📱 BlazorWebViewTests (StaticContentCacheControlProviderCanOverrideCacheControlHeader, StaticContentCacheControlProviderReturningNullKeepsDefaultNoStore, StaticContentCacheControlProviderReturningEmptyStringKeepsDefaultNoStore, StaticContentCacheControlProviderReturningWhitespaceKeepsDefaultNoStore, StaticContentCacheControlProviderReturningValueWithNewlinesKeepsDefaultNoStore, StaticContentCacheControlProviderThrowingKeepsDefaultNoStore, StaticContentCacheControlProviderReceivesResolvedContentType, StaticContentCacheControlProviderReceivesQueryString, StaticContentCacheControlProviderReceivesQueryStringForFolderServedContent): PASS ✅ · 146s
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35706 | Add public opt-in provider, shared resolver/validation, and per-platform header override call sites | ✅ PASSED (Gate) | src/BlazorWebView/src/Maui/*, platform handlers, PublicAPI, device tests |
Original PR; gate result provided by caller. |
🔬 Code Review — Deep Analysis
Code Review — PR #35706
Independent Assessment
What this changes: Adds an opt-in BlazorWebView.StaticContentCacheControlProvider API so apps can override Cache-Control for served static assets per request. The implementation applies the override across Android, iOS/MacCatalyst, Windows, and Tizen, while preserving the historical no-store default.
Inferred motivation: Avoid BlazorWebView static asset flicker/re-fetching by allowing selected assets (for example images) to be cacheable without changing default behavior.
Reconciliation with PR Narrative
Author claims: The PR fixes #8279 by adding an opt-in per-resource cache-control hook; default remains unchanged; provider receives URI/content type and is guarded for null/empty/invalid/throwing cases.
Agreement/disagreement: Matches the code. The current implementation preserves defaults, passes original query-bearing URIs, rejects unsafe CR/LF values, catches provider exceptions, and adds device tests.
Prior Review Reconciliation
| Prior ❌ Error Finding | Source | Status | Evidence |
|---|---|---|---|
| Query string stripped before provider | MauiBot inline | ✅ Fixed | Android/iOS/Tizen/WinUI now preserve original request URI before stripping. |
| WinUI folder-serving path used stripped URI | MauiBot inline | ✅ Fixed | TryServeFromFolderAsync receives originalRequestUri. |
Tizen Content-Type indexer could throw |
MauiBot inline | ✅ Fixed | Tizen now uses TryGetValue. |
| Provider exception could crash/hang request path | MauiBot inline | ✅ Fixed | StaticContentCacheControl.ResolveOverride catches exceptions and logs. |
| Cacheable test URL could be satisfied from stale cache | MauiBot inline | ✅ Fixed | Override test uses per-run nonce and asserts provider invocation. |
| Earlier gate/build ❌ reports | MauiBot reviews | 🔄 Obsolete | They refer to older commits; current head includes subsequent fixes. Current CI is still red separately. |
Blast Radius Assessment
- Runs for all instances: No behavioral change unless
StaticContentCacheControlProvideris set; default provider is null. - Startup impact: No new startup work; provider is read only during static resource request handling.
- Static/shared state: No new shared mutable state. Existing default constant only.
External Output Contract
| Consumer token/pattern | Producer location | Producer emission condition | Consumer assumption | Ordinary negative case | Downstream effect |
|---|---|---|---|---|---|
| None | N/A | N/A | N/A | N/A | N/A |
CI Status
- Required-check result:
gh pr checks --requiredunavailable becauseghis unauthenticated. Public REST fallback shows current head checkmaui-prcompleted failure; 16 failed check runs. - Classification: Undetermined from available public REST summary; failed AzDO build
1535989has multiple failed timeline records. - Action taken: Invoked
azdo-build-investigatorcontext; confidence capped low. Per skill rules, no LGTM with red/undetermined CI.
Findings
No actionable code findings from my review or the expert reviewer.
Failure-Mode Probing
- Provider unset/default:
ResolveOverridereturns null and existing no-store header remains. - Provider throws: exception is caught, logged, and default header is used; Windows deferral completion is not skipped.
- Provider returns empty/whitespace/newline: value is rejected and default header is used, avoiding malformed/header-injection cases.
- Query-string cache busting: original URI is preserved for provider decisions, while stripped URI is still used for file resolution.
- Handler disconnect/reconnect: no new subscriptions or static state; request paths use existing handler/view references.
Verdict: NEEDS_DISCUSSION
Confidence: low
Summary: Code review found no concrete implementation defects, and prior inline findings appear addressed. However, current CI is red and could not be fully classified with the available unauthenticated tooling, so the skill rules prohibit LGTM until CI is green or failures are confirmed unrelated.
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix-1 | Headers-at-construction: move cache-control resolution into StaticContentProvider.GetResponseHeaders overload instead of using the PR's separate resolver/mutate-after-build pattern |
7 implementation/API files + PublicAPI | Compiled cleanly; not demonstrably better than PR because device validation blocked and design is more Windows-centric. | |
| 2 | try-fix-2 | Default interface method: let IBlazorWebView mutate response headers via ApplyStaticContentResponseHeaders instead of platform code calling a shared resolver |
18 files | Viable but leaks ILogger? into public interface surface; not better than PR. |
|
| 3 | try-fix-3 | DI-first cache policy: resolve an app-registered Func<BlazorWebViewStaticContentRequest,string?> service before falling back to the PR-style per-control provider |
❌ FAIL (script-compatible run failed before test execution; original -Project path was rejected by runner ValidateSet, then baseline build errors blocked) |
5 files | Minimal but not validated; service-first does not replace the tested public property acceptance boundary. |
| 4 | try-fix-4 | BindableProperty provider: store StaticContentCacheControlProvider as a MAUI BindableProperty to preserve property name while avoiding interface expansion |
BlazorWebView/API/platform/PublicAPI files | Clean self-review; viable, but adds bindable-property machinery without clear benefit over PR's simple CLR property. | |
| 5 | try-fix-5 | Zero-public-API heuristic: auto-cache fingerprinted _framework/*/_content/* assets and keep no-store for host/runtime/user assets |
❌ FAIL (compile-time acceptance-boundary failure) | 4 files plus deleted request DTO | Breaks BlazorWebViewStaticContentRequest/provider tests; useful only as a possible default layered under the PR API. |
| PR | PR #35706 | Add public opt-in provider, shared resolver/validation, and per-platform header override call sites | ✅ PASSED (Gate) | 18 files | Original PR; gate already completed. |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | Yes | Internal per-WebView URL namespace / instance-id routing for policy lookup. Not run: high-risk routing change and does not improve on PR for this issue. |
| claude-opus-4.7 | 2 | Yes | Zero-API heuristic for fingerprinted Blazor assets. Run as candidate 5; failed because regression tests lock the public API shape. |
| gpt-5.3-codex | 2 | Yes | Change the global default Cache-Control to allow caching. Not run: too broad and intentionally changes existing default behavior for all assets. |
| gpt-5.5 | 2 | Yes | IStaticContentHeadersProvider/metadata on IFileProvider. Not run: larger provider-contract API with no advantage over PR callback for the tested scenario. |
Candidate Narratives
try-fix-1 — Headers-at-construction
- Approach: Embed cache-control resolution into Windows
StaticContentProvider.GetResponseHeaders/header construction instead of PR's separateStaticContentCacheControl.ResolveOverridehelper. - Test: Build completed, but Windows BlazorWebView device test execution could not produce results in the headless session.
- Failure analysis: Environment blocked runtime validation. Design is viable but more Windows/header-factory centered and not demonstrably better than the PR.
- Diff: Full diff in
try-fix/attempt-1/fix.diff; narrative intry-fix-1/content.md.
try-fix-2 — Default interface method header mutation
- Approach: Move validation and header mutation into a default interface method on
IBlazorWebViewso platforms delegate to the view rather than a static resolver. - Test: Build completed, then hit the same Windows headless device-test result collection failure.
- Failure analysis: Self-review found one moderate API smell:
ILogger?leaks into a public interface method. This is worse public API design than PR's internal resolver. - Diff: Full diff in
try-fix/attempt-2/fix.diff; narrative intry-fix-2/content.md.
try-fix-3 — DI-first cache policy
- Approach: Resolve a cache-control policy delegate from handler services first, then fall back to the per-control provider.
- Test: Initial requested command used an invalid
-Projectvalue for the runner; script-compatible retry failed before execution on unrelated baseline build errors. - Failure analysis: DI can layer on top of the PR surface but does not replace the property that the regression tests exercise.
- Diff: Full diff in
try-fix/attempt-3/fix.diff; narrative intry-fix-3/content.md.
try-fix-4 — BindableProperty provider
- Approach: Keep
StaticContentCacheControlProviderpublic property name/signature, but back it with a MAUIBindablePropertyinstead of a CLR field/property. - Test: Build completed, then hit the same Windows headless device-test result collection failure.
- Failure analysis: Clean self-review and viable, but it adds bindable-property overhead without a demonstrated need for binding/XAML support; PR's simple CLR property is clearer.
- Diff: Full diff in
try-fix/attempt-4/fix.diff; narrative intry-fix-4/content.md.
try-fix-5 — Zero-public-API heuristic
- Approach: Remove the public provider/request DTO and automatically cache clearly fingerprinted Blazor assets while preserving no-store for host/runtime/user assets.
- Test: Failed at compile time because existing regression tests reference
BlazorWebViewStaticContentRequestandStaticContentCacheControlProvider. - Failure analysis: Confirms the regression suite is a hard acceptance boundary for PR [BlazorWebView] Add StaticContentCacheControlProvider for static content caching #35706's public API shape. The heuristic might be useful only as an additional default under the provider API, not as a replacement.
- Diff: Full diff in
try-fix/attempt-5/fix.diff; narrative intry-fix-5/content.md.
Exhausted: Yes — remaining cross-pollination ideas either fail the tested public API contract, broaden caching defaults too much, or introduce larger/riskier API/routing surfaces than the PR.
Selected Fix: PR #35706 — It is the only candidate with a completed passing gate signal, preserves default behavior, keeps customization explicit and per control, and avoids the API/design drawbacks found in alternatives.
📝 Recommended PR Title & Description
Assessment: ✏️ Recommend updating — the current title is accurate, but the winning pr-plus-reviewer fix adds a successful-response-only guard that the description should mention so it does not imply cache overrides apply to missing/404 responses.
Recommended title
[BlazorWebView] Add StaticContentCacheControlProvider for static content caching
Recommended description
### Description
`BlazorWebView` serves every static 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 static resource:
```csharp
public Func<BlazorWebViewStaticContentRequest, string?>? StaticContentCacheControlProvider { get; set; }
- The default is
null, so behavior is unchanged (no-storeeverywhere). This is intentionally not a behavioral change — existing apps are unaffected. - When set, the callback is invoked for each successfully served static asset with the request
Uriand resolvedContentType. Return aCache-Controlvalue to use, ornullto keep the default for that request. - Missing/404 static-content responses keep the default header instead of receiving app-provided cacheable headers, avoiding long-lived cached misses and keeping Android, iOS/MacCatalyst, Windows, and Tizen aligned.
Usage
blazorWebView.StaticContentCacheControlProvider = request =>
request.ContentType.StartsWith("image/", StringComparison.Ordinal)
? "max-age=86400"
: null; // null => existing no-store behaviorThis 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 already strips the query before resolving the file, so img.png?v=2 is a fresh cache key that still maps to img.png).
Why opt-in rather than changing the default
Changing the default to allow caching would be a behavioral breaking change: the WebView cache persists across app updates on all three platforms (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. 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 for successful static-content responses when the callback returns a non-null valid value; otherwise the historical default header is preserved exactly. A shared StaticContentCacheControl helper holds the default constant and the resolution logic, and is wired into the request handling of all four platform handlers.
Testing
Adds device tests in BlazorWebViewTests.StaticContentCaching.cs:
- the override is applied to the served
Cache-Controlheader, - returning
nullpreserves the defaultno-store(the non-breaking guarantee), - empty, whitespace-only, and CR/LF-containing values preserve the default,
- a throwing provider preserves the default instead of breaking static asset serving,
- the callback receives the resolved content type,
- the callback receives the original request URI including query strings, including the Windows folder-serving path.
I was not able to execute the device tests locally (they require the Android/iOS/Windows device-test runners). I verified the change with dotnet build of the library across the net, Android, iOS, and Windows targets (including the PublicAPI analyzer) and by review; the device-test runs in CI would be the real validation.
Related
Addresses #8279 by providing an opt-in mechanism (does not change default behavior).
</details>
---
<details>
<summary><strong>🏁 Report — Final Recommendation</strong></summary>
<br/>
## Comparative Report — PR #35706
### Candidates Compared
| Candidate | Result | Strengths | Weaknesses | Rank |
|---|---|---|---|---:|
| `pr-plus-reviewer` | Not separately gate-run; derived from gate-passing PR plus sandboxed expert fixes | Keeps the PR's public API, preserves default no-store behavior, keeps query/content-type/exception validation, and fixes the Android/Tizen 404-cache issue found by expert review. | Needs the small sandbox guard applied to the actual PR branch and should ideally gain a missing-asset regression test. | 1 |
| `pr` | ✅ Gate PASSED | Only candidate with completed passing gate signal; public API and cross-platform wiring match acceptance tests; broad tests cover override, fallback, invalid values, exceptions, content type, and query strings. | Expert review found Android/Tizen apply provider overrides to 404 responses, which can cache missing assets and differs from Windows/iOS. | 2 |
| `try-fix-1` | ⚠️ BLOCKED after build success | Preserves API and compiles; simplifies header construction by centralizing more logic in `StaticContentProvider`. | Windows device execution blocked; more Windows/header-factory centric and not demonstrably better than PR. | 3 |
| `try-fix-4` | ⚠️ BLOCKED after build success | Preserves property signature while avoiding `IBlazorWebView` expansion; self-review clean. | Adds bindable-property machinery without demonstrated need; validation blocked; diff also included unrelated-looking `HybridWebView.js` churn in the recorded attempt. | 4 |
| `try-fix-2` | ⚠️ BLOCKED after build success | Centralizes mutation/validation in a default interface method. | Leaks `ILogger?` into public interface API; adds allocation on iOS; validation blocked. | 5 |
| `try-fix-3` | ❌ FAIL / unvalidated | DI-first policy could layer on top of PR's property. | Run failed before test execution and baseline build errors blocked validation; DI service does not replace the tested per-control API boundary. Failed candidates rank below passed/blocked viable candidates. | 6 |
| `try-fix-5` | ❌ FAIL | Explores a zero-public-API automatic caching heuristic. | Breaks the public API/test acceptance boundary by removing `StaticContentCacheControlProvider`/`BlazorWebViewStaticContentRequest`; only Windows wired; heuristic risks immutable-caching future framework exceptions. | 7 |
### Winning Candidate
`pr-plus-reviewer` wins.
The raw PR is the best validated foundation: it passed the gate and supplies the additive opt-in API required by the tests. The expert reviewer found one concrete correctness gap in that foundation: Android and Tizen can apply cacheable headers to 404 static-content responses, unlike Windows/iOS. `pr-plus-reviewer` preserves every positive property of the PR while applying the smallest targeted fix: only resolve/apply the provider when the handled static-content response is successful (`statusCode == 200`) on Android and Tizen.
### Why the Try-Fix Candidates Do Not Win
Candidates that failed regression tests are ranked below candidates that passed them. `try-fix-3` and `try-fix-5` therefore cannot outrank the PR-based candidates. The blocked candidates (`try-fix-1`, `try-fix-2`, `try-fix-4`) produced useful design alternatives, but none improves on the PR enough to offset weaker validation or API/design drawbacks. In particular, `try-fix-2` exposes logging in a public interface, `try-fix-4` adds bindable-property complexity without demonstrated need, and `try-fix-1` mainly relocates resolver logic rather than addressing a behavior gap.
### Required Follow-Up for Winner
Apply the sandbox `pr-plus-reviewer` guard to the actual PR branch:
```csharp
var cacheControlOverride = statusCode == 200
? StaticContentCacheControl.ResolveOverride(...)
: null;
This should be applied in:
src/BlazorWebView/src/Maui/Android/WebKitWebViewClient.cssrc/BlazorWebView/src/Maui/Tizen/BlazorWebViewHandler.Tizen.cs
A follow-up missing-asset regression test would further lock the behavior, but the winning fix itself is the PR plus this reviewer feedback.
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
Description
BlazorWebViewserves every static asset from the app content root with this response header:The
no-storedirective 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 #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
HybridWebViewsibling 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-Controlvalue per resource:null, so behavior is unchanged (no-storeeverywhere). This is intentionally not a behavioral change — existing apps are unaffected.Uriand resolvedContentType. Return aCache-Controlvalue to use, ornullto keep the default for that request.Usage
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 already strips the query before resolving the file, soimg.png?v=2is a fresh cache key that still maps toimg.png).Why opt-in rather than changing the default
Changing the default to allow caching would be a behavioral breaking change: the WebView cache persists across app updates on all three platforms (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. 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 when the callback returns a non-null value; otherwise the historical default header is preserved exactly. A shared
StaticContentCacheControlhelper holds the default constant and the resolution logic, and is wired into the request handling of all four platform handlers.Testing
Adds device tests in
BlazorWebViewTests.StaticContentCaching.cs:Cache-Controlheader,nullpreserves the defaultno-store(the non-breaking guarantee),I was not able to execute the device tests locally (they require the Android/iOS/Windows device-test runners). I verified the change with
dotnet buildof the library across thenet, Android, iOS, and Windows targets (including the PublicAPI analyzer) and by review; the device-test runs in CI would be the real validation.Related
Addresses #8279 by providing an opt-in mechanism (does not change default behavior).