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)
Tracking
(firefox17 verified, firefox18 verified, firefox19 verified)
VERIFIED
FIXED
Firefox 19
People
(Reporter: bnicholson, Assigned: bnicholson)
References
Details
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
kats
:
review+
akeybl
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•12 years ago
|
||
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.
Assignee | ||
Comment 2•12 years ago
|
||
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)
Assignee | ||
Comment 3•12 years ago
|
||
(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/
Comment 4•12 years ago
|
||
(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.
Comment 5•12 years ago
|
||
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.
Comment 6•12 years ago
|
||
I'm not familiar with that part of the code. pcwalton worked on the android views bits.
Comment 7•12 years ago
|
||
(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.
Assignee | ||
Comment 8•12 years ago
|
||
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)
Assignee | ||
Comment 9•12 years ago
|
||
(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.
Comment 10•12 years ago
|
||
(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.
Assignee | ||
Comment 11•12 years ago
|
||
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)
Assignee | ||
Comment 12•12 years ago
|
||
Workaround for black screen flash on Droid RAZR.
Updated•12 years ago
|
Attachment #670053 -
Flags: review?(bugmail.mozilla) → review+
Assignee | ||
Comment 13•12 years ago
|
||
Whiteboard: [leave open]
Comment 14•12 years ago
|
||
Assignee | ||
Comment 15•12 years ago
|
||
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?
Assignee | ||
Updated•12 years ago
|
Attachment #670053 -
Flags: approval-mozilla-beta?
Assignee | ||
Updated•12 years ago
|
status-firefox17:
--- → affected
status-firefox18:
--- → affected
Comment 16•12 years ago
|
||
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+
Assignee | ||
Comment 17•12 years ago
|
||
Assignee | ||
Comment 18•12 years ago
|
||
(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.
Assignee | ||
Comment 19•12 years ago
|
||
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?
Comment 20•12 years ago
|
||
That is worth mentioning, I'll approve for beta uplift.
Updated•12 years ago
|
Attachment #670053 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee | ||
Comment 21•12 years ago
|
||
Assignee | ||
Comment 22•12 years ago
|
||
Filed bug 817064 separately for the Droid RAZR issue.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
Assignee | ||
Updated•12 years ago
|
Whiteboard: [leave open]
Comment 23•12 years ago
|
||
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
status-firefox19:
--- → verified
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•