1089266 - crash in java.lang.NullPointerException: at org.mozilla.gecko.widget.ThemedImageButton.onAttachedToWindow(ThemedImageButton.java)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aaron Train [:aaronmt]]

This bug was filed from the Socorro interface and is 
report bp-1e2b833f-056c-400d-a271-a9be12141024.
=============================================================

java.lang.NullPointerException
	at org.mozilla.gecko.widget.ThemedImageButton.onAttachedToWindow(ThemedImageButton.java:56)
	at android.view.View.dispatchAttachedToWindow(View.java:12585)
	at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:2465)
	at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:2465)
	at android.view.ViewGroup.addViewInner(ViewGroup.java:3601)
	at android.view.ViewGroup.addViewInLayout(ViewGroup.java:3538)
	at android.widget.ListView.setupChild(ListView.java:1840)
	at android.widget.ListView.makeAndAddView(ListView.java:1793)
	at android.widget.ListView.fillDown(ListView.java:691)
	at android.widget.ListView.fillFromTop(ListView.java:752)
	at android.widget.ListView.layoutChildren(ListView.java:1630)
	at android.widget.AbsListView.onLayout(AbsListView.java:2087)
	at android.view.View.layout(View.java:14817)
	at android.view.ViewGroup.layout(ViewGroup.java:4631)
	at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1671)
	at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1525)
	at android.widget.LinearLayout.onLayout(LinearLayout.java:1434)
	at android.view.View.layout(View.java:14817)
	at android.view.ViewGroup.layout(ViewGroup.java:4631)
	at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1671)
	at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1525)
	at android.widget.LinearLayout.onLayout(LinearLayout.java:1434)
	at android.view.View.layout(View.java:14817)
	at android.view.ViewGroup.layout(ViewGroup.java:4631)
	at android.widget.FrameLayout.layoutChildren(FrameLayout.java:453)
	at android.widget.FrameLayout.onLayout(FrameLayout.java:388)
	at android.view.View.layout(View.java:14817)
	at android.view.ViewGroup.layout(ViewGroup.java:4631)
	at android.widget.FrameLayout.layoutChildren(FrameLayout.java:453)
	at android.widget.FrameLayout.onLayout(FrameLayout.java:388)
	at android.view.View.layout(View.java:14817)
	at android.view.ViewGroup.layout(ViewGroup.java:4631)
	at android.widget.FrameLayout.layoutChildren(FrameLayout.java:453)
	at android.widget.FrameLayout.onLayout(FrameLayout.java:388)
	at android.view.View.layout(View.java:14817)
	at android.view.ViewGroup.layout(ViewGroup.java:4631)
	at android.view.ViewRootImpl.performLayout(ViewRootImpl.java:1983)
	at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:1740)
	at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:996)
	at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:5600)
	at android.view.Choreographer$CallbackRecord.run(Choreographer.java:761)
	at android.view.Choreographer.doCallbacks(Choreographer.java:574)
	at android.view.Choreographer.doFrame(Choreographer.java:544)
	at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:747)
	at android.os.Handler.handleCallback(Handler.java:733)
	at android.os.Handler.dispatchMessage(Handler.java:95)
	at android.os.Looper.loop(Looper.java:136)
	at android.app.ActivityThread.main(ActivityThread.java:5001)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:515)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:785)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:601)
	at dalvik.system.NativeStart.main(Native Method)

Comment from Myk: long-pressed on link in the app

Comment 1

[Richard Newman [:rnewman]]

This crash is:

* Only BrowserApp calls GeckoApplication#prepareLightweightTheme.
* ThemedImage* expect their mTheme, retrieved from GeckoApplication, to be non-null.

There are a set of related fixes:

* We can have WebappImpl call prepareLightweightTheme.
* We can move theme initialization into GeckoApp (which WebappImpl extends), rather than BrowserApp.
* We can have ThemedImage* handle null themes.

Does anyone have a strong preference here?

Comment 2

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

What's happening here? Are we trying to display a context menu?

