Closed Bug 930162 Opened 7 years ago Closed 7 years ago

crash in java.lang.NullPointerException: at org.mozilla.gecko.home.TabMenuStrip.onPageScrolled(TabMenuStrip.java)

Categories

(Firefox for Android :: General, defect)

27 Branch
All
Android
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Firefox 28
Tracking Status
firefox25 --- unaffected
firefox26 - wontfix
firefox27 --- fixed
firefox28 --- fixed

People

(Reporter: aaronmt, Assigned: sriram)

References

(Depends on 1 open bug)

Details

(Keywords: crash, steps-wanted)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-d71598d5-6e58-4355-b5a9-2404d2131022.
=============================================================

java.lang.NullPointerException
	at org.mozilla.gecko.home.TabMenuStrip.onPageScrolled(TabMenuStrip.java:107)
	at org.mozilla.gecko.home.HomePager$1.onPageScrolled(HomePager.java:114)
	at android.support.v4.view.ViewPager.onPageScrolled(ViewPager.java:1647)
Attached patch PatchSplinter Review
There are no steps to reproduce. Hence don't know why any of these views could be null.
Just a preventive fix here.
Attachment #821902 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 821902 [details] [diff] [review]
Patch

Should this check be before the call to setScrollingData ? Just asking.
(In reply to Mark Finkle (:mfinkle) from comment #2)
> Comment on attachment 821902 [details] [diff] [review]
> Patch
> 
> Should this check be before the call to setScrollingData ? Just asking.

setScrollingData() sets the values for "fromTab" and "toTab", which is null for some reason.
We are cross checking what setScrollingData() did.
Comment on attachment 821902 [details] [diff] [review]
Patch

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

Again, not keen on doing these things without understand the underlying issue. But this check looks more harmless than the ones on the other bugs.

::: mobile/android/base/home/TabMenuStrip.java
@@ +103,5 @@
>          }
>  
>          setScrollingData(position, positionOffset);
>  
> +        if (fromTab == null || toTab == null) {

Please file a follow-up bug to rename:
fromTab -> mFromTab
toTab -> mToTab
progress -> mProgress
Attachment #821902 - Flags: review?(lucasr.at.mozilla) → review+
Depends on: 935169
https://hg.mozilla.org/mozilla-central/rev/fefaca8e866e
Assignee: nobody → sriram
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
155 reports I see, should get this uplifted maybe. Don't have steps to reproduce though to verify this.
Still crashed on the 2013110600 build I think the patch should have made it into that build as it was merged to m-c at 5 PM Pacific and the Nightly build was created in the early AM on the 6th. The crash is https://crash-stats.mozilla.com/report/index/8f54b9f2-ba57-445a-9409-aec6e2131106

Signature shifted slightly from Aaron's crash ID.

java.lang.NullPointerException
	at org.mozilla.gecko.home.TabMenuStrip.onPageScrolled(TabMenuStrip.java:107)
	at org.mozilla.gecko.home.HomePager$1.onPageScrolled(HomePager.java:114)
	at android.support.v4.view.ViewPager.onPageScrolled(ViewPager.java:1647)
	at android.support.v4.view.ViewPager.pageScrolled(ViewPager.java:1585)
	at android.support.v4.view.ViewPager.computeScroll(ViewPager.java:1550)
	at android.view.ViewGroup.drawChild(ViewGroup.java:2729)
	at android.view.ViewGroup.dispatchDraw(ViewGroup.java:2489)
	at android.view.View.getDisplayList(View.java:10444)
	at android.view.ViewGroup.drawChild(ViewGroup.java:2850)
	at android.view.ViewGroup.dispatchDraw(ViewGroup.java:2489)
	at android.view.View.getDisplayList(View.java:10444)
	at android.view.ViewGroup.drawChild(ViewGroup.java:2850)
	at android.view.ViewGroup.dispatchDraw(ViewGroup.java:2489)
	at android.view.View.draw(View.java:11010)
	at android.view.View.getDisplayList(View.java:10446)
	at android.view.ViewGroup.drawChild(ViewGroup.java:2850)
	at android.view.ViewGroup.dispatchDraw(ViewGroup.java:2489)
	at android.view.View.getDisplayList(View.java:10444)
	at android.view.ViewGroup.drawChild(ViewGroup.java:2850)
	at android.view.ViewGroup.dispatchDraw(ViewGroup.java:2489)
	at android.view.View.getDisplayList(View.java:10444)
	at android.view.ViewGroup.drawChild(ViewGroup.java:2850)
	at android.view.ViewGroup.dispatchDraw(ViewGroup.java:2489)
	at android.view.View.getDisplayList(View.java:10444)
	at android.view.ViewGroup.drawChild(ViewGroup.java:2850)
	at android.view.ViewGroup.dispatchDraw(ViewGroup.java:2489)
	at android.view.View.getDisplayList(View.java:10444)
	at android.view.HardwareRenderer$GlRenderer.draw(HardwareRenderer.java:842)
	at android.view.ViewRootImpl.draw(ViewRootImpl.java:1924)
	at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:1647)
	at android.view.ViewRootImpl.handleMessage(ViewRootImpl.java:2462)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:4533)
	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)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
