1304777 - Crash in java.lang.NullPointerException: Attempt to invoke virtual method ''void android.widget.TextView.setTextColor(int)'' on a null object reference at org.mozilla.gecko.home.TabMenuStripLayout.onPageSelected(TabMenuStripLayout.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect, P1)

Description

[Kevin Brosnan [Ex-Mozilla]]

This bug was filed from the Socorro interface and is 
report bp-c721f644-9fb2-4f66-b556-786dd2160922.
=============================================================

java.lang.NullPointerException: Attempt to invoke virtual method 'void android.widget.TextView.setTextColor(int)' on a null object reference
	at org.mozilla.gecko.home.TabMenuStripLayout.onPageSelected(TabMenuStripLayout.java:99)
	at org.mozilla.gecko.home.TabMenuStrip.onPageSelected(TabMenuStrip.java:89)
	at org.mozilla.gecko.home.HomePager.setCurrentItem(HomePager.java:283)
	at org.mozilla.gecko.home.HomePager.access$600(HomePager.java:37)
	at org.mozilla.gecko.home.HomePager$ConfigLoaderCallbacks.onLoadFinished(HomePager.java:492)
	at android.support.v4.app.LoaderManagerImpl$LoaderInfo.callOnLoadFinished(Unknown Source)
	at android.support.v4.app.LoaderManagerImpl.initLoader(Unknown Source)
	at org.mozilla.gecko.home.HomePager.load(HomePager.java:230)
	at org.mozilla.gecko.BrowserApp.showHomePagerWithAnimator(BrowserApp.java:2763)
	at org.mozilla.gecko.BrowserApp.hideBrowserSearch(BrowserApp.java:2923)
	at org.mozilla.gecko.BrowserApp.access$1000(BrowserApp.java:195)
	at org.mozilla.gecko.BrowserApp$19.onStopEditing(BrowserApp.java:1209)
	at org.mozilla.gecko.toolbar.BrowserToolbar.stopEditing(BrowserToolbar.java:828)
	at org.mozilla.gecko.toolbar.BrowserToolbar.commitEdit(BrowserToolbar.java:813)
	at org.mozilla.gecko.BrowserApp.access$700(BrowserApp.java:195)
	at org.mozilla.gecko.BrowserApp$14.onCommit(BrowserApp.java:1152)
	at org.mozilla.gecko.toolbar.ToolbarEditText$KeyListener.onKey(ToolbarEditText.java:606)
	at android.view.View.dispatchKeyEvent(View.java:8884)
	at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1695)
	at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1695)
	at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1695)
	at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1695)
	at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1695)
	at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1695)
	at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1695)
	at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1695)
	at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1695)
	at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1695)
	at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1695)
	at com.android.internal.policy.impl.PhoneWindow$DecorView.superDispatchKeyEvent(PhoneWindow.java:2671)
	at com.android.internal.policy.impl.PhoneWindow.superDispatchKeyEvent(PhoneWindow.java:1787)
	at android.app.Activity.dispatchKeyEvent(Activity.java:2837)
	at android.support.v7.internal.view.WindowCallbackWrapper.dispatchKeyEvent(Unknown Source)
	at android.support.v7.app.AppCompatDelegateImplBase$AppCompatWindowCallbackBase.dispatchKeyEvent(Unknown Source)
	at com.android.internal.policy.impl.PhoneWindow$DecorView.dispatchKeyEvent(PhoneWindow.java:2549)
	at android.view.ViewRootImpl$ViewPostImeInputStage.processKeyEvent(ViewRootImpl.java:4662)
	at android.view.ViewRootImpl$ViewPostImeInputStage.onProcess(ViewRootImpl.java:4617)
	at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4175)
	at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:4228)
	at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:4194)
	at android.view.ViewRootImpl$AsyncInputStage.forward(ViewRootImpl.java:4304)
	at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:4202)
	at android.view.ViewRootImpl$AsyncInputStage.apply(ViewRootImpl.java:4361)
	at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4175)
	at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:4228)
	at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:4194)
	at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:4202)
	at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4175)
	at android.view.ViewRootImpl.deliverInputEvent(ViewRootImpl.java:6653)
	at android.view.ViewRootImpl.doProcessInputEvents(ViewRootImpl.java:6537)
	at android.view.ViewRootImpl.enqueueInputEvent(ViewRootImpl.java:6508)
	at android.view.ViewRootImpl$ViewRootHandler.handleMessage(ViewRootImpl.java:3739)
	at android.os.Handler.dispatchMessage(Handler.java:102)
	at android.os.Looper.loop(Looper.java:145)
	at android.app.ActivityThread.main(ActivityThread.java:6117)
	at java.lang.reflect.Method.invoke(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:372)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1399)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1194)


