Document nullable favicon contract in DefaultWebClient.onPageStarted (closes #490) - #1086
Conversation
There was a problem hiding this comment.
Pull request overview
This PR documents the Android WebView contract that onPageStarted’s favicon parameter may legitimately be null, aiming to prevent Kotlin overrides from incorrectly treating it as non-null and crashing (issue #490).
Changes:
- Add an inline comment in
DefaultWebClient.onPageStartedclarifyingfaviconmay benullper platform contract.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Issue #490: Some sites (notably Kotlin overrides) crashed because | ||
| // favicon arrived as null and downstream code dereferenced it. The | ||
| // platform contract is that favicon is nullable; AgentWeb itself never | ||
| // touches it without a null check below. |
There was a problem hiding this comment.
The new comment is factually inconsistent with the method body: there is no null check on favicon “below” (it’s only forwarded to super.onPageStarted(...)). Consider rewording to avoid implying a null check exists (e.g., say AgentWeb doesn’t dereference favicon here), and consider adding an explicit @Nullable annotation to the favicon parameter so Kotlin overrides are forced/encouraged to declare it nullable (the project already uses androidx nullability annotations).
|
感谢贡献。这个 PR 只新增了 4 行注释,没有任何代码变更,但标题写的是 注释内容本身是准确的: 但 #490 的真正原因在使用方:Kotlin override 把参数签名写成了非空 override fun onPageStarted(view: WebView?, url: String?, favicon: Bitmap?) { }我会把这个结论直接补充到 issue #490 里,比放在源码注释中更容易被遇到同样问题的人看到。 另外本 PR 与同批次的 #1083 是对同一 issue 的两种互相矛盾的方案,#1083 也已一并关闭并说明了原因。 |
Document the nullable favicon contract in
DefaultWebClient.onPageStartedCloses #490
What changed
agentweb-core/src/main/java/com/just/agentweb/DefaultWebClient.javaAdd an inline note in
onPageStartedthatfaviconcan legitimately arrive asnull. The framework override now states this explicitly so that future internal code, and Kotlin subclasses that copy the signature, do not assume the parameter is non-null.The Kotlin crash the reporter saw is the
WebViewClient.onPageStarted(view: WebView?, url: String?, favicon: Bitmap?)override being declared asBitmap(non-nullable) and then dereferenced. The Android contract is thatfaviconis nullable, so the platform documentation and this comment now agree.Key snippet
Recommendation for downstream callers
In Kotlin, declare the parameter as nullable and avoid
!!: