Skip to content

Document nullable favicon contract in DefaultWebClient.onPageStarted (closes #490) - #1086

Closed
jim-daf wants to merge 1 commit into
Justson:androidxfrom
jim-daf:fix/onpagestarted-favicon-nullable-issue-490
Closed

Document nullable favicon contract in DefaultWebClient.onPageStarted (closes #490)#1086
jim-daf wants to merge 1 commit into
Justson:androidxfrom
jim-daf:fix/onpagestarted-favicon-nullable-issue-490

Conversation

@jim-daf

@jim-daf jim-daf commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Document the nullable favicon contract in DefaultWebClient.onPageStarted

Closes #490

What changed

agentweb-core/src/main/java/com/just/agentweb/DefaultWebClient.java

Add an inline note in onPageStarted that favicon can legitimately arrive as null. 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 as Bitmap (non-nullable) and then dereferenced. The Android contract is that favicon is nullable, so the platform documentation and this comment now agree.

Key snippet

public void onPageStarted(WebView view, String url, Bitmap favicon) {
    // 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.
    if (!mWaittingFinishSet.contains(url)) {
        ...
    }
}

Recommendation for downstream callers

In Kotlin, declare the parameter as nullable and avoid !!:

override fun onPageStarted(view: WebView?, url: String?, favicon: Bitmap?) {
    super.onPageStarted(view, url, favicon)
}

@jim-daf
jim-daf marked this pull request as ready for review April 23, 2026 09:47
Copilot AI review requested due to automatic review settings April 23, 2026 09:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.onPageStarted clarifying favicon may be null per platform contract.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +459 to +462
// 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.

Copilot AI Apr 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
@Justson

Justson commented Aug 27, 2026

Copy link
Copy Markdown
Owner

感谢贡献。这个 PR 只新增了 4 行注释,没有任何代码变更,但标题写的是 closes #490 —— 仅凭注释无法关闭一个崩溃类 issue,所以这里不合入。

注释内容本身是准确的:DefaultWebClient#onPageStarted 确实只是把 favicon 透传给 super,全程没有解引用,库这一侧不存在空指针风险。

#490 的真正原因在使用方:Kotlin override 把参数签名写成了非空 Bitmap,应改为可空 Bitmap?

override fun onPageStarted(view: WebView?, url: String?, favicon: Bitmap?) { }

我会把这个结论直接补充到 issue #490 里,比放在源码注释中更容易被遇到同样问题的人看到。

另外本 PR 与同批次的 #1083 是对同一 issue 的两种互相矛盾的方案,#1083 也已一并关闭并说明了原因。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Kotlin使用agentweb引起WebView的onPageStarted中favicon为空导致崩溃

3 participants