833777 - UI fails to load intermittently

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Paul [pwd]]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:21.0) Gecko/20130122 Firefox/21.0
Build ID: 20130122030956

Steps to reproduce:

I'm getting this a couple times a day. I'll go back into Fennec, having left a few tabs open, only to see a blank blue-ish/grey screen. The only thing you can do is kill it from the task manager and then load Fennec again. It makes Firefox look wholly unreliable.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Can you post the logcat when this happens?

Comment 2

[Paul [pwd]]

Sadly I have no access to logcat right now as I'm unrooted on Android 4.1. I was actually hoping that this has been reported previously and I was just unable to find the bug.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

If you have the android developer tools installed you can still use adb to get the logcat on an unrooted device. Alternatively if you can trigger a crash in FF that and submit a crash report we'll get the last bit of logcat, but last I checked the add-on to crash firefox didn't work very well. And I don't know if you'd be able to invoke it while in that state anyway.

Comment 4

[Paul [pwd]]

Attachment: LogCat of a dead UI

Comment 5

[Aaron Train [:aaronmt]]

Big log-cat, but I do see a whole lot of onTrimMemory() calls; anything else in there?

Comment 6

[Kartikaya Gupta (email:[e-mail])]

I see a couple of these:

02-05 11:00:15.742 E/GeckoConsole(30908): [JavaScript Error: "TypeError: BrowserApp.selectedTab is null" {file: "chrome://browser/content/browser.js" line: 7049}]

which (if my local browser.js is close to the one in that build) is happening in application-background or application-foreground handler in ActivityObserver.observe. This might explain why nothing gets painted on the screen. CC'ing bnicholson who is expert on all selectedTab-like things.

Comment 7

[Paul [pwd]]

(In reply to Aaron Train [:aaronmt] from comment #5)
> Big log-cat, but I do see a whole lot of onTrimMemory() calls; anything else
> in there?

Apologies, wanted to make sure I got everything. 

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #6)
> I see a couple of these:
> 
> 02-05 11:00:15.742 E/GeckoConsole(30908): [JavaScript Error: "TypeError:
> BrowserApp.selectedTab is null" {file: "chrome://browser/content/browser.js"
> line: 7049}]
> 
> which (if my local browser.js is close to the one in that build) is
> happening in application-background or application-foreground handler in
> ActivityObserver.observe. This might explain why nothing gets painted on the
> screen. CC'ing bnicholson who is expert on all selectedTab-like things.

I should mention that I'm unable to confirm whether this was a logcat for a completely unpainted (blue-grey) UI, or if it was a UI that had no content (web page) and wouldn't accept touches. Apologies, I have no idea why I didn't save the logcat with more information. "Dead UI" suggests the latter to me though.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

