1083173 - Content view sometimes blank on load

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Description

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

+++ This bug was initially created as a clone of Bug #1030523 +++

Bug 1030523 (hopefully) fixed an issue where the content view would get permanently stuck displaying blank white. However as mentioned in bug 1030523 comment 8 there is still an issue where the screen might show up as blank when switching into Fennec, and you have to switch to some other app and re-enter to kick it into working again. This bug is to track this issue.

STR:
1. Install a webapp (e.g. go to https://staktrace.com/util/webapp.php and click the button to install webapp)
2. Close all fennec instances on the device
3. Start the webapp but then immediately hit the home button to minimize the app
4. Wait a few seconds while gecko starts up
5. Click on the app again to bring it to the foreground

Expected:
The app shows content

Actual:
The app shows blank white. Using the app-switcher to re-select the app makes it render content again.

Comment 1

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

Since we have reliable STR now we should probably prioritize fixing this. I don't know if I'll have time to work on this in the near future though.

Comment 2

[Aaron Train [:aaronmt]]

I still see this (or bug 1030523) in regular content as well not necessisarily web-apps. I just updated to Nightly (10/20) from (10/19) and first site I loaded is blank and continues to be blank; same with about:

Comment 3

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

Does it remain blank even if you switch to another app and then switch back?

Comment 4

[Aaron Train [:aaronmt]]

Yes. I seem to have steps to reproduce too trying out here which hit this again.

 Launch Nightly under a new profile
 Sign-in to Sync
 Visit http://ubuntuforums.org

At least on my Nexus 5 (4.4.4), the site, and any other site afterwards is blank as well. Gecko seems hosed at this point.

Comment 5

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

Might be good to file a new bug for that then as it is probably a different issue and won't be fixed by this one.

Comment 7

[Eugen Sawin [:esawin]]

Attachment: blank-page-resume.patch

We never seem to notify Gecko about resuming the compositor because in this case, updateCompositor is only called while we have no valid compositor.

Comment 8

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

I don't think I understand why this patch works. Can you explain in a bit more detail what's going on? What I would expect to happen with the existing code using the STR in comment 0 is something along the lines of this:

1) Start the app, a surface becomes available
2) GLController.serverSurfaceChanged is called, which schedules a call to updateCompositor
3) updateCompositor runs, but gecko is not running yet so the if condition at the bottom fails
4) Hide the app, the surface is destroyed
5) GLController.serverSurfaceDestroyed is called. Since mCompositorCreated is false no pause event is sent
6) Gecko starts up
7) GLController.updateCompositor is called (from GeckoLayerClient) and it tries to create a compositor, however there is no surface available so this fails
8) Bring app back to foreground
9) same as steps 2 and 3 except this time compositor creation succeeds

Is this accurate? If so, in step 9 is the compositor paused? I would not expect that to be the case, but that seems to be the case your patch would handle.

Comment 9

[Eugen Sawin [:esawin]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #8)
> I don't think I understand why this patch works. Can you explain in a bit
> more detail what's going on? What I would expect to happen with the existing
> code using the STR in comment 0 is something along the lines of this:
> 
> 1) Start the app, a surface becomes available
> 2) GLController.serverSurfaceChanged is called, which schedules a call to
> updateCompositor
> 3) updateCompositor runs, but gecko is not running yet so the if condition
> at the bottom fails
> 4) Hide the app, the surface is destroyed
> 5) GLController.serverSurfaceDestroyed is called. Since mCompositorCreated
> is false no pause event is sent
> 6) Gecko starts up
> 7) GLController.updateCompositor is called (from GeckoLayerClient) and it
> tries to create a compositor, however there is no surface available so this
> fails
> 8) Bring app back to foreground
> 9) same as steps 2 and 3 except this time compositor creation succeeds
> 
> Is this accurate? If so, in step 9 is the compositor paused? I would not
> expect that to be the case, but that seems to be the case your patch would
> handle.

