Closed Bug 1401733 Opened 2 years ago Closed 2 years ago

Crash in java.lang.NoSuchMethodException: <init> [class android.content.Context, interface android.util.AttributeSet] at java.lang.Class.getConstructor0(Class.java)

Categories

(Firefox for Android :: Activity Stream, defect, P1, critical)

Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 57
Iteration:
1.30
Tracking Status
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 --- fixed

People

(Reporter: jchen, Assigned: mcomella)

References

Details

(Keywords: crash, Whiteboard: [mobileAS])

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-d952a0fa-cd6e-4db1-9648-e39fe0170920.
=============================================================

Crash first started with the 9-19 Nightly,

> Trace 	
> 
> java.lang.NoSuchMethodException: <init> [class android.content.Context, interface android.util.AttributeSet]
> 	at java.lang.Class.getConstructor0(Class.java:2320)
> 	at java.lang.Class.getConstructor(Class.java:1725)
> 	at android.view.LayoutInflater.createView(LayoutInflater.java:615)
> 	at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:790)
> 	at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:730)
> 	at android.view.LayoutInflater.rInflate(LayoutInflater.java:863)
> 	at android.view.LayoutInflater.rInflateChildren(LayoutInflater.java:824)
> 	at android.view.LayoutInflater.rInflate(LayoutInflater.java:866)
> 	at android.view.LayoutInflater.rInflateChildren(LayoutInflater.java:824)
> 	at android.view.LayoutInflater.inflate(LayoutInflater.java:515)
> 	at android.view.LayoutInflater.inflate(LayoutInflater.java:423)
> 	at org.mozilla.gecko.activitystream.homepanel.StreamRecyclerAdapter.onCreateViewHolder(StreamRecyclerAdapter.java:131)
> 	at org.mozilla.gecko.activitystream.homepanel.StreamRecyclerAdapter.onCreateViewHolder(StreamRecyclerAdapter.java:47)
> 	at android.support.v7.widget.RecyclerView$Adapter.createViewHolder(Unknown Source:6)
> 	at android.support.v7.widget.RecyclerView$Recycler.getViewForPosition(Unknown Source:358)
> 	at android.support.v7.widget.RecyclerView$Recycler.getViewForPosition(Unknown Source:1)
> 	at android.support.v7.widget.LinearLayoutManager$LayoutState.next(Unknown Source:11)
> 	at android.support.v7.widget.LinearLayoutManager.layoutChunk(Unknown Source:3)
> 	at android.support.v7.widget.LinearLayoutManager.fill(Unknown Source:47)
> 	at android.support.v7.widget.LinearLayoutManager.onLayoutChildren(Unknown Source:374)
> 	at android.support.v7.widget.RecyclerView.dispatchLayoutStep2(Unknown Source:44)
> 	at android.support.v7.widget.RecyclerView.dispatchLayout(Unknown Source:51)
> 	at android.support.v7.widget.RecyclerView.onLayout(Unknown Source:6)
> 	at android.view.View.layout(View.java:19586)
> 	at android.view.ViewGroup.layout(ViewGroup.java:6053)

Can you take a look, Mike?
Flags: needinfo?(michael.l.comella)
tracking-fennec: --- → ?
Flags: needinfo?(michael.l.comella)
Whiteboard: [mobileAS]
Assignee: nobody → michael.l.comella
Regression from bug 1400397: in that bug we changed the (Context, Attributeset) constructor to three arguments for StreamOverridablePageIconLayout [1]. This constructor is expected to get called by `BrowserApp.onCreateView`, but it apparently isn't for a small number of users.

[1]: http://searchfox.org/mozilla-central/rev/d08b24e613cac8c9c5a4131452459241010701e0/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/StreamOverridablePageIconLayout.java#81
Blocks: 1400397
It looks like all crashes are API 26 (android O).
tracking-fennec: ? → ---
Iteration: --- → 1.30
Priority: -- → P1
Things I noticed:

1) `onCreateView(String, Context, AttributeSet)` is apparently for pre-HC devices: "This implementation does nothing and is for pre-HONEYCOMB apps. Newer apps should use onCreateView(View, String, Context, AttributeSet)." [1] is the most recent constructor.

2) I can't reproduce this on an Android O emulator.

[1]: https://developer.android.com/reference/android/support/v4/app/FragmentActivity.html#onCreateView(android.view.View,%20java.lang.String,%20android.content.Context,%20android.util.AttributeSet)
I checked with Snorp, who has a physical 6P and Pixel with Android O: he is also unable to reproduce.

Perhaps these views are getting inflated through an alternate code path (e.g. on rotation? when resuming from OOM?).
Looking at the last invocation of createViewFromTag (in the stack trace), here's the source [1]:

