Closed Bug 833777 Opened 7 years ago Closed 7 years ago

UI fails to load intermittently

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 21
Tracking Status
firefox19 + affected
firefox20 + fixed
firefox21 + fixed
firefox22 + fixed

People

(Reporter: pwd.mozilla, Assigned: bnicholson)

Details

Attachments

(3 files)

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.
OS: Windows 7 → Android
Hardware: x86 → ARM
Can you post the logcat when this happens?
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.
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.
Attached file LogCat of a dead UI
Big log-cat, but I do see a whole lot of onTrimMemory() calls; anything else in there?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(bugmail.mozilla)
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.
Flags: needinfo?(bugmail.mozilla) → needinfo?(bnicholson)
(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:kats@mozilla.com) 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.
(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:kats@mozilla.com) 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.
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?
Flags: needinfo?(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.
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.
Attachment #712643 - Flags: review?(bnicholson)
Attachment #712643 - Flags: review?(bnicholson) → review+
Marking as [leave open] since this probably won't fix the problem.
Whiteboard: [leave open]
Assigning to myself since this is likely an issue with session restore.
Assignee: nobody → bnicholson
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.
Attachment #713281 - Flags: review?(mark.finkle)
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.
Attachment #713281 - Flags: review?(mark.finkle) → review+
(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
Whiteboard: [leave open]
https://hg.mozilla.org/mozilla-central/rev/740ae7d06796
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
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
Attachment #712643 - Flags: approval-mozilla-beta?
Comment on attachment 713281 [details] [diff] [review]
Ensure that a tab is selected after an OOM restore

See above
Attachment #713281 - Flags: approval-mozilla-beta?
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
Attachment #712643 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #713281 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(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 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.
You need to log in before you can comment on or make changes to this bug.