See Also: → 926402
Can we get an update on whether this has actually shifted in volume?  Also any URLs that might help with STR?
Flags: needinfo?(aaron.train)
URLs are unlikely to be helpful for a UI crash. As expected about:home and about:blank are top URLs. 

I'm unwilling to make volume assessments in Nightly or Aurora. If you want to try this patch speculatively on the next beta there have been 163 crashes on Beta 2 over the last 7 days.

The about:blank being the top URL suggests this may happen when getting a URL from and external app or opening in a new tab? Are there any other places we create a tab with about:blank set?
There's not enough to show that this is worth considering on Beta if it's not even confirmed to fix the issue on Nightly - too speculative at this point and it's not a topcrasher.  Go ahead with a nom for Aurora if you want to try this in the early days of FF27 Betas and see if the data shows this helped.
Flags: needinfo?(sriram)
I tried double-tapping on History button. I couldn't reproduce it. I am fine with uplifting this as this is a safety check.
Flags: needinfo?(sriram)
Flags: needinfo?(aaron.train)
let's do it
(In reply to Aaron Train [:aaronmt] from comment #13)
> let's do it

Do what exactly? Didn't comment #8 state that the patch didn't actually fix the crash?
Err, saw the r+ and scrolled all the way down :)
(In reply to Aaron Train [:aaronmt] from comment #15)
> Err, saw the r+ and scrolled all the way down :)

I don't see this bug among the FF28 top crashers. I wonder if the patch has actually worked around this crash after all?
(In reply to Lucas Rocha (:lucasr) from comment #16)
> (In reply to Aaron Train [:aaronmt] from comment #15)
> > Err, saw the r+ and scrolled all the way down :)
> 
> I don't see this bug among the FF28 top crashers. I wonder if the patch has
> actually worked around this crash after all?

I agree. This crash no longer appears anywhere in the Fx28 crash list, but it's still #14 and #9 in Fx27 and Fx26, respectively. Let's uplift this patch.
Comment on attachment 821902 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Crashes
Testing completed (on m-c, etc.): No longer crashing on m-c
Risk to taking this patch (and alternatives if risky): Low risk patch
String or IDL/UUID changes made by this patch: none
Attachment #821902 - Flags: approval-mozilla-beta?
Attachment #821902 - Flags: approval-mozilla-aurora?
Attachment #821902 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 821902 [details] [diff] [review]
Patch

There's too much speculation and as Lucas pointed out in comment 4, we don't know the root cause. We've already built the final Beta and RC for FF26 and comment 8 shows we don't have a guarantee that landing this fixes the issue.  Not willing to take the risk of landing this without time to sit with users before ship.  Uplift to FF27 and it will be in the first beta next week.
Attachment #821902 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
I'm going to close this one given that this crasher disappeared from our top crashers in 28.
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.