770            View view;
771            if (mFactory2 != null) {
772                view = mFactory2.onCreateView(parent, name, context, attrs);
773            } else if (mFactory != null) {
774                view = mFactory.onCreateView(name, context, attrs);
775            } else {
776                view = null;
777            }
778
779            if (view == null && mPrivateFactory != null) {
780                view = mPrivateFactory.onCreateView(parent, name, context, attrs);
781            }
782
783            if (view == null) {
784                final Object lastContext = mConstructorArgs[0];
785                mConstructorArgs[0] = context;
786                try {
787                    if (-1 == name.indexOf('.')) {
788                        view = onCreateView(parent, name, attrs);
789                    } else {
790                        view = createView(name, null, attrs);

We're throwing in createView, at the bottom, but on 772 and 774, our `onCreateView` methods should get called first. Either:

1) mFactory is not set properly (or the intended behavior has changed), for whatever reason
2) Our use of onCreateView is incorrect so it's not finding the class name correctly (Proguard?).

We may be able to resolve 1) by using the mFactory2 form of onCreateView.

[1]: http://androidxref.com/8.0.0_r4/xref/frameworks/base/core/java/android/view/LayoutInflater.java#790
ProGuard doesn't seem like it'd be the problem: neither BrowserToolbar or TabsPanel.TabsLayout does anything special for ProGuard.

Also, for the other names that use onCreateView:
- BrowserToolbar is an abstract class (but has a Context, AttributeSet constructor)
- TabsPanel.TabsLayout is an interface
Continuing with the source code, mFactory2 gets set in:
- LayoutInflater.setFactory2 [1]
- LayoutInflater(LayoutInflater, Context) [2] (a copy constructor)

Assuming we're not dealing with a copy, setFactory2 is called from:
- LayoutInflaterCompatBaseImpl.setFactory2 [3]
- LayoutInflaterCompatApi21Impl.setFactory2 & setFactory (overrides previous) [4]
- LayoutInflaterCompat.setFactory2 [5] (calls out to ^ above) [5]

Looking at the Nougat code (one API level down), setFactory2 is called from completely different places! [6]
- LayoutInflaterCompatHC.setFactory [7]
- LayoutInflaterCompatLollipop.setFactory [8]

Because the code path has changed, I think it's possible there could be a bug in the new Android O implementation (the deprecated method would likely be less tested) and that switching to the other onCreateView could solve this crash.

[1]: http://androidxref.com/8.0.0_r4/xref/frameworks/base/core/java/android/view/LayoutInflater.java#312
[2]: http://androidxref.com/8.0.0_r4/xref/frameworks/base/core/java/android/view/LayoutInflater.java#219

[3]: http://androidxref.com/8.0.0_r4/xref/frameworks/support/compat/java/android/support/v4/view/LayoutInflaterCompat.java#102
[4]: http://androidxref.com/8.0.0_r4/xref/frameworks/support/compat/java/android/support/v4/view/LayoutInflaterCompat.java#130
[5]: http://androidxref.com/8.0.0_r4/xref/frameworks/support/compat/java/android/support/v4/view/LayoutInflaterCompat.java#179

[6]: http://androidxref.com/7.1.1_r6/search?q=setFactory2&defs=&refs=&path=&hist=&project=frameworks
[7]: http://androidxref.com/7.1.1_r6/xref/frameworks/support/compat/honeycomb/android/support/v4/view/LayoutInflaterCompatHC.java#51
[8]: http://androidxref.com/7.1.1_r6/xref/frameworks/support/compat/api21/android/support/v4/view/LayoutInflaterCompatLollipop.java#24
Without doing more source diving (and we may be hitting the point of diminishing returns), I think updating onCreateView to the new version could fix this problem.

That being said, considering the imminent merge and risk it carries to make speculative fixes, we could fix this in a "less correct way" by either:
- calling `setCache` after construction. It's messy to pass the instances into all of these views, however.
- Using a static cache for all instances. This would be fairly similar to the current implementation, except that the cache will live past the Activity, until the class is unloaded.
I decided to not make the speculative fix - I moved the upgrade to the new onCreateView to bug 1401779 so it can be independently reviewed and backed out in case of issues.
NI self to verify this gets into 57.
Flags: needinfo?(michael.l.comella)
Comment on attachment 8910539 [details]
Bug 1401733: Make StreamOverridablePageIconLayout caches static.

https://reviewboard.mozilla.org/r/181978/#review187360

Okay, I don't love the static cache either, but hopefully this is pretty small cache bc it's just url strings. Making it static *does* fix this problem, and since we're close to merge, a sure fix for a crash sounds good to me.
Attachment #8910539 - Flags: review?(liuche) → review+
Pushed by cliu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7bb527a1748f
Make StreamOverridablePageIconLayout caches static. r=liuche
FWIW, bug 1377819 is another unrelated crash where `BrowserApp.onCreateView` is apparently never called.
https://hg.mozilla.org/mozilla-central/rev/7bb527a1748f
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
(In reply to Michael Comella (:mcomella) from comment #11)
> NI self to verify this gets into 57.

Done.
Flags: needinfo?(michael.l.comella)
You need to log in before you can comment on or make changes to this bug.