Here are my complete logs: https://gist.github.com/eamsen/696d1d9000b4139bef45

I don't log when the compositor is being created, but it obviously happens after calling updateCompositor when resuming.

Comment 10

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

So those logs seem to match my expectations, see my mapping from my steps to line numbers in your gistfile:

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #8)
> 1) Start the app, a surface becomes available

this is line 8

> 2) GLController.serverSurfaceChanged is called, which schedules a call to
> updateCompositor

this is line 9

> 3) updateCompositor runs,

line 20

> but gecko is not running yet so the if condition
> at the bottom fails

line 28

> 4) Hide the app, the surface is destroyed
> 5) GLController.serverSurfaceDestroyed is called. Since mCompositorCreated
> is false no pause event is sent

line 33

> 6) Gecko starts up

line 36

> 7) GLController.updateCompositor is called (from GeckoLayerClient) and it

line 37

> tries to create a compositor, however there is no surface available so this
> fails

line 44

> 8) Bring app back to foreground

line 51

> 9) same as steps 2 and 3 except this time compositor creation succeeds

lines 58 onwards, compositor getting created somewhere around lines 69-72.

> If so, in step 9 is the compositor paused? I would not
> expect that to be the case, but that seems to be the case your patch would
> handle.

This still isn't clear to me. Can you add additional logging around the mPaused variable in gfx/layers/ipc/CompositorParent.cpp to see when it gets flipped and why? If mPaused remains false throughout, then it might be that the call to resume you added is just kicking the compositor to schedule a composite [1] and that's all that's needed. In which case you should be able to achieve the same effect by scheduling a composite via [2] instead of "resuming" the compositor.

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/CompositorParent.cpp?rev=02e93e0c09de#621
[2] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoAppShell.java?rev=99d578d3c8a1#280

Comment 11

[Eugen Sawin [:esawin]]

Here are the logs with additional CompositorParent logging: https://gist.github.com/eamsen/56eb7f4867b15e0b3471

You're right about mPaused remaining false unless you hide and resume the app for a second time. Calling GeckoAppShell::scheduleComposite on compositorCreated does not trigger a nsWindow::redrawAll and doesn't seem to help in this case.

Also, according to code comments, asking for Gecko to resume the compositor can cause flickering/black frames, so I assume it's safer to just call resumeCompositor instead in my patch (will update it accordingly).

Please let me know if you have further concerns or ideas.

Comment 12

[Eugen Sawin [:esawin]]

Attachment: blank-page-resume.patch

Comment 13

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

Ah, I understand what's going on now. With your first patch I assumed that sending the resume event would actually trigger the resume code in the compositor, which is not the case at all; it just triggers a call to win->RedrawAll. That explains the observed behaviour then - gecko actually paints the content while the app is in the background, but that paint is lost because the compositor hasn't yet been initialized. When we bring the app to the foreground the compositor comes up but we don't do a repaint, and so the screen just remains blank. The critical part of your patches (should have been obvious had I looked more carefully) was the call to RedrawAll.

However, I think that instead of doing this from the java GL controller, which involves an extra round trip to C++ code, we should just be able to do it from C++ code at the end of the compositor creation, i.e. just before the break at http://mxr.mozilla.org/mozilla-central/source/widget/android/nsWindow.cpp?rev=138515da8c06#934. And actually the best way to do it would be to swap the COMPOSITOR_PAUSE and COMPOSITOR_RESUME hunks, and just let the COMPOSITOR_CREATE case statement fall through into _RESUME. That way if sCompositorPaused is false after the creation attempt then it won't do the RedrawAll, which is what we want.

Comment 14

[Eugen Sawin [:esawin]]

Attachment: blank-page-resume.patch

I haven't thought about doing it there directly for some reason, but that works and seems like the cleaner solution, thanks!

Comment 15

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

Comment on attachment 8518168 [details] [diff] [review]
blank-page-resume.patch

Review of attachment 8518168 [details] [diff] [review]:
-----------------------------------------------------------------

