Simplify WebView API-arkui3-AtomGit

1. The controller moved into the style bag, which breaks the framework's controller convention and makes a shared style a shared live controller
arktsx/webview/lib/types.ets:16,40 · arktsx/webview/lib/webview.ets:329

WebViewStyle extends Style now carries controller?: WebViewController. Three consequences, in increasing order of concreteness:

Convention. Every other framework component that carries a controller or state takes it as a separate leading parameter, never inside the *Style bag: Slider(state, style?) (components/slider.ets:297), ComboBox(state, count, style?, content?) (combobox.ets:275), TextField(state, …) (textField.ets:488), CheckBoxEx(state, …) (checkbox.ets:219), and the closest analogue ContextMenu(controller, style, menuItems) (layer/contextMenu.ets:177). WebView followed this until now.
The type is transportable. Because WebViewStyle is a Style, it can be assigned to a Style-typed variable or hoisted to a module constant — an established idiom here (DARK_THEME: DiagramTheme at diagram/demo/demo.ets:74, SPACIOUS/DENSE: SlideDensity at preso/lib/theme.ets:352-356, DEFAULT_POLICY_FALLBACK: HapticPolicy at framework/lib/haptics/hapticPolicy.ets:97). A hoisted WebViewStyle shared by two WebView instances now shares one live controller. WebViewController.attach() calls this.detach() first (controller.ets:63-66), which nulls the handle and clears the state fields, so host B silently steals the link from host A; disposing either view then kills the survivor, because WebViewHost.close() calls controller.detach() (webview.ets:134) and every subsequent command is a silent no-op while the view still repaints. Passing one controller to two views was always possible with a leading parameter — what changed is that the controller now rides inside a value that is natural to hoist, with no signal at the call site that a live object is inside.
A new module cycle. types.ets:16 imports WebViewController from ./controller while controller.ets:19 imports WebScriptMessage from ./types. Type-only on both sides, so nothing breaks, but types.ets is no longer the package's leaf module. The read-once carve-out this forces (types.ets:37-40, WebView_Module.md:65) is the symptom: no other Style field has read-once semantics, because no other Style field is state.

Suggested fix: keep the simplification, keep the controller out of the type — WebView(style: WebViewStyle, controller?: WebViewController, callbacks?: WebViewCallbacks, @memo content?: () => void). The one-argument WebView({ initialUrl: … }) that both demos now use still compiles unchanged. If style.controller is deliberate, it needs a recorded reason in the design doc's "Open decisions (resolved in-session)" section (10_Architecture/WebView_Module_Design.md:182), which already holds decisions of exactly this weight, and attach() should report an already-attached controller loudly instead of calling detach() silently.

Suggestions (9)

1. The rememberDisposable dispose lambda lost its explicit annotations
arktsx/webview/lib/webview.ets:338-339 — now (hst?: WebViewHost) => { hst?.close() }. Coding_Standards.md:1419 documents that this exact shape — a @memo function with a function-typed parameter (@memo content?: () => void) using rememberDisposable internally — can crash the compiler (SIGSEGV in IsETSPrimitiveType) or fail type inference at the call site, and prescribes explicit parameter and return-type annotations on every lambda. The previous (hst: WebViewHost | undefined): void => was the compliant form. CI compiles green today, but the guard existed for a reason.

2. The demos no longer demonstrate the controller, so the new path has no coverage
arktsx/webview/demo/src/ets/demo.ets:17-24 · arktsx/demo/lib/tabs/webview.ets:24-26 — both demos drop WebViewController and the callbacks argument. After this PR nothing in the repo constructs a WebViewController outside test/webview.test.ets, which exercises only the detached state machine. The expression this PR introduced, style.controller ?? new WebViewController() (webview.ets:329), is covered by no demo and no test, in a change whose entire subject is that parameter — and WebView_Module.md:9-13 still lists WebViewController as a main export while WebView_Module_Design.md:82 frames the module as a "Controller/widget split (Flutter model)". Keeping a remember(() => new WebViewController()) plus an onPageFinished callback in arktsx/webview/demo/src/ets/demo.ets would cover both new paths in one file; the arktsx/demo tab can stay declarative.

