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

RESOLVED FIXED in Firefox 27

Status

()

defect
--
critical
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: aaronmt, Assigned: sriram)

Tracking

(Depends on 1 bug, {crash, steps-wanted})

27 Branch
Firefox 28
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox25 unaffected, firefox26- wontfix, firefox27 fixed, firefox28 fixed)

Details

(crash signature)

Attachments

(1 attachment)

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)
Posted 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: 6 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: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.