849254 - java.lang.NullPointerException: at org.mozilla.gecko.BrowserApp.setToolbarHeight(BrowserApp.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Scoobidiver (away)]

It has been hit several times at startup in 22.0a1/20130308 by a single user. Here is a crash report: bp-9876f544-35c0-4a7f-8eb2-eb1a82130308.
Based on the stack trace, it's a regression from bug 716403 whose the patch landed in that build.

java.lang.NullPointerException
	at org.mozilla.gecko.BrowserApp.setToolbarHeight(BrowserApp.java:488)
	at org.mozilla.gecko.BrowserToolbarLayout.refreshMargins(BrowserToolbarLayout.java:62)
	at org.mozilla.gecko.BrowserApp.showAboutHome(BrowserApp.java:965)
	at org.mozilla.gecko.BrowserApp.onTabChanged(BrowserApp.java:117)
	at org.mozilla.gecko.Tabs$5.run(Tabs.java:450)
	at android.app.Activity.runOnUiThread(Activity.java:4185)
	at org.mozilla.gecko.Tabs.notifyListeners(Tabs.java:440)
	at org.mozilla.gecko.Tabs.notifyListeners(Tabs.java:436)
	at org.mozilla.gecko.Tabs$3.run(Tabs.java:165)
	at android.os.Handler.handleCallback(Handler.java:605)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:4507)
	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:787)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:554)
	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.BrowserApp.setToolbarHeight%28BrowserApp.java%29

Comment 1

[Scoobidiver (away)]

Hit by a second user.

Comment 2

[Chris Lord [:cwiiis]]

Attachment: Fix NPE when setting toolbar height

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 723428 [details] [diff] [review]
Fix NPE when setting toolbar height

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

I think moving the call to refreshMargins() into the AboutHomeRunnable should be sufficient to fix this bug; the rest of it is not needed and makes reading the code more confusing.

::: mobile/android/base/BrowserApp.java
@@ +486,5 @@
>                  // about:home widget directly - this is to avoid resizing the
>                  // LayerView, which can cause visible artifacts.
> +                if (mAboutHomeContent != null) {
> +                    mAboutHomeContent.setPadding(0, aVisibleHeight, 0, 0);
> +                }

This should be unnecessary after moving the call to refreshMargins() into the AboutHomeRunnable.

@@ +491,4 @@
>              } else {
> +                if (mToolbarSpacer != null) {
> +                    mToolbarSpacer.setPadding(0, aVisibleHeight, 0, 0);
> +                }

mToolbarSpacer gets initialized in onCreate, so it should never be null here

@@ +496,5 @@
>              aHeight = aVisibleHeight = 0;
>          } else {
> +            if (mToolbarSpacer != null) {
> +                mToolbarSpacer.setPadding(0, 0, 0, 0);
> +            }

Ditto

Comment 4

[Chris Lord [:cwiiis]]

Attachment: Fix NPE when setting toolbar height v2

Done as suggested. You're right, I was just being extra-safe.

Comment 5

[Chris Lord [:cwiiis]]

Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/cecc73dc627a

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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