Closed Bug 852526 Opened 9 years ago Closed 9 years ago

Launching a web apps for the first time displays white screen

Categories

(Firefox for Android Graveyard :: Toolbar, defect)

22 Branch
ARM
Android
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 22

People

(Reporter: paul.feher, Assigned: cwiiis)

References

Details

Attachments

(3 files, 3 obsolete files)

Nightly Fennec 16.0a1 (2013-03-19)
Devices: Samsung Galxy SII (Android 4.0.3), Samsung Galaxy Tab (4.0.4), HTC Desire Z (Android 2.3.3)

Steps to Reproduce:

1. Open about:apps
1. Install a web app(ex Twitter)
3. Open the web app.

Expected:
The web app opens correctly.

Actual:
A white screen appears when opening the web app. After tapping the home screen button and reopening the web app the app is loaded correctly.
Sorry, this is reproducible on Nightly 22.0a1 (2013-03-19)
You sure it's not crashing? I havn't been able to launch web-apps for a week now, bug 844895

Can you check the logcat?
Attached file Log File
(In reply to Paul Feher from comment #3)
> Created attachment 726663 [details]
> Log File

I see no crashes in the log. If you rotate the device, does the screen paint the webapp?
Yes, after changing orientation the webapp is painted properly
Oh snap, I see something related to this on my Motorola Droid Bionic (Android 2.3.4) Nightly (03/19), in about:config, resetting a preference wont update the screen unless I rotate.
See Also: → 852507
Probably related to this:

03-20 16:43:04.035 I/CompositorParent(26499): Unable to renew compositor surface; remaining in paused state
03-20 16:43:04.035 I/CompositorParent(26499): Unable to renew compositor surface; remaining in paused state
03-20 16:43:04.035 D/GeckoLayerClient(26499): Window-size changed to (480,430)
03-20 16:43:04.035 D/PowerManagerService( 1974): onSensorChanged: light value: 1000
03-20 16:43:04.080 W/WifiStateTracker( 1974): getNetworkInfo : NetworkInfo: type: WIFI[], state: CONNECTED/CONNECTED, reason: (unspecified), extra: (none), roaming: false, failover: false, isAvailable: true
03-20 16:43:04.095 E/libEGL  (26499): eglMakeCurrent:629 error 3009 (EGL_BAD_MATCH)

I will try to reproduce and debug if I can.
Also, can you let me know exactly what you did to reproduce this? e.g. after installing Twitter, did you exit fennec before opening twitter, or did you just put it in the background? did you do any device rotations during that time? as much detail as possible would be appreciated.
I see the same thing on my Motorola Droid Bionic (Android 2.3.4)

In the odd case where I don't crash: new profile, launch Nightly, visit the marketplace, install Twitter and launch Twitter
tracking-fennec: --- → ?
Component: Web Apps → Graphics, Panning and Zooming
QA Contact: aaron.train
I've tried several methods first launching the app directly by pressing the  launch button immediately after the app was installed without closing or minimizing the application and second by closing firefox and launch the app from the home screen. The result  was the same.
I can reproduce the blank screen that shows up after the splash screen goes away. The blank screen is replaced by the twitter sign in page on rotation or home/re-entry. However I don't see the same logcat output so maybe it's a different problem.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #11)
> I can reproduce the blank screen that shows up after the splash screen goes
> away. The blank screen is replaced by the twitter sign in page on rotation
> or home/re-entry. However I don't see the same logcat output so maybe it's a
> different problem.

Sounds exactly like what I see with my installed webapps.
This is a regression from bug 850690; the compositor thinks the EGL surface size is 0x0. I guess the codepath that was removed was needed after all. :(
Blocks: 850690
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #13)
> This is a regression from bug 850690; the compositor thinks the EGL surface
> size is 0x0. I guess the codepath that was removed was needed after all. :(

Well, at least we know, that makes fixing it a bit easier... Perhaps I can reinstate the old fix, with a fix on top to stop it from squashing subsequent resizes.
Assignee: nobody → chrislord.net
Blocks: 851861
Status: NEW → ASSIGNED
Duplicate of this bug: 852786
Reading the code, could the fix be as simple as just setting the surface size on compositor creation? From a cursory look, it seems that doesn't happen...
So I think this happens because we don't implement GetBounds in nsWindow and that's what's used to initialise the compositor size when it gets created. If we return gAndroidBounds in GetBounds, this ought to stop a 0x0 surface size when the window size is set before the compositor gets created. Writing/testing a patch now.
Actually, it isn't as simple as that - this could happen if the compositor was created by the non-main window though I guess, so I still think overriding GetBounds might be a reasonable thing to do, or otherwise adding an Android-specific #ifdef if that causes bad behaviour.
Oh if only it were as simple as I thought... Either way, pretty easy to reproduce, will track down.
Attached patch Set surface size on startup (obsolete) — Splinter Review
So I had the right idea, but the wrong reasoning. It seems the compositor is always created at 0x0 at the moment, as there's (almost certainly) no chance that Gecko will process the resize event we send on startup before we create the compositor.

Calling a resume after creating with a size fixes this for me. This is the smallest fix, but perhaps we should alter the resumeComposition event to always take a size, which would involve fewer round-trips - I'm not sure if we'd want this or not, so feel free to r- and say that we do if you'd prefer that.
Attachment #727064 - Flags: review?(bugmail.mozilla)
So I implemented this other way of doing it (adding width/height properties to the resume event), and that doesn't fix this... So I think that webapps are ending up with a paused compositor? Investigating.
This applies over the previous patch and basically does the same thing, but with one less round-trip.

Ends up there were two probems;

1- The compositor has a 0x0 size on creation, and may not necessarily get a size again until the surface changes for an unrelated reason
2- nsWindow doesn't realise that after creation, the compositor is unpaused

Fixing these two fixes the issue in the same way as part 1, but with fewer round-trips.
Attachment #727103 - Flags: review?(bugmail.mozilla)
Duplicate of this bug: 852507
Comment on attachment 727103 [details] [diff] [review]
Proper fix for surface size mismatch/compositor pause on start

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

::: mobile/android/base/gfx/GLController.java
@@ +159,5 @@
>              }
>          });
>      }
>  
> +    void createCompositor(int width, int height) {

This is unneeded; mWidth and mHeight will already have been set

@@ +164,4 @@
>          ThreadUtils.assertOnUiThread();
>  
> +        mWidth = width;
> +        mHeight = height;

This is unneeded

@@ +183,5 @@
>          }
>      }
>  
> +    void createCompositor() {
> +        createCompositor(mWidth, mHeight);

This is unneeded

::: mobile/android/base/gfx/GeckoLayerClient.java
@@ +142,5 @@
> +                // that the above resize event would have been handled before
> +                // this point, and otherwise the compositor surface size is
> +                // only updated on surface size changes and resumes, neither of
> +                // which are guaranteed to happen before the view is presented.
> +                mView.getGLController().createCompositor(mWindowSize.width, mWindowSize.height);

This change (and the previous patch) are unneeded.

::: widget/android/nsWindow.cpp
@@ +894,5 @@
> +                win->CreateLayerManager();
> +                sCompositorPaused = false;
> +
> +                win->mBounds.width = width;
> +                win->mBounds.height = height;

As discussed on IRC, I don't really like this code. I realize that we need a hack somewhere we interact with the window bounds because that code is pretty messed up, but I would prefer the hack to be a call to ScheduleResumeComposition which should have fewer (and better-understood) side effects.

::: widget/android/nsWindow.h
@@ +221,5 @@
>                               const nsIntPoint &refPoint);
>      void DispatchGestureEvent(uint32_t msg, uint32_t direction, double delta,
>                                const nsIntPoint &refPoint, uint64_t time);
>      void HandleSpecialKey(mozilla::AndroidGeckoEvent *ae);
> +    void CreateLayerManager();

