930163 - crash in java.lang.NullPointerException: at org.mozilla.gecko.home.TwoLinePageRow.updateFromCursor(TwoLinePageRow.java)

Status: RESOLVED WORKSFORME

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Aaron Train [:aaronmt]]

This bug was filed from the Socorro interface and is 
report bp-4793b55e-4c7f-469a-a915-cfb182131023.
=============================================================

java.lang.NullPointerException
	at org.mozilla.gecko.home.TwoLinePageRow.updateFromCursor(TwoLinePageRow.java:207)
	at org.mozilla.gecko.home.BookmarksListAdapter.bindView(BookmarksListAdapter.java:193)
	at org.mozilla.gecko.home.MultiTypeCursorAdapter.getView(MultiTypeCursorAdapter.java:62)
	at android.widget.AbsListView.obtainView(AbsListView.java:2350)

Comment 1

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

Comment 2

[Lucas Rocha (:lucasr)]

Comment on attachment 821895 [details] [diff] [review]
Patch

Review of attachment 821895 [details] [diff] [review]:
-----------------------------------------------------------------

I'm uneasy with just adding these null checks without understanding the real cause of these crashes. For instance, the row with a null URL will probably look broken and not work at all. What would be the point of working around the NPE?

I'd like to hear a better reason for why this is better than simply exposing the bug (and fixing the real cause) before giving r+.

Comment 3

[Sriram Ramasubramanian [:sriram]]

I'll wait for steps needed then. This is hard to speculate what could go wrong to cause this issue.

Comment 4

[Richard Newman [:rnewman]]

Nothing should ever be setting a bookmark URL to null, but it's not a NOT NULL column, so it's not impossible. 

I don't think waiting for steps will help -- this could be the result of an import from the Android browser, a weird Sync result (unlikely, but possible), or even a JNI add-on. Those things aren't going to happen in routine QA.

Sriram: spend some time trying to find how nulls could leak into the DB this way?

Comment 5

[Lucas Rocha (:lucasr)]

(In reply to Richard Newman [:rnewman] from comment #4)
> Nothing should ever be setting a bookmark URL to null, but it's not a NOT
> NULL column, so it's not impossible. 
> 
> I don't think waiting for steps will help -- this could be the result of an
> import from the Android browser, a weird Sync result (unlikely, but
> possible), or even a JNI add-on. Those things aren't going to happen in
> routine QA.
> 
> Sriram: spend some time trying to find how nulls could leak into the DB this
> way?

Agree. This bug seems to have the same root cause than bug 883500 i.e. null URLs coming from the DB. The crash just happens at a different point here because in we're using TextUtils.equals() in Beta, instead of url.equals().

Based on the crash comments in bug 883500, this seems to be happening more often while scrolling bookmarks. Sriram, maybe start from there?

Comment 6

[Kevin Brosnan [Ex-Mozilla]]

I don't see this signature any more.

https://crash-stats.mozilla.com/query/?product=Fennec&version=ALL%3AALL&range_value=4&range_unit=weeks&query_search=signature&query_type=contains&query=java.lang.NullPointerException%3A+at+org.mozilla.gecko.home.TwoLinePageRow.updateFromCursor&reason=&release_channels=&build_id=&process_type=any&hang_type=any

Comment 7

[Chris Peterson [:cpeterson]]

Removing steps-wanted keyword because this bug has been resolved.

Comment 8

[Chris Peterson [:cpeterson]]

Removing steps-wanted keyword because this bug has been resolved.