This is an order of magnitude higher crash than the number 2 crash. We need to address this ASAP.

Comment 1

[Marcia Knous [:marcia]]

Tracking 52+ for this higher volume Fennec crash.

Comment 2

[Gerry Chang [:gchang]]

Un track for 51 as the volume is low.

Comment 3

[Kevin Brosnan [Ex-Mozilla]]

[Tracking Requested - why for this release]: There are ~1000 Aurora Android users. If something is the number one top crash on beta 50 I fully expect it to be the top crash beta 51 if not fixed. Unfortunately the only way to get acceptable crash data for Firefox for Android is in the beta channel.

Comment 4

[Andrzej Hunt :ahunt]

This looks like it could be a side-effect of Bug 1251362 (removal of another homepanel).

It seems that migration might have a bug causing us to end up with no default homepanel, in which case we try to open panel=-1 at startup.

Comment 5

[Andrzej Hunt :ahunt]

Repro steps:

1. download beta 49, e.g.:
https://archive.mozilla.org/pub/mobile/releases/49.0b11/android-api-15/en-US/fennec-49.0b11.en-US.android-arm.apk

2. Make History the default home panel (via settings->general->home->history->set as default)

3. Install beta 50, e.g.
https://archive.mozilla.org/pub/mobile/releases/50.0b1/android-api-15/en-US/fennec-50.0b1.en-US.android-arm.apk

4. Start beta -> crash

Comment 6

[Andrzej Hunt :ahunt]

Interestingly, after 4: it's possible to start firefox again, and it does open history as the default panel. However every clean start (e.g. by force-killing the app, then reopening) results in the crash on startup. Opening settings->General->Home also results in a crash.

I have a patch ready, I'll try to get a beta build to test it on later locally.

Note that this is pretty much exactly what happened in Bug 1264136. We're unlikely to have any more home panel migrations anytime soon (we're as lean as it goes), so it's probably not worth spending time implementing checks that would catch these issues sooner - especially since ActivityStream *might* see the HomePager being removed completely.

Comment 7

[Andrzej Hunt :ahunt]

Attachment: Bug 1304777 - Part 1: Perform homeconfig cleanup to ensure default still exists

Bug 1251362 can result in there no default panel being set (this happens if the history
panel was the default panel before the panel migration). We can reuse the code from a previous
bug which sets history as the default panel if no other panel is set as default.

A second commit fixes the broken migration to ensure that history remains the default
homepanel, however this cleanup migration will help ensure existing clients get fixed.

Review commit: https://reviewboard.mozilla.org/r/81184/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81184/

Comment 8

[Andrzej Hunt :ahunt]

Attachment: Bug 1304777 - Part 2: Fix panel removal migration to not discard DEFAULT flag if set

If the replacement panel was DEFAULT, then the migration should retain the DEFAULT flag.
By trying to ensure visibility of the default panel, we accidentally discarded the DEFAULT flag.
This patch provides a minimal fix for not discarding the default flag. It might be better
to rearchitect the whole method, but a minimal solution is preferred for now since this requires
uplifting to beta.

Review commit: https://reviewboard.mozilla.org/r/81186/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81186/

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8794995 [details]
Bug 1304777 - Part 1: Perform homeconfig cleanup to ensure default still exists

https://reviewboard.mozilla.org/r/81184/#review79944

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8794997 [details]
Bug 1304777 - Part 2: Fix panel removal migration to not discard DEFAULT flag if set

https://reviewboard.mozilla.org/r/81186/#review79946

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/03843beb3dfd
Part 1: Perform homeconfig cleanup to ensure default still exists r=sebastian
https://hg.mozilla.org/integration/autoland/rev/aa465e8e3723
Part 2: Fix panel removal migration to not discard DEFAULT flag if set r=sebastian

Comment 12

[Andrzej Hunt :ahunt]

NI myself as a reminder to get this uplifted (after a few days on nightly perhaps).

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

In the channel meeting this morning, I was told that this crash accounts for 1/3rd of all Fennec crashes on 50.0b1. Tracked as blocking bug for Fx50.

Comment 14

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Barbara, Sebastian, at the channel meeting today we decided to block 50.0b2 on this crash due to it's severity. Could you please get a fix uplifted to Beta50 asap so we can include it in time for 50.0b4? 50.0b2 will be skipped. We don't plan to push any more Beta builds out until this issue is fixed on Fennec Beta50.

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/03843beb3dfd
https://hg.mozilla.org/mozilla-central/rev/aa465e8e3723

