Closed Bug 1089266 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

36 Branch
All
Android
defect
Not set
critical

Tracking

(firefox36 verified, fennec36+)

VERIFIED FIXED
Firefox 36
Tracking Status
firefox36 --- verified
fennec 36+ ---

People

(Reporter: aaronmt, Assigned: rnewman)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

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
tracking-fennec: ? → +
tracking-fennec: + → 36+
Assignee: nobody → 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?
Status: NEW → ASSIGNED
Flags: needinfo?(myk)
Flags: needinfo?(mark.finkle)
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.
Flags: needinfo?(mark.finkle)
(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).
Flags: needinfo?(myk)
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.
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)
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.
Attachment #8520909 - Flags: review?(mark.finkle)
(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.
Attachment #8520909 - Flags: review?(mark.finkle) → review+
Myk, does this need to be uplifted?
Flags: needinfo?(myk)
https://hg.mozilla.org/mozilla-central/rev/426f385c391d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
(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?
Flags: needinfo?(myk)
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.
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.