Last Comment Bug 781014 - Exiting full-screen gaia apps causes bad things to happen
: Exiting full-screen gaia apps causes bad things to happen
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Gonk (show other bugs)
: unspecified
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla17
Assigned To: Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
:
Mentors:
: 779350 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-07 15:02 PDT by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2012-08-20 23:44 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+


Attachments
Small gaia test (1.22 KB, patch)
2012-08-07 15:04 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Splinter Review
Make sure the toplevel gonk widget doesn't resize smaller than screen on exiting fullscreen (2.45 KB, patch)
2012-08-08 02:00 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
cpearce: review+
mwu.code: review+
Details | Diff | Splinter Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-07 15:02:08 PDT
If you open the gaia video app, then press HOME to return to homescreen, the homescreen gets into a very bad state where it only shows content in the lower left of the window.

I see a similar problem with a much simpler test case, which I'll attach here in a second.

I'm filing in Boot2Gecko for now because I'm not sure whether this is a gaia or a gecko bug.
Comment 1 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-07 15:04:19 PDT
Created attachment 649828 [details] [diff] [review]
Small gaia test

STR
 (1) cd $b2g/gaia && git apply 781014-test.patch && make install-gaia
 (2) Open "Template" app
 (3) Press "Toggle fullscreen"
 (4) Press HOME button

The homescreen gets into a bad state where the old Template still appears, but not fullscreen, and the animated sprite for the app on minimize is left on screen.  The homescreen stops responding to input.