Perfect, thanks!

Comment 16

[Eugen Sawin [:esawin]]

Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=249144b8bd98

https://hg.mozilla.org/integration/mozilla-inbound/rev/663a6bc26926

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/663a6bc26926

Comment 18

[Aaron Train [:aaronmt]]

Should we get this on 35?

http://www.reddit.com/r/firefox/comments/2nio62/android_firefox_displays_white_page_instead_of/

Comment 19

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

If they report that it's fixed on nightly then yeah it would be worth uplifting both bug 1030523 and this one.

Comment 20

[Kevin Brosnan [Ex-Mozilla]]

[Tracking Requested - why for this release]: fixes a bad issue.

Comment 21

[James Willcox (:snorp) ([e-mail]) (he/him)]

Eugen, can you request beta uplift please?

Comment 22

[Eugen Sawin [:esawin]]

Comment on attachment 8518168 [details] [diff] [review]
blank-page-resume.patch

Approval Request Comment
[Feature/regressing bug #]: page loading
[User impact if declined]: pages and web apps fail to load, show blank screen
[Describe test coverage new/current, TBPL]: has been on Nightly for a month
[Risks and why]: none, resuming compositor on creation has no other side effects
[String/UUID change made/needed]: none

Comment 23

[Sylvestre Ledru [:Sylvestre]]

Eugen, 35 is marked as affected and it is currently beta. Any reason why you didnt' request the uplift for it?

Comment 24

[Eugen Sawin [:esawin]]

Comment on attachment 8518168 [details] [diff] [review]
blank-page-resume.patch

(In reply to Sylvestre Ledru [:sylvestre] from comment #23)
> Eugen, 35 is marked as affected and it is currently beta. Any reason why you
> didnt' request the uplift for it?
No, there isn't, sorry :)

Approval Request Comment
[Feature/regressing bug #]: page loading
[User impact if declined]: pages and web apps fail to load, show blank screen
[Describe test coverage new/current, TBPL]: has been on Nightly for a month
[Risks and why]: none, resuming compositor on creation has no other side effects
[String/UUID change made/needed]: none

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c67b5306f2a5

Comment 26

[u421692]

Can still reproduce this issue with the steps from comment 0 on Firefox 35 Beta 4 on Samsung Galaxy Nexus(Android 4.2.1). Sometimes Firefox crashes with:
https://crash-stats.mozilla.com/report/index/e4e50013-0be3-4b15-9069-a97f32141217

Comment 27

[Eugen Sawin [:esawin]]

(In reply to Mihai Pop from comment #26)
> Can still reproduce this issue with the steps from comment 0 on Firefox 35
> Beta 4 on Samsung Galaxy Nexus(Android 4.2.1). Sometimes Firefox crashes
> with:
> https://crash-stats.mozilla.com/report/index/e4e50013-0be3-4b15-9069-
> a97f32141217

The crash signature doesn't look related to this, that's bug 1112614.
Can you still reproduce this?

Comment 28

[u421692]

Tested on latest Nightly on Samsung Galaxy Nexus (Android 4.2.1) and Nexus 7 (Android 5.0.1).
I wasn't able to reproduce the crash, but still able to reproduce the blank screen after running a random webapp from marketplace.
Here is the logcat:
http://pastebin.mozilla.org/8259760

Comment 29

[James Willcox (:snorp) ([e-mail]) (he/him)]

Reopening per comment #28

Comment 30

[James Willcox (:snorp) ([e-mail]) (he/him)]

Actually, leaving this one fixed and will clone (yet another) instance.

Comment 31

[Eugen Sawin [:esawin]]

It's not reproducing for me on Nightly with Nexus 7 Android 4.4. Given the nature of this bug, I doubt that we are hitting the same issue.

Comment 32

[Aaron Train [:aaronmt]]

I suspect in cases like this it's in scenarios where Firefox is open and a device is left idle overnight and picked up the next day and something happens to Gecko.