3. Dead import left behind: getTimestamp
arktsx/webview/lib/webview.ets:23 — the PR removes lastCaptureTs (its intended consumer) and re-quotes this import line, but keeps getTimestamp, which now has zero uses in the file. It was already dead at base, but this is the natural moment to drop it.

4. scheduleCallback trailing-lambda rewrite diverges from every other call site
arktsx/webview/lib/webview.ets:127-129, 183-185 — scheduleCallback() { this.pollStep() } is the only trailing-lambda use of this API in the repository; all other call sites (53 files, including chromium.ets in this same package, the gaze tracker, the framework scroll adapter) use scheduleCallback(() => …). The standards' trailing-lambda guidance targets memo content closures, not plain runtime functions, and since the callback parameter is optional, scheduleCallback() { reads like a no-arg call followed by a block. No functional issue — the two forms are pure syntax sugar and pass the identical function value to the runtime, with the same negligible per-frame closure either way. This is a consistency point only.

5. The new controller docstring is verbose and its contract is stated three times
arktsx/webview/lib/types.ets:30-39 — nine lines hard-wrapped one clause per line, unlike every other docstring in the repo, and five of them restate what the type name already says. The non-obvious part is the read-once contract, which is also duplicated in the @param style block at webview.ets:306-313 and again in both .md files. Suggest keeping only the why: /** Optional external controller. Read once at host creation; a different instance on a later re-composition is ignored. */

6. WebView_Module.md still reports no dependents
docs/second-brain/30_Support_Libraries/WebView_Module.md:47 — "Dependents — (none yet)" is false: arktsx/demo/arkts.yml:33 declares arktsx.webview: "*" and arktsx/demo/lib/tabs/webview.ets:16 imports WebView. Pre-existing, but this PR edits this exact file and the line is two lines of rg to confirm.

7. Blank line after the license header removed in all four .ets files
webview.ets:14-15, types.ets:14-15, webview/demo/src/ets/demo.ets:14-15, demo/lib/tabs/webview.ets:14-15 — every other .ets file sampled keeps a blank line between */ and the first import.

8. Quote direction: the package is left half-migrated
webview.ets, types.ets and the demos are now single-quoted while controller.ets and chromium.ets in the same lib/ directory remain double-quoted. Coding_Standards.md states no quote rule (repo-wide imports run about 3:1 double), so nothing is violated — but either finish the package or codify a rule in the standards.

9. Idle-path power cost of the poll loop (pre-existing, in touched lines)
arktsx/webview/lib/webview.ets:160-187 — on the mobile/macOS path, while active the loop runs at vsync rate indefinitely: controller.poll() crosses FFI seven times per frame and writes six MutableState fields per frame regardless of change. Only the snapshot upload is throttled (IDLE_SNAPSHOT_INTERVAL); the native polling and state writes are not, and there is no stop condition for a hidden or backgrounded host — constant CPU wakeups for zero information on a static page. Follow-up, not a blocker: gate controller.poll() on the same idle cadence as pollFrame, and stop the loop when the host is not visible. The Linux CDP path already does this correctly.

Positive Observations