I don't really want WebApps to depend on LWTs. I suppose if this becomes a painful dependency, we could do option #2 (move theme initialization into GeckoApp). Making ThemedView.java.frag handle a null mTheme might not be too bad either.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> What's happening here? Are we trying to display a context menu?
> 
> I don't really want WebApps to depend on LWTs.

I agree.


> I suppose if this becomes a
> painful dependency, we could do option #2 (move theme initialization into
> GeckoApp).

This feels slightly better than option #1, but not much.  It still causes webapps to initialize LWTs, even though they don't use them.


Making ThemedView.java.frag handle a null mTheme might not be too
> bad either.

This seems like the best option.  Besides fixing this problem, it also makes Themed* robust against the possibility of GeckoApplication.getLightweightTheme returning null for other subclasses of GeckoApp (including BrowserApp, in case some change in the future causes it to sometimes not call GeckoApplication.prepareLightweightTheme).

Comment 4

[Richard Newman [:rnewman]]

GeckoApp#onCreateOptionsMenu ends up going down this call path, as does GeckoPopupMenu, as does the action bar.

Without tracing the constructor call it's not clear exactly where in the webapp implementation we're building this. I'll do that when I get a chance.

Comment 5

[Richard Newman [:rnewman]]

Here's the stack:

Initializing ThemedImageButton.
java.lang.RuntimeException: stack
	at org.mozilla.gecko.widget.ThemedImageButton.initialize(ThemedImageButton.java:44)
	at org.mozilla.gecko.widget.ThemedImageButton.<init>(ThemedImageButton.java:40)
	at org.mozilla.gecko.menu.MenuItemActionBar.<init>(MenuItemActionBar.java:31)
	at org.mozilla.gecko.menu.MenuItemActionBar.<init>(MenuItemActionBar.java:26)
	at java.lang.reflect.Constructor.newInstance(Native Method)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:288)
	at android.view.LayoutInflater.createView(LayoutInflater.java:607)
	at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:743)
	at android.view.LayoutInflater.rInflate(LayoutInflater.java:806)
	at android.view.LayoutInflater.rInflate(LayoutInflater.java:809)
	at android.view.LayoutInflater.inflate(LayoutInflater.java:479)
	at android.view.LayoutInflater.inflate(LayoutInflater.java:414)
	at android.view.LayoutInflater.inflate(LayoutInflater.java:365)
	at org.mozilla.gecko.menu.MenuItemActionView.<init>(MenuItemActionView.java:53)
	at org.mozilla.gecko.menu.MenuItemActionView.<init>(MenuItemActionView.java:36)
	at org.mozilla.gecko.widget.GeckoActionProvider.onCreateActionView(GeckoActionProvider.java:90)
	at org.mozilla.gecko.prompts.PromptListAdapter.getView(PromptListAdapter.java:239)

Comment 6

[Richard Newman [:rnewman]]

Attachment: Don't rely on lightweight themes in webapps. v1

This papers over the issue by allowing the context menu to open.

However, the root cause is that context menus in webapps don't make a whole lot of sense -- for example, in my test I see:

      http://the/app/url...
      Share Link
      Open Link in New Tab
      Open Link in Private Tab
      Copy Link
      Bookmark Link
      Open With Music App

If you perform one of the tab actions, you'll see a toast offering to switch tab, and an intent handler overlay will fire!

I suggest filing a follow-up bug for someone with more familiarity with WebappImpl to address this.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

(In reply to Richard Newman [:rnewman] from comment #6)
> I suggest filing a follow-up bug for someone with more familiarity with
> WebappImpl to address this.

Good point, filed as bug 1097261.

Comment 8

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/fx-team/rev/426f385c391d

Comment 9

[Richard Newman [:rnewman]]

Myk, does this need to be uplifted?

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/426f385c391d

Comment 11

[Myk Melez [:myk] [@mykmelez]]

(In reply to Richard Newman [:rnewman] from comment #9)
> Myk, does this need to be uplifted?

No, it only exists on Central.  Unsure why, perhaps it's a regression from bug 1077755?

Comment 12

[Cristina Madaras, QA [:CristinaM]]

Verified as fixed in Firefox for Android 36.0a2 (2014-12-04);
Device: Nexus 4 (Android 4.4.4);
No crash when long tapping on link in the app.