Spurious change? Not sure why you moved this line.

::: widget/xpwidgets/nsBaseWidget.cpp
@@ +864,5 @@
>    renderToEGLSurface = true;
>  #endif
>    nsIntRect rect;
>    GetBounds(rect);
> +  printf_stderr("Creating %dx%d compositor\n", rect.width, rect.height);

Take this out
Attachment #727103 - Flags: review?(bugmail.mozilla) → review-
Attachment #727064 - Flags: review?(bugmail.mozilla) → review-
Attached patch Fix compositor creation (obsolete) — Splinter Review
This creates a new create-compositor event that takes a width/height and makes sure the paused state is correct after creation. Benefit, creation no longer has an unnecessary RedrawAll and doesn't require a synchronous composite.

The confusing mBounds stuff has been removed and replaced by an extra vfunc on nsBaseWidget for creating a compositor with a size.
Attachment #727064 - Attachment is obsolete: true
Attachment #727103 - Attachment is obsolete: true
Attachment #727210 - Flags: review?(bugmail.mozilla)
kats noticed that I didn't add the new create event to the list of events that get moved to the front of the queue in nsAppShell.
Attachment #727210 - Attachment is obsolete: true
Attachment #727210 - Flags: review?(bugmail.mozilla)
Attachment #727260 - Flags: review?(bugmail.mozilla)
Comment on attachment 727260 [details] [diff] [review]
Fix compositor creation v2

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

Much better, thanks! Just the one thing below that needs to be fixed, r=me with that, assuming try is all green.

::: widget/android/nsAppShell.cpp
@@ +648,5 @@
>              {
>                  uint32_t i = 0;
>                  while (i < mEventQueue.Length() &&
>                         (mEventQueue[i]->Type() == AndroidGeckoEvent::COMPOSITOR_PAUSE ||
>                          mEventQueue[i]->Type() == AndroidGeckoEvent::COMPOSITOR_RESUME)) {

This while condition also needs to be updated. I don't think the bad code path should get hit in practice but better to update it anyway.
Attachment #727260 - Flags: review?(bugmail.mozilla) → review+
Thanks for the thorough review, this is a much nicer patch than the original versions - try was green even without the above mentioned fix, but fixed and pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/e70bea1fd166
https://hg.mozilla.org/mozilla-central/rev/e70bea1fd166
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
Verified fixed on:
-build: Firefox for Android 23.0a1 (2013-04-08)
-device: Samsung Galaxy S II
-OS: Android 4.0.3
Status: RESOLVED → VERIFIED
tracking-fennec: ? → ---
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.