(In reply to Paul [sabret00the] from comment #7)
> (In reply to Aaron Train [:aaronmt] from comment #5)
> > Big log-cat, but I do see a whole lot of onTrimMemory() calls; anything else
> > in there?
> 
> Apologies, wanted to make sure I got everything. 
> 

Better to have too much log than too little :)

> (In reply to Kartikaya Gupta (email:[e-mail]) from comment #6)
> > 02-05 11:00:15.742 E/GeckoConsole(30908): [JavaScript Error: "TypeError:
> > BrowserApp.selectedTab is null" {file: "chrome://browser/content/browser.js"
> > line: 7049}]
>
> I should mention that I'm unable to confirm whether this was a logcat for a
> completely unpainted (blue-grey) UI, or if it was a UI that had no content
> (web page) and wouldn't accept touches. Apologies, I have no idea why I
> didn't save the logcat with more information. "Dead UI" suggests the latter
> to me though.

Yeah based on the error I would expect the latter behaviour as well.

Comment 9

[Brian Nicholson (:bnicholson)]

All tabs, including the initial tab, are loaded asynchronously after Gecko is ready, so there's a small window between Gecko:Ready and the first tab being created where the selected tab might be null. Generally, we handle this with null checks whenever we try to use the selected tab. 

The only check we have for Viewport:Change is isBrowserContentDocumentDisplayed(), but if there's no tab, it returns true: http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#572. Should this be false?

Comment 10

[Brian Nicholson (:bnicholson)]

(In reply to Brian Nicholson (:bnicholson) from comment #9)
> All tabs, including the initial tab, are loaded asynchronously after Gecko
> is ready, so there's a small window between Gecko:Ready and the first tab
> being created where the selected tab might be null. Generally, we handle
> this with null checks whenever we try to use the selected tab. 
> 
> The only check we have for Viewport:Change is
> isBrowserContentDocumentDisplayed(), but if there's no tab, it returns true:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/
> browser.js#572. Should this be false?

Sorry, not sure why I was looking at that callback. In our "application-background" observer, I think we might just be able to do a null check on the selected tab like we do a few lines below.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Attachment: Guard against null selectedTab in application-background handler

This should guard against selectedTab being null. However it seems like this should be really rare to hit considering the narrow window in which selectedTab is null. Based on that I'm not convinced this will fix the original issue (but it's still good to have).

Getting proper STR for the original issue would be good.

Comment 12

[Brian Nicholson (:bnicholson)]

Marking as [leave open] since this probably won't fix the problem.

Comment 13

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9672b9989f34

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/9672b9989f34

Comment 15

[Brian Nicholson (:bnicholson)]

Assigning to myself since this is likely an issue with session restore.

Comment 16

[Brian Nicholson (:bnicholson)]

Attachment: Ensure that a tab is selected after an OOM restore

Trying to reproduce this, I've run into cases where tabs fail to load at startup. This happens if, from a cold Fennec state, I repeatedly hit a homescreen shortcut, press home, press a homescreen shortcut, press home, etc. Doing this 5 times or so during startup will often prevent the first tab from loading, and even clicking on the tab doesn't make it load. When this happens, the session restore data becomes broken, and there is no selected index in the JSON.

The issue above can be fixed in its own bug, but we can guard against this (and hopefully other similar issues) by ensuring that a tab is selected when parsing the data at startup.

Comment 17

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 713281 [details] [diff] [review]
Ensure that a tab is selected after an OOM restore


>-        for (int i = 0; i < tabs.length(); i++) {
>+        boolean hasSelectedTab = false;
>+        int numTabs = tabs.length();

Could we save some code and just say:

  selected = (selected == -1 ? numTabs : selected);

Or are we trying to protect against some other case besides "selected: -1" ?

r+, but if we can use less code, i'd prefer it.

Comment 18

[Brian Nicholson (:bnicholson)]

(In reply to Mark Finkle (:mfinkle) from comment #17)
> Comment on attachment 713281 [details] [diff] [review]
> Or are we trying to protect against some other case besides "selected: -1" ?

I also wanted to guard against the possibility of having a selected tab where the tab isn't within the valid tab range. But yeah, that patch was more complicated than it should have been.

https://hg.mozilla.org/integration/mozilla-inbound/rev/740ae7d06796

Comment 19

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/740ae7d06796

Comment 20

[Brian Nicholson (:bnicholson)]

Comment on attachment 712643 [details] [diff] [review]
Guard against null selectedTab in application-background handler

[Approval Request Comment]
Bug caused by (feature/regressing bug #): unknown
User impact if declined: UI may get in broken state
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none

Comment 21

[Brian Nicholson (:bnicholson)]

Comment on attachment 713281 [details] [diff] [review]
Ensure that a tab is selected after an OOM restore

See above

Comment 22

[Alex Keybl [:akeybl]]

Could this be related to comments like "Downloaded the new update and now it wont work...will let me enter web address then nothing" in the 19.0 Play Store reviews?

/me wonders if we should consider this as a ride-along if we do build a 19.0.1

Comment 23

[Brian Nicholson (:bnicholson)]

(In reply to Alex Keybl [:akeybl] from comment #22)
> Could this be related to comments like "Downloaded the new update and now it
> wont work...will let me enter web address then nothing" in the 19.0 Play
> Store reviews?
> 
> /me wonders if we should consider this as a ride-along if we do build a
> 19.0.1

It's possible....if there's an error during session restore, that could break everything.

Comment 24

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/releases/mozilla-beta/rev/d60c19fb2c71
https://hg.mozilla.org/releases/mozilla-beta/rev/598f6f4dc746

Comment 25

[Alex Keybl [:akeybl]]

(In reply to Brian Nicholson (:bnicholson) from comment #23)
> (In reply to Alex Keybl [:akeybl] from comment #22)
> > Could this be related to comments like "Downloaded the new update and now it
> > wont work...will let me enter web address then nothing" in the 19.0 Play
> > Store reviews?
> > 
> > /me wonders if we should consider this as a ride-along if we do build a
> > 19.0.1
> 
> It's possible....if there's an error during session restore, that could
> break everything.

In that case, we'll leave it on the FF19 tracking list.