cpearce, can you take a look?
Comment 2 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-07 15:05:49 PDT
Or I guess better to start with: does anyone know the expected sequence of events when minimizing a full-screen app?
Comment 3 PTO until Sep 5 NZ time; Chris Pearce (:cpearce) 2012-08-07 15:19:56 PDT
I suspect this is may have the same root cause as bug 775965, and I'm working on a patch to fix what I think is the root cause of that now.
Comment 4 PTO until Sep 5 NZ time; Chris Pearce (:cpearce) 2012-08-07 22:03:52 PDT
I don't think this is caused by the same root cause as bug 775965, my patch for that bug doesn't fix this one. :(
Comment 5 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-08 01:02:50 PDT
When exiting fullscreen, we do something bad

Breakpoint 1, nsWindow::Resize (this=0x146f9b0, aX=0, aY=0, aWidth=320, aHeight=480, aRepaint=true) at /home/cjones/mozilla/inbound/widget/gonk/nsWindow.cpp:396
(gdb) bt
#0  nsWindow::Resize (this=0x146f9b0, aX=0, aY=0, aWidth=320, aHeight=480, aRepaint=true) at /home/cjones/mozilla/inbound/widget/gonk/nsWindow.cpp:396
#1  0x417ef1aa in nsBaseWidget::MakeFullScreen (this=0x146f9b0, aFullScreen=false) at /home/cjones/mozilla/inbound/widget/xpwidgets/nsBaseWidget.cpp:726
[snip]
(gdb) f 1
#1  0x417ef1aa in nsBaseWidget::MakeFullScreen (this=0x146f9b0, aFullScreen=false) at /home/cjones/mozilla/inbound/widget/xpwidgets/nsBaseWidget.cpp:726
(gdb) p mOriginalBounds->Size()
$5 = {
  <mozilla::gfx::BaseSize<int, nsIntSize>> = {
    width = 320, 
    height = 480


The problem seems to be that during startup we resize to ... strange bounds sometimes.

Breakpoint 1, nsWindow::Resize (this=0x146f9b0, aX=0, aY=0, aWidth=480, aHeight=800, aRepaint=false) at /home/cjones/mozilla/inbound/widget/gonk/nsWindow.cpp:396
(gdb) c
Continuing.

Breakpoint 1, nsWindow::Resize (this=0x146f9b0, aX=0, aY=0, aWidth=320, aHeight=480, aRepaint=true) at /home/cjones/mozilla/inbound/widget/gonk/nsWindow.cpp:396
(gdb) 
Continuing.

Breakpoint 1, nsWindow::Resize (this=0x1208410, aX=0, aY=0, aWidth=320, aHeight=480, aRepaint=false) at /home/cjones/mozilla/inbound/widget/gonk/nsWindow.cpp:396
(gdb) 
Continuing.

Breakpoint 1, nsWindow::Resize (this=0x146f9b0, aX=0, aY=0, aWidth=480, aHeight=800, aRepaint=true) at /home/cjones/mozilla/inbound/widget/gonk/nsWindow.cpp:396
(gdb) 
Continuing.

Breakpoint 1, nsWindow::Resize (this=0x1208410, aX=0, aY=0, aWidth=480, aHeight=800, aRepaint=false) at /home/cjones/mozilla/inbound/widget/gonk/nsWindow.cpp:396

I think the 3rd->4th transition is when the XUL <window> finally decides, for real, to go fullscreen, and then we save the old mOriginalBounds there.  But that's obviously not the "real" original bounds.

I think this is a widget bug caused by bad assumptions of full-screen, namely that widgets never start fullscreen but only enter it after being non-fullscreen.  Then the problem is compounded by gonk having to redraw the entire screen, so there's no such thing as a "non-fullscreen widget".  A similar problem could be reproduced on "desktop", but it wouldn't lead to the same symptoms.  Maybe on fennec.

Two options to fix
 - make XUL <window> not start in fullscreen, but automatically resize to screen bounds (somehow)
 - override nsIWidget::MakeFullScreen(bool aFullScreen) for gonk nsWindow
Comment 6 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-08 01:20:46 PDT
> I think the 3rd->4th transition is when the XUL <window> finally decides, for real, to go
> fullscreen, and then we save the old mOriginalBounds there.  But that's obviously not the
> "real" original bounds.

By which I meant 3rd->5th ... the other stuff is the hidden window.  Sigh.
Comment 7 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-08 01:36:41 PDT
Poked a little more: the secondary resizes are for a DocumentViewerImpl child widget of the top-level <xul:window>.  They can be ignored since fullscreen state is being set on the toplevel window.
Comment 8 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-08 02:00:34 PDT
Created attachment 650000 [details] [diff] [review]
Make sure the toplevel gonk widget doesn't resize smaller than screen on exiting fullscreen

This fixes the resizing-and-going-nuts bug, but there's still what appears to be a b2g-chrome/gaia problem left over: when pressing HOME to exit the app, we cancel fullscreen, but the app content remains "open" onscreen, along with our hacky little window sprite.  I suspect that the window manager either isn't calling wasFullScreenFrame.setVisible(false), or it is and that's not having the intended effect.

Can fix that in a companion bug.
Comment 9 Michael Wu [:mwu] 2012-08-08 11:15:40 PDT
Comment on attachment 650000 [details] [diff] [review]
Make sure the toplevel gonk widget doesn't resize smaller than screen on exiting fullscreen

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

r=me with if statement inverted to reduce indentation.

::: widget/gonk/nsWindow.cpp
@@ +510,5 @@
>  
> +NS_IMETHODIMP
> +nsWindow::MakeFullScreen(bool aFullScreen)
> +{
> +    if (mWindowType == eWindowType_toplevel) {

if (mWindowType != eWindowType_toplevel) {
    NS_WARNING("MakeFullScreen() on a dialog or child widget?");
    return nsBaseWidget::MakeFullScreen(aFullScreen);
}
Comment 10 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-14 09:39:51 PDT
*** Bug 779350 has been marked as a duplicate of this bug. ***
Comment 11 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-14 23:14:41 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/3dbfe7ede3d4
Comment 12 :Ms2ger (⌚ UTC+1/+2) 2012-08-15 08:58:30 PDT
Comment on attachment 650000 [details] [diff] [review]
Make sure the toplevel gonk widget doesn't resize smaller than screen on exiting fullscreen

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

::: widget/gonk/nsWindow.h
@@ +90,5 @@
>          return NS_ERROR_NOT_IMPLEMENTED;
>      }
>      NS_IMETHOD ReparentNativeWidget(nsIWidget* aNewParent);
>  
> +    NS_IMETHOD MakeFullScreen(bool aFullScreen) /*MOZ_OVERRIDE*/;

What's up with the commented-out MOZ_OVERRIDE here?
Comment 13 Ed Morley [:emorley] 2012-08-15 09:47:55 PDT
https://hg.mozilla.org/mozilla-central/rev/3dbfe7ede3d4
Comment 14 David Flanagan [:djf] 2012-08-16 09:19:08 PDT
cjones: did you file a companion bug per comment #8?  May be related to cpierce's recent fullscreen notifications.  On leaving fullscreen, I've seen notifications that the system app has become fullscreen, which is obviously wrong...
Comment 15 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-16 10:37:55 PDT
(In reply to David Flanagan [:djf] from comment #14)
> cjones: did you file a companion bug per comment #8?

Tim fixed ( / slightly worked around ...) the bug in Gaia.

> May be related to
> cpierce's recent fullscreen notifications.  On leaving fullscreen, I've seen
> notifications that the system app has become fullscreen, which is obviously
> wrong...

That's sort of believable --- the system app is in a full-screen <xul:window> --- but I agree probably not expected based on the fullscreen API.

Will probably need cpearce to comment on that.
Comment 16 David Flanagan [:djf] 2012-08-16 10:41:10 PDT
cpearce: please see https://github.com/mozilla-b2g/gaia/issues/3512
Comment 17 PTO until Sep 5 NZ time; Chris Pearce (:cpearce) 2012-08-16 22:30:53 PDT
(In reply to David Flanagan [:djf] from comment #16)
> cpearce: please see https://github.com/mozilla-b2g/gaia/issues/3512

Comments and pull request to fix that issue in that issue.
Comment 18 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-08-20 23:15:39 PDT
I tried to remove the workaround I did at 
https://github.com/mozilla-b2g/gaia/issues/3013#issuecomment-7634235

but I don't think Gecko works even with patches from this bug.
Comment 19 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-08-20 23:44:02 PDT
I've just submit a pull request to Gaia that completely decouple mozRequestFullScreen from full screen apps. So fixing this bug is no longer required for Gaia to function correctly without workaround.

https://github.com/mozilla-b2g/gaia/pull/3626

Note You need to log in before you can comment on or make changes to this bug.