Closed Bug 799617 Opened 12 years ago Closed 12 years ago

Screen goes black when starting Fennec from external URL or resuming session

Categories

(Firefox for Android Graveyard :: Toolbar, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox17 verified, firefox18 verified, firefox19 verified)

VERIFIED FIXED
Firefox 19
Tracking Status
firefox17 --- verified
firefox18 --- verified
firefox19 --- verified

People

(Reporter: bnicholson, Assigned: bnicholson)

References

Details

Attachments

(2 files, 2 obsolete files)

This was fixed awhile ago with bug 725428 by setting a white background color on the LayerView. Since then, this change got lost somewhere: https://hg.mozilla.org/mozilla-central/rev/3d3aff127856#l1.19. It won't work anyway, though, because the LayerView has been changed to be a FrameLayout, and the SurfaceView is now a child of LayerView rather than the LayerView itself. Setting the LayerView's background color won't help since the child sits on top of it, so we have to change the background of the child view instead.
STR: From a cold start, launch Fennec from an external URL. *Or* kill Fennec (either sigkill or OOM) and reopen it. In both of these cases, the screen is black for over 1 second before anything is drawn.
I've noticed that with just the background fix applied, there is still a flash of black before the first draw (though it's very brief). I remember noticing the same thing back when bug 799617 was working. Does the endDrawing() callback get executed before the actual draw happens? I was able to work around this by adding another state, PAINT_AFTER_SECOND, to the LayerView, which clears the white background after the second draw instead of the first. This doesn't seem like the best fix, but it works.
Attachment #669689 - Flags: review?(bugmail.mozilla)
(In reply to Brian Nicholson (:bnicholson) from comment #2) > I've noticed that with just the background fix applied, there is still a > flash of black before the first draw (though it's very brief). I remember > noticing the same thing back when bug 799617 was working. Does the > endDrawing() callback get executed before the actual draw happens? s/bug 799617/bug 725428/
(In reply to Brian Nicholson (:bnicholson) from comment #2) > Created attachment 669689 [details] [diff] [review] > Set SurfaceView background to white before drawing > > I've noticed that with just the background fix applied, there is still a > flash of black before the first draw (though it's very brief). I remember > noticing the same thing back when bug 799617 was working. Does the > endDrawing() callback get executed before the actual draw happens? > endDrawing should only be getting called after the composite is complete. Most likely this is caused by us compositing an empty frame or something like that. > I was able to work around this by adding another state, PAINT_AFTER_SECOND, > to the LayerView, which clears the white background after the second draw > instead of the first. This doesn't seem like the best fix, but it works. I'd like to dig a little to see if we can figure out why that empty frame is being drawn. Adding BenWa to see if he has any ideas.
Also, just to clarify: if it turns out that the empty frame will always be there, then I'm fine with this patch. I just don't want to end up in a situation where that empty frame is only *sometimes* there, because then we'll leave the view with a white background until the next frame is drawn, which might not be for a while.
I'm not familiar with that part of the code. pcwalton worked on the android views bits.
(In reply to Brian Nicholson (:bnicholson) from comment #2) > Created attachment 669689 [details] [diff] [review] > Set SurfaceView background to white before drawing > > I've noticed that with just the background fix applied, there is still a > flash of black before the first draw (though it's very brief). I remember > noticing the same thing back when bug 799617 was working. Do you have a specific page you're loading when you see this? I applied the background fix parts of your patch and ran: am start -a android.intent.action.VIEW -n org.mozilla.fennec_kats/.App -d "https://staktrace.com" and I don't see a black flash at all (whereas I do see a black flash on stock m-c without your patch). If I can repro the short flash that you're seeing I can try to attach gdb and see why the compositor is giving us a frame at that point.
Cleaned up the patch a bit (in case we decide to keep this approach). I think this check only needs to be done at startup, so there's no need to reset the paint state for every setFirstPaintViewport() call.
Attachment #669689 - Attachment is obsolete: true
Attachment #669689 - Flags: review?(bugmail.mozilla)
Attachment #669753 - Flags: review?(bugmail.mozilla)
(In reply to Kartikaya Gupta (:kats) from comment #7) > Do you have a specific page you're loading when you see this? I applied the > background fix parts of your patch and ran: > > am start -a android.intent.action.VIEW -n org.mozilla.fennec_kats/.App -d > "https://staktrace.com" I see it when I run this command (same site), but it doesn't always appear. I had to try 5 times before I finally saw it (where I did menu > Quit between each attempt). This is on a Droid RAZR, non-debug build.
(In reply to Brian Nicholson (:bnicholson) from comment #9) > > am start -a android.intent.action.VIEW -n org.mozilla.fennec_kats/.App -d > > "https://staktrace.com" > > I see it when I run this command (same site), but it doesn't always appear. > I had to try 5 times before I finally saw it (where I did menu > Quit > between each attempt). The fact that it doesn't happen every time worries me, because of what I said in comment #5. > > This is on a Droid RAZR, non-debug build. Is this the same mythical device you were seeing bug 751948 on? :) Do you see this flash on any other device? I think to be safe I would like to split this patch into two - one that just sets the background color on the child element, and a second one that does the state changes. We can land the first one right away but I'd like to investigate a bit more to see if the second one is really needed. Ideally what I want is the backtrace from the first composite to see what's triggering it but since that happens so early it's going to be hard to attach gdb in time. We might have to insert a sleep call somewhere to allow enough time to attach gdb.
Sets the background color on LayerView's child.
Attachment #669753 - Attachment is obsolete: true
Attachment #669753 - Flags: review?(bugmail.mozilla)
Attachment #670053 - Flags: review?(bugmail.mozilla)
Workaround for black screen flash on Droid RAZR.
Attachment #670053 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 670053 [details] [diff] [review] Set SurfaceView background to white before drawing, v3 [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: screen can flash black at startup 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 #670053 - Flags: approval-mozilla-aurora?
Attachment #670053 - Flags: approval-mozilla-beta?
Comment on attachment 670053 [details] [diff] [review] Set SurfaceView background to white before drawing, v3 Unless this is a recent regression in Fennec Native, let's only uplift this fix to FF18 on Aurora.
Attachment #670053 - Flags: approval-mozilla-beta?
Attachment #670053 - Flags: approval-mozilla-beta-
Attachment #670053 - Flags: approval-mozilla-aurora?
Attachment #670053 - Flags: approval-mozilla-aurora+
(In reply to Alex Keybl [:akeybl] from comment #16) > Comment on attachment 670053 [details] [diff] [review] > Set SurfaceView background to white before drawing, v3 > > Unless this is a recent regression in Fennec Native, let's only uplift this > fix to FF18 on Aurora. It's worth mentioning that this was in fact introduced in FF17. We've had this same issue before 17 (fixed by bug 725428), but something in 17 regressed it by changing the layout.
Comment on attachment 670053 [details] [diff] [review] Set SurfaceView background to white before drawing, v3 Re-noming to be sure (see comment 18).
Attachment #670053 - Flags: approval-mozilla-beta- → approval-mozilla-beta?
That is worth mentioning, I'll approve for beta uplift.
Attachment #670053 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Depends on: 817064
Filed bug 817064 separately for the Droid RAZR issue.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
Whiteboard: [leave open]
Verified fixed on: - build: Firefox for Android 18.0 Beta 3 (2012-12-06), Firefox for Android 19.0a2 (2012-12-06), Firefox for Android 17.0 - deviceL Samsung Galaxy Nexus - OS: Android 4.0.2
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: