Repository navigation
Add generic DevFlow WebView package with layered Blazor support - #651
mattleibow wants to merge 1 commit into
Conversation
Extract shared CDP tooling for WebView and HybridWebView, preserve Blazor-specific integration, and fix context selection, exact-once actions, error reporting, and screenshots. Add maintained mixed-host samples, regression coverage, and protocol documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 606dd4d0-f861-42fc-8722-8b7df1a81773
Expert Code Review — PR #651Methodology: 2 independent reviewers (1 failed — reduced coverage, no tiebreaker) with adversarial consensus. Because this 63-file PR required batch splitting, batch-local findings were included at downgraded severity and marked low confidence. Results: 2 findings posted as inline comments (0 critical, 0 moderate, 2 minor). CI status: 56 checks reported; most completed checks are successful or skipped, several builds remain in progress, and Test coverage: The PR adds or updates targeted CLI, agent integration, client, registry/action, and WebView debug-service tests for the changed behavior. Runtime parity is not claimed for Windows, GTK, or WPF in the PR description.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Expert Code Review: 2 low-confidence batch-split findings posted inline. See the lean summary comment for methodology, CI state, and test coverage.
Generated by Expert Code Review (auto) for #651 · copilot · gpt56 · 178.9 AIC · ⌖ 12.4 AIC · ⊞ 37.6K
| { | ||
| ObjectDisposedException.ThrowIf(_disposed, this); | ||
| var bridge = new WebViewBridge(this, evalJs, reload, navigate, automationId, deactivated, hostKind, owner); | ||
| _bridges.Add(bridge); |
There was a problem hiding this comment.
🟢 MINOR — low confidence, single reviewer (batch split)
Each handler attachment appends a bridge, but deactivation never removes or reuses it. Repeated page/WebView recreation therefore leaves the singleton service retaining all historical bridges and their synchronization/lifetime objects, while Initialize() and Bridges continue traversing them. Please retire/dispose inactive bridges or reuse stable slots, with a repeated attach/detach test that verifies bounded storage.
| return bridge?.InitializeAndMonitorAsync() ?? Task.CompletedTask; | ||
| } | ||
|
|
||
| public Task<string> SendCdpCommandAsync(string cdpJson) => SendCdpCommandAsync(0, cdpJson); |
There was a problem hiding this comment.
🟢 MINOR — low confidence, single reviewer (batch split)
SendCdpCommandAsync(string) always targets bridge index 0. After a WebView handler is replaced, the old bridge is deactivated and the replacement is appended, so this compatibility overload can keep sending to the inactive bridge and return “WebView is no longer available” while an active replacement exists. Please resolve the current active/ready bridge (or track an explicit default) and cover detach/reattach through this overload.
|
Superseded by the verified native stack: #652 corrects the existing protocol specs without changing runtime, and #655 carries the generic WebView implementation, additive Blazor adapter, clean context contract, tests and documentation. All intended feature changes were carried forward; compatibility shims and aliases were deliberately removed under the lockstep agent/CLI/bridge upgrade decision. Closing this original PR without merging; review and merge the replacement stack instead. |
1 participant
Summary
Microsoft.Maui.DevFlow.WebViewfor standard MAUI WebView and HybridWebView, sharing one embedded CDP engine without ASP.NET/Razor dependencies.Schema rationale
readyandisReadywere already both emitted by the agent. The schema now documents the existing compatibility alias rather than inventing a second readiness state.index,automationId, andactivewere likewise existing response fields absent from the schema.urlalready could be null; its schema now permits null. These corrections are optional-field documentation, not new required fields.hostKindis new and identifies webview/hybrid/blazor adapters. Owner-aware contexts receive stablewebview-<index>IDs so duplicate AutomationIds and retained inactive pages cannot target another view; existing UI aliases remain supported. Readiness now explicitly describes initialized CDP, not merely loaded HTML.The screenshot contract documents native-first PNG capture, which CLI/MCP now use. Browser-network capture remains unimplemented;
/api/v1/webview/networkexplicitly returns 501 instead of misrepresenting native .NET captures as browser traffic./api/v1/network/requestsremains available.Compatibility / breaking changes
WebViewBridge/Bridgessurface must rebuild because nested type identities change during extraction. Existing builder registration and source entry points remain.AgentClient.SendCdpCommandAsyncthrows for HTTP/CDP/JavaScript errors, and CLI failures return nonzero exits rather than success-shaped error data.includeWebViewElementsenables generic DOM layout enrichment; omission retains the legacyincludeBlazorElementsbehavior.Verification
The broader CLI subset reported two failures in unchanged gesture paths (
UiGesture_ReportsWhichTierHandledIt,UiGesture_Pinch_SendsScaleToGestureRoute); targeted WebView checks are green. Apple validation used Xcode 26.6 per process because the existing sample lacks the scene lifecycle required by the selected Xcode 27.