781014 - Exiting full-screen gaia apps causes bad things to happen

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Gonk, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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]

Attachment: 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]

Or I guess better to start with: does anyone know the expected sequence of events when minimizing a full-screen app?

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

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

[Chris Pearce [:cpearce (Not reading bugmail)]]

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]

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]

> 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]

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]

Attachment: 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]]

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 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3dbfe7ede3d4

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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]]

https://hg.mozilla.org/mozilla-central/rev/3dbfe7ede3d4

Comment 14

[David Flanagan [:djf]]

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]

(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]]

cpearce: please see https://github.com/mozilla-b2g/gaia/issues/3512

Comment 17

[Chris Pearce [:cpearce (Not reading bugmail)]]

(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)]

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)]

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