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

VERIFIED FIXED in Firefox 17

Status

()

Firefox for Android
Graphics, Panning and Zooming
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: bnicholson, Assigned: bnicholson)

Tracking

unspecified
Firefox 19
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox17 verified, firefox18 verified, firefox19 verified)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

5 years ago
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

5 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

5 years ago
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?

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

5 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/
(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.
(Assignee)

Comment 8

5 years ago
Created attachment 669753 [details] [diff] [review]
Set SurfaceView background to white before drawing, v2

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

5 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.
(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

5 years ago
Created attachment 670053 [details] [diff] [review]
Set SurfaceView background to white before drawing, v3

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

5 years ago
Created attachment 670054 [details] [diff] [review]
Wait until second draw to clear white background

Workaround for black screen flash on Droid RAZR.
Attachment #670053 - Flags: review?(bugmail.mozilla) → review+
(Assignee)

Comment 13

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/46b4fcdd20e6
Whiteboard: [leave open]
https://hg.mozilla.org/mozilla-central/rev/46b4fcdd20e6
(Assignee)

Comment 15

5 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

5 years ago
Attachment #670053 - Flags: approval-mozilla-beta?
(Assignee)

Updated

5 years ago
status-firefox17: --- → affected
status-firefox18: --- → affected
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

5 years ago
http://hg.mozilla.org/releases/mozilla-aurora/rev/c932b7b0c55d
status-firefox17: affected → wontfix
status-firefox18: affected → fixed
(Assignee)

Comment 18

5 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

5 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?
That is worth mentioning, I'll approve for beta uplift.
status-firefox17: wontfix → affected
Attachment #670053 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(Assignee)

Comment 21

5 years ago
http://hg.mozilla.org/releases/mozilla-beta/rev/27eed0f4867f
status-firefox17: affected → fixed
(Assignee)

Updated

5 years ago
Depends on: 817064
(Assignee)

Comment 22

5 years ago
Filed bug 817064 separately for the Droid RAZR issue.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
(Assignee)

Updated

5 years ago
Whiteboard: [leave open]

Comment 23

5 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-firefox17: fixed → verified
status-firefox18: fixed → verified
status-firefox19: --- → verified
You need to log in before you can comment on or make changes to this bug.