837373 - java.lang.NullPointerException: at org.mozilla.gecko.Favicons.isLargeFavicon(Favicons.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Scoobidiver (away)]

It's #1 top crasher in the first hours of 19.0b4. The regression range is:
http://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=44ad9c166d46&tochange=f70101f7c7fa
It's likely a regression from bug 819973 although Aurora is unaffected.

Here is a crash report: bp-d7e861f1-9c72-4905-af2f-b75df2130202.

java.lang.NullPointerException
	at org.mozilla.gecko.Favicons.isLargeFavicon(Favicons.java:183)
	at org.mozilla.gecko.Favicons.scaleImage(Favicons.java:189)
	at org.mozilla.gecko.AwesomeBarTab.updateFavicon(AwesomeBarTab.java:97)
	at org.mozilla.gecko.BookmarksTab$BookmarksListAdapter.getView(BookmarksTab.java:328)
	at android.widget.AbsListView.obtainView(AbsListView.java:2033)
	at android.widget.ListView.makeAndAddView(ListView.java:1772)
	at android.widget.ListView.fillDown(ListView.java:672)
	at android.widget.ListView.fillFromTop(ListView.java:732)
	at android.widget.ListView.layoutChildren(ListView.java:1625)
	at android.widget.AbsListView.onLayout(AbsListView.java:1863)
	at android.view.View.layout(View.java:11282)
	at android.view.ViewGroup.layout(ViewGroup.java:4224)
	at android.support.v4.view.ViewPager.onLayout(ViewPager.java:1388)
	at android.view.View.layout(View.java:11282)
	at android.view.ViewGroup.layout(ViewGroup.java:4224)
	at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1628)
	at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1486)
	at android.widget.LinearLayout.onLayout(LinearLayout.java:1399)
	at org.mozilla.gecko.AwesomeBarTabs$Background.onLayout(AwesomeBarTabs.java:361)
	at android.view.View.layout(View.java:11282)
	at android.view.ViewGroup.layout(ViewGroup.java:4224)
	at android.widget.FrameLayout.onLayout(FrameLayout.java:431)
	at android.view.View.layout(View.java:11282)
	at android.view.ViewGroup.layout(ViewGroup.java:4224)
	at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1628)
	at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1486)
	at android.widget.LinearLayout.onLayout(LinearLayout.java:1399)
	at android.view.View.layout(View.java:11282)
	at android.view.ViewGroup.layout(ViewGroup.java:4224)
	at android.widget.FrameLayout.onLayout(FrameLayout.java:431)
	at android.view.View.layout(View.java:11282)
	at android.view.ViewGroup.layout(ViewGroup.java:4224)
	at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1628)
	at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1486)
	at android.widget.LinearLayout.onLayout(LinearLayout.java:1399)
	at android.view.View.layout(View.java:11282)
	at android.view.ViewGroup.layout(ViewGroup.java:4224)
	at android.widget.FrameLayout.onLayout(FrameLayout.java:431)
	at android.view.View.layout(View.java:11282)
	at android.view.ViewGroup.layout(ViewGroup.java:4224)
	at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:1514)
	at android.view.ViewRootImpl.handleMessage(ViewRootImpl.java:2467)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:4424)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:511)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:784)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:551)
	at dalvik.system.NativeStart.main(Native Method)

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.lang.NullPointerException%3A+at+org.mozilla.gecko.Favicons.isLargeFavicon%28Favicons.java%29

Comment 1

[Aaron Train [:aaronmt]]

Finding Beta pretty much unusable on my cousins Galaxy SIII with this crash; he just opens up the awesome-screen and this happens.

Comment 2

[Alex Keybl [:akeybl]]

We should strongly consider backing out bug 819973 for beta 5 - it wasn't a critical regression as compared to this startup instability.

Comment 3

[Brian Nicholson (:bnicholson)]

Attachment: Check for valid favicon before scaling it

This should fix both this bug and 837404.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 709799 [details] [diff] [review]
Check for valid favicon before scaling it

Hmm, this works but makes me want to fix Favicons.scaleImage to do the check and maybe return null :)

Comment 5

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e3ed7d764b33

Comment 6

[Brian Nicholson (:bnicholson)]

Comment on attachment 709799 [details] [diff] [review]
Check for valid favicon before scaling it

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 819973
User impact if declined: crashes
Testing completed (on m-c, etc.): just landed m-i
Risk to taking this patch (and alternatives if risky): very low, just adds null and size checks
String or UUID changes made by this patch: none

Comment 7

[Alex Keybl [:akeybl]]

(In reply to Brian Nicholson (:bnicholson) from comment #3)
> Created attachment 709799 [details] [diff] [review]
> Check for valid favicon before scaling it
> 
> This should fix both this bug and 837404.

What about bug 812867, which is also a suspected favicon regression? If we're not confident that we've resolved all of the major regressions from bug 819973, I think a backout is still in order.

Comment 8

[Brian Nicholson (:bnicholson)]

If we uplift both this and bug 812867, these regressions should all be fixed.

Comment 9

[Alex Keybl [:akeybl]]

Comment on attachment 709799 [details] [diff] [review]
Check for valid favicon before scaling it

Approving to land alongside bug 812867. We'll take the forward fixes here, but need to be very diligent about being on the lookout for regressions.

Comment 10

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/releases/mozilla-beta/rev/b9384c85454d

Comment 11

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d97ded59975b

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e3ed7d764b33