The signature simplification itself is the right call and it is landed consistently. Making style a required parameter suits a component that cannot render without layout options; @param style is documented on the public function, the design doc's signature line was updated in the same commit, and both call sites were migrated. No stale call site or prose survives anywhere in the tree — initialUrl, WebView(undefined and WebView(controller were grepped across arktsx/ and docs/.
Doc coverage is complete for everything the change touches. Both affected notes were updated (10_Architecture/WebView_Module_Design.md, 30_Support_Libraries/WebView_Module.md), and the design doc's types.ets layout line correctly drops the WebViewOptions type that never existed.
The controller allocation change is behaviour-neutral. rememberDisposable's compute runs once per mount, so style.controller ?? new WebViewController() inside it allocates exactly one controller per mount, identical to the remember(() => controller ?? …) caching it replaces — and the read is inside the closure, so it is not the state-capture anti-pattern. The read-once semantics were the same before; the new doc comment just states them.
The polled-callback passthrough survives the reparameterisation intact. WebViewHost.dispatchCallbacks() still diffs prevUrl/prevTitle/prevProgress/prevLoading against controller state and fires user callbacks only on change; dropping lastCaptureTs is a clean removal of a field with no reader.
Security is unaffected. The controller never crosses the IDL boundary as an argument, and style payloads are not serialized generically — appendDefinedStyle (components/style.ets:642) copies field by field and silently drops controller, and the only other PartialStyleNode consumer (grasp/component-hierarchy.ets:425) emits a type name only, so no path carries a controller instance into a dump, log or native call. The fctx ? … : nullptr narrowing is equivalent (getForeignContext() returns ForeignContextData | undefined, an object type that is never falsy), and the dispose lambda fires at most once, from ScopeImpl.dispose().
Native resource ownership is careful. close() releases every native resource on the disposal path — Chromium tab, native handle, lastImage, VideoTexture — rather than relying on GC, and the comment explaining why painterState is deliberately not cleared during scope disposal documents a real runtime constraint. detach() nulls the controller's pointer before the host is dropped, so a caller-supplied controller cannot retain a dangling handle.
The demo simplifications are a small win. Dropping the wrapper Column in TabWebView removes one measure/layout node per frame, and sizing is safe: StyleImpl falls back to SizePolicy.Stretched when neither width nor widthPolicy is given (components/style.ets:541,567), so WebView({ initialUrl }) still fills the tab.
All always-flagged anti-patterns clean: no state captured in closures, no I/O or computation outside remember, no loop-built children, no forbidden @koalaui/* imports. Copyright headers correct (2026 for files created 2026-09-03).

Review Details

Review agents: Architecture, Security, Performance, Conventions
Context: Checked against project second brain (design principles, coding standards, dependency graph)
Note: The Important finding concerns the API shape rather than its implementation — the code works as written, and both the shared-controller hazard and the module cycle are latent until a style is hoisted or reused.

Reviewed by arkui3 automated review system

sergeymalenkov成员

9月26日评论：

Review correction — Important finding #1 withdrawn

PR: #26 — Simplify WebView API

The review above raised one Important finding: that moving the controller into WebViewStyle breaks the framework's controller convention. That finding is withdrawn. The claim was wrong on its central point, and it should not gate this change.

What was wrong. The finding asserted that every other framework component carrying state takes it as a separate leading parameter. That is not true, and the counter-precedents are direct:

AdaptiveGridStyle.adapter?: ScrollAdapter (arktsx/framework/lib/layout/adaptiveGrid.ets:68)
GridStyle via GridOptions<ScrollAdapter> → adapter?: ScrollAdapter (arktsx/framework/lib/layout/grid.ets:62)

ScrollAdapter carries scroll state, animation callbacks and bindToCurrentComponent() — the same class of live, stateful object as WebViewController. It lives inside a *Style interface, and like style.controller it is optional: absent in the common case, supplied when the caller needs external control. So this PR follows an established framework pattern rather than departing from one.

The ergonomics point in the finding also ran the wrong way. The previous leading-parameter form forced the declarative case to write WebView(undefined, { … } as WebViewStyle) — exactly what both demos did before this PR. Placing a usually-absent object in the style bag is what removes that.

What survives, at lower weight. Two points remain as documentation notes, not blockers:

A WebViewStyle hoisted to a module constant and shared by two WebView instances would share one controller, and WebViewController.attach() calls detach() first (lib/controller.ets:63-66), so the second attach steals the link from the first. This hazard is inherent to the pattern and applies equally to a shared adapter today, so it is a doc sentence at most — not a reason to change the API.
types.ets:16 imports WebViewController from ./controller while controller.ets:19 imports WebScriptMessage from ./types. Type-only on both sides and it compiles, but it is a new module edge and types.ets is no longer the package's leaf module.

Everything else in the review above stands, including the suggestions the demos should keep exercising a WebViewController and the callbacks argument, the dead getTimestamp import, and the rememberDisposable annotation regression.

Corrected by arkui3 automated review system

sergeymalenkov成员

9月26日合入了pull request