Comment 16

[Gerry Chang [:gchang]]

Track 51+ as this is a regression and crash.

Comment 17

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Andrzej, could you please nominate this for uplift to Aurora51, Beta50? 

Hi Kevin, this landed on Nightly52. Is there a way to get some verification on this one from Nightly?

Comment 18

[Andrzej Hunt :ahunt]

Comment on attachment 8794995 [details]
Bug 1304777 - Part 1: Perform homeconfig cleanup to ensure default still exists

Approval Request Comment
[Feature/regressing bug #]: Bug 1251362
[User impact if declined]: Crash when opening the HomePager (topsites/bookmarks/etc), which can happen at startup for some users. This happens for all users that had the history panel set as their default panel before upgrading to beta 50 from beta 49.
[Describe test coverage new/current, TreeHerder]: Manual testing.
[Risks and why]: Low risk: this patch reuses cleanup code that was already used to fix an almost identical crash that happened after another botched migration.
[String/UUID change made/needed]: none.

Comment 19

[Andrzej Hunt :ahunt]

Comment on attachment 8794997 [details]
Bug 1304777 - Part 2: Fix panel removal migration to not discard DEFAULT flag if set

Approval Request Comment
[Feature/regressing bug #]: Bug 1251362
[User impact if declined]: Crash when opening the HomePager (topsites/bookmarks/etc), which can happen at startup for some users. This happens for all users that had the history panel set as their default panel before upgrading to beta 50 from beta 49.
[Describe test coverage new/current, TreeHerder]: Manual testing.
[Risks and why]: Low risk: this patch fixes the migration code to preserve the DEFAULT flag if necessary, instead of discarding it unconditionally (which originally caused the crash)
[String/UUID change made/needed]: none.

Comment 20

[Sebastian Kaspari (:sebastian; :pocmo)]

Thanks!

Comment 21

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Andrzej, Sebastian, given how bad this regression was, do you think we can add some automated tests (is this only on update scenarios?) that catch this regression even before we push the Beta build out? Thanks!

Comment 22

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8794995 [details]
Bug 1304777 - Part 1: Perform homeconfig cleanup to ensure default still exists

Top crasher on Beta50, this should be in 50.0b4.

Comment 23

[Andrzej Hunt :ahunt]

(In reply to Ritu Kothari (:ritu) from comment #21)
> Hi Andrzej, Sebastian, given how bad this regression was, do you think we
> can add some automated tests (is this only on update scenarios?) that catch
> this regression even before we push the Beta build out? Thanks!

I've filed Bug 1306140 for that, should *hopefully* be done either tonight or tomorrow morning.

This only happens for update scenarios, but we can hopefully run through every possible update permutation in a test.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8de5082984bd
https://hg.mozilla.org/releases/mozilla-aurora/rev/d9015b8e5f6e

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/402809f101da
https://hg.mozilla.org/releases/mozilla-beta/rev/d10f05314bc9

Comment 26

[Andrzej Hunt :ahunt]

(In reply to Andrzej Hunt :ahunt from comment #23)
> (In reply to Ritu Kothari (:ritu) from comment #21)
> > Hi Andrzej, Sebastian, given how bad this regression was, do you think we
> > can add some automated tests (is this only on update scenarios?) that catch
> > this regression even before we push the Beta build out? Thanks!
> 
> I've filed Bug 1306140 for that, should *hopefully* be done either tonight
> or tomorrow morning.
> 
> This only happens for update scenarios, but we can hopefully run through
> every possible update permutation in a test.

I've got a patch ready that tests these migrations (Bug 1306140) - removing the crash-fix patches from this bug causes these tests to fail, with these patches the tests pass.

The tests should also stop us from ever hitting this issue again (since they enforce adding the required data for any new migrations), however I don't think it's likely that there will be any migrations anytime soon.

Comment 27

[Sebastian Kaspari (:sebastian; :pocmo)]

Awesome, thanks! I just reviewed it.

Comment 28

[Kevin Brosnan [Ex-Mozilla]]

Confirm that this fixes the issue. I do note that we re-enabled the history view for users that have disabled all the panels. Filed bug 1306784 for that, if we don't fix this for the beta 50 cycle we should wontfix it.

Comment 29

[u421692]

Verified as fixed on Nexus 6P (Android 7.0) on Fennec 50 Beta 4