fix(WebViewClientDelegate): supply non-null favicon placeholder to delegates (#490) - #1083
fix(WebViewClientDelegate): supply non-null favicon placeholder to delegates (#490)#1083jim-daf wants to merge 1 commit into
Conversation
…legates (Justson#490) Resolves Justson#490 WebView delivers onPageStarted with a null favicon during early page-start. WebViewClientDelegate forwards that value straight to the user's delegate. Kotlin delegates whose signature declares the favicon parameter as a non-null Bitmap (the default in Kotlin source unless explicitly Bitmap?) crash at the synthetic Intrinsics.checkParameterIsNotNull bytecode: Parameter specified as non-null is null: method kotlin.jvm.internal.Intrinsics.checkParameterIsNotNull, parameter favicon This change substitutes a 1x1 transparent ARGB_8888 placeholder bitmap for a null favicon before forwarding to the delegate. The placeholder is lazily created and cached, so there is no cost on the common path where the platform supplies a real bitmap. Delegates that declared the parameter as Bitmap? still work, just with a non-null value.
eca795d to
3092103
Compare
There was a problem hiding this comment.
Pull request overview
Fixes a Kotlin crash when WebView invokes onPageStarted with a null favicon by ensuring delegates always receive a non-null Bitmap.
Changes:
- Add a lazily-initialized 1×1 transparent
Bitmapplaceholder cached inWebViewClientDelegate. - Substitute the placeholder for
nullfavicons when forwardingonPageStartedto the delegate (and when callingsuper).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private static volatile Bitmap sFaviconPlaceholder; | ||
|
|
||
| private static Bitmap faviconPlaceholder() { | ||
| Bitmap placeholder = sFaviconPlaceholder; | ||
| if (placeholder == null) { | ||
| synchronized (WebViewClientDelegate.class) { | ||
| placeholder = sFaviconPlaceholder; | ||
| if (placeholder == null) { | ||
| placeholder = Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888); | ||
| sFaviconPlaceholder = placeholder; | ||
| } | ||
| } | ||
| } | ||
| return placeholder; | ||
| } |
There was a problem hiding this comment.
The placeholder bitmap is cached statically and the same instance is forwarded to external delegates. Because this instance escapes library ownership, a delegate could mutate or call recycle() on it, which would leave sFaviconPlaceholder in a bad state and potentially crash later calls. Consider either recreating the placeholder when placeholder.isRecycled() is true, or returning a fresh/copy placeholder per callback to avoid sharing a mutable/recyclable Bitmap instance with user code.
| // signatures keep working; nullable signatures see no observable | ||
| // change beyond receiving a non-null bitmap. |
There was a problem hiding this comment.
The comment claims “nullable signatures see no observable change beyond receiving a non-null bitmap”, but receiving a non-null placeholder instead of null is an observable behavior change (callers may have been using null to mean “no favicon yet”). Please reword the comment to acknowledge this behavior change and/or describe how callers can detect the placeholder if needed.
| // signatures keep working; nullable signatures see no observable | |
| // change beyond receiving a non-null bitmap. | |
| // signatures keep working. This is an observable change for nullable | |
| // consumers because they receive the shared placeholder instead of | |
| // null; callers that need to detect "no favicon yet" can compare the | |
| // received Bitmap by identity with faviconPlaceholder(). |
|
感谢贡献,但这个修法方向上有问题,暂不合入。 PR 在 favicon 为 null 时塞入一个静态共享的 1×1 透明 1. 违反平台契约 2. 静态共享 Bitmap 有回收风险 3. 问题根因不在库这一侧 override fun onPageStarted(view: WebView?, url: String?, favicon: Bitmap) { } // ❌
override fun onPageStarted(view: WebView?, url: String?, favicon: Bitmap?) { } // ✅正确写法是把参数声明为可空 AgentWeb 自身在 另外,本 PR 与同批次的 #1086 是对同一 issue 的两种互相矛盾的方案(一个主张「就该可空」、一个主张「要填非空」),也侧面说明这个方向需要再斟酌。 |
Forward a non-null favicon to delegates to fix Kotlin crashes
Resolves #490 -
IllegalArgumentException: Parameter specified as non-null is null: ... favicon.Why Kotlin callers crash
WebViewis allowed to deliverWebViewClient.onPageStartedwithfavicon == nullduring early page-start (e.g. when the page hasn't yet announced an icon).
WebViewClientDelegate.onPageStartedsimply forwards whatever it received tothe user's delegate.
Kotlin source compiles non-nullable parameter declarations into bytecode that
calls
Intrinsics.checkParameterIsNotNull(...)on entry. A user delegate thatdeclares its
onPageStarted(view: WebView?, url: String?, favicon: Bitmap)--i.e. the favicon parameter is non-nullable -- therefore throws
IllegalArgumentExceptionthe moment AgentWeb forwards a null. This is theexact stack trace in #490.
Fix
agentweb-core/src/main/java/com/just/agentweb/WebViewClientDelegate.java:onPageStarted, whenfaviconis null, substitute a tiny lazilyallocated 1×1 transparent ARGB_8888 placeholder bitmap before calling the
delegate.
Bitmap?signature observe no behavior change beyondreceiving a non-null value.
Bitmap(non-nullable) signature no longer crash.with a real favicon allocates nothing.
This avoids forcing every Kotlin caller to either remember to declare the
parameter as nullable or wire up a
MiddlewareWebClientpurely as anull-shim (the workaround the issue author had to apply).