Custom internal webview

[phiviet01]

No description provided.

[JakubMrozek]

@claude

[claude]

Claude finished @JakubMrozek's task in 2m 15s —— View job

---

Code Review

• Read and analyze changed files
• Review Dart layer (compat_types, webview backends, in_app_webview)
• Review Android native layer (KontextInAppWebView.kt, KontextInAppWebViewPlugin.kt)
• Review iOS native layer (KontextInAppWebViewPlugin.swift)
• Review widget modifications (ad_format, ads_provider, kontext_webview)
• Review tests
• Post final review

---

Summary

This PR replaces flutter_inappwebview with a custom native webview implementation, adding a compat layer that allows switching between the custom and upstream backends via a single line in in_app_webview.dart. The architecture is sound and the overall approach is well-structured. Below are findings ranging from behavioral bugs to minor issues.

---

🔴 Critical / Behavioral Bugs

1. iOS: callHandler doesn't return a Promise [ios/Classes/KontextInAppWebViewPlugin.swift:332]

The iOS makeBridgeScript() installs a callHandler that returns undefined, while:

• The upstream flutter_inappwebview returns a Promise
• The Android custom implementation correctly returns a Promise via _callHandlerID + resolveJavaScriptCall

This means any ad iframe JS code doing await window.flutter_inappwebview.callHandler(...) will behave differently on iOS vs Android — on iOS it will get undefined immediately rather than a resolved value.

Fix this →

2. iOS: Wrong URL in error callbacks [ios/Classes/KontextInAppWebViewPlugin.swift:262-275]

Both didFail and didFailProvisionalNavigation pass webView.url as failingURL. For provisional navigation (e.g., a new load that fails before committing), webView.url is the previous page's URL, not the failing URL. The failing URL should be extracted from the error's userInfo (NSURLErrorFailingURLStringErrorKey).

// current (wrong for provisional):
sendLoadError(error, failingURL: webView.url)

// better:
let failingURL = (error as NSError).userInfo[NSURLErrorFailingURLStringErrorKey] as? String
    .flatMap { URL(string: $0) } ?? webView.url

Fix this →

---

🟡 Important Issues

3. Android: onPageStarted fallback bridge injection race [android/.../KontextInAppWebView.kt:233-238]

When documentStartSupported is false (older Android), injectFallbackStartScripts() is called in onPageStarted. However, evaluateJavascript is asynchronous — the bridge JS may not be injected before the page's own scripts run, meaning early callHandler calls from the page will fail. This is a known limitation with older WebView APIs, but it's worth documenting or adding a comment.

4. Android: bypassMainFrameLoads can grow unbounded [android/.../KontextInAppWebView.kt:199,212]

If shouldOverrideUrlLoading returns "ALLOW" and the URL is added to bypassMainFrameLoads, but the WebView never navigates to that URL (e.g., the user navigates away), the URL is never removed from the set. Consider adding a size cap or TTL, or clearing the set on page start.

5. Android: Deprecated onReceivedError may double-fire [android/.../KontextInAppWebView.kt:248-283]

Both the deprecated onReceivedError(view, errorCode, description, failingUrl) and the modern onReceivedError(view, request, error) are overridden. On API >= 23, only the modern one fires. On older APIs, only the deprecated one fires. This is correct, but be aware that on API < 23 the type field in the error map will always be the numeric errorCode (not null), while on API >= 23 it's error.errorCode. The Dart-side WebResourceError.type will have different semantics depending on API level.

6. iOS: console.info is not captured [ios/Classes/KontextInAppWebViewPlugin.swift:367]

makeConsoleShimScript() patches ['log', 'warn', 'error', 'debug'] but not console.info. Any ad code calling console.info will bypass the shim entirely. Android's WebChromeClient.onConsoleMessage captures all levels automatically.

---

🟢 Minor / Nits

7. Docs contain absolute local paths [docs/internal_webview_compat.md:13,26]

[lib/src/webview/in_app_webview.dart](/Users/duc/StudioProjects/kontext_flutter_sdk/lib/src/webview/...)

These local machine paths should be relative repo paths (or just inline code without links).

8. _pendingJavaScriptCalls can grow unbounded [lib/src/webview/backends/custom_in_app_webview.dart:181]

If the native side sends onJavaScriptHandler for a handler name that's never registered via addJavaScriptHandler, the entry accumulates in _pendingJavaScriptCalls indefinitely. Low risk in practice, but worth noting.

9. Android: Missing @Suppress("DEPRECATION") [android/.../KontextInAppWebView.kt:266]

The deprecated onReceivedError(view, errorCode, description, failingUrl) override will generate a lint warning on API >= 23. Add @Suppress("DEPRECATION") or @SuppressWarnings("deprecation").

10. upstream_in_app_webview.dart is dead code in production [lib/src/webview/in_app_webview.dart:5]

The upstream backend is commented out and only used for comparison during development. Consider whether this file should remain in the repo long-term or be removed once the custom backend is validated.

---

Positive Notes

• The compat layer design is clean — the InAppWebView API surface is faithfully mirrored, making backend switching genuinely transparent.
• The Android DOCUMENT_START_SCRIPT / WebViewCompat.addDocumentStartJavaScript path (with documentStartSupported fallback) is a solid approach.
• iOS cookie seeding with the async completion counter pattern (remainingCookies) is correct and avoids loading before cookies are set.
• Removal of the flutter_inappwebview debugLoggingSettings global side effect from AdsProvider is a good cleanup.
• The _hasDeliveredOnWebViewCreated guard correctly prevents double-firing onWebViewCreated across iOS/Android platform view creation lifecycle differences.

[JakubMrozek]

@claude

[claude]

Claude finished @JakubMrozek's task in 59s —— View job

---

Review Follow-up

All issues raised in the previous review have been resolved in subsequent commits:

Issue	Fix Commit
iOS: callHandler should return a Promise	360c281 Fix: Make iOS callHandler return a Promise like Android
iOS: Wrong URL in error callbacks	02e8275 Fix: use failing navigation URL in iOS webview error callbacks
iOS: console.info not captured	ce0bd98 Capture console.info
Android: @Suppress("DEPRECATION") missing	8d5879a Suppress deprecation warning for legacy Android error callback
Android: bypassMainFrameLoads unbounded growth	3d376c7 Fix: cap Android bypass main-frame load tracking
Android: onPageStarted fallback race	d3f573a Retry Android fallback start scripts at document end
Docs: absolute local paths	4a7d0db Use relative markdown links instead of absolute workspace paths

The PR is in good shape — all 🔴 critical and 🟡 important findings have been addressed. The two remaining open items from the original review are minor/informational:

• _pendingJavaScriptCalls unbounded growth (lib/src/webview/backends/custom_in_app_webview.dart:181) — low risk, only accumulates entries for unregistered handler names.
• upstream_in_app_webview.dart dead code — intended as a comparison/debugging aid; acceptable to keep during the validation period.

[JakubMrozek]

@phiviet01 Merged to v4 (different repo)