1214388 - Homescreen receives 'sizechange' event when switching away from a landscape app

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

Description

[Adam Alldredge [:AdamA][Inactive]]

Attachment: logcat

Description:
If the user has the new homescreen enabled and exits an app that is landscape then the homescreen icons will be misplaced for a moment before correcting themselves.

Prerequisite:
New homescreen is turned on in settings

Repro Steps:
1) Update a Aries to 20151013110851
2) Open a landscape app (swoop is preinstalled and is landscape)
3) press home button
4) Observe homescreen

Actual:
the homescreen icons are misplaced 

Expected:
It is expected that the icons are correctly placed when exiting a landscape app

Environmental Variables:
Device: Aries 2.5 [Full Flash]
Build ID: 20151013110851
Gaia: d400cda6bf0f8b30dcf7d7d71bfa61f29a3f1588
Gecko: 607a236c229994df99766c005f9ec729532d7747
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 44.0a1 (2.5)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0

Repro frequency: 10/10
See attached: Video clip, logcat

Comment 1

[Adam Alldredge [:AdamA][Inactive]]

This issue DOES occur on Flame 2.5.

Environmental Variables:
Device: Flame 2.5 [Full Flash]
BuildID: 20151013030230
Gaia: d400cda6bf0f8b30dcf7d7d71bfa61f29a3f1588
Gecko: 607a236c229994df99766c005f9ec729532d7747
Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd
Version: 44.0a1 (2.5) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0

Result:
the homescreen icons are misplaced for a moment.
----------------------------------------
New homescreen is not present on Flame 2.2 so I did not check this issue there.

Comment 2

[Chris Lord [:cwiiis]]

I wonder whether we want to fix this at the system level (so that background apps don't change orientation), or at the homescreen level (handle landscape orientation in css)...

I'll take this though.

Comment 3

[Chris Lord [:cwiiis]]

I forget what exactly we said on IRC, but Etienne, as I understand it, the homescreen (and indeed background apps?) shouldn't rotate when the foreground app changes orientation, right? (feel free to elaborate as much as you like :))

Comment 4

[Etienne Segonzac (:etienne)]

(In reply to Chris Lord [:cwiiis] from comment #3)
> I forget what exactly we said on IRC, but Etienne, as I understand it, the
> homescreen (and indeed background apps?) shouldn't rotate when the
> foreground app changes orientation, right? (feel free to elaborate as much
> as you like :))

We need all AppWindow elements to change size on rotation change (for the edge gestures while in landscape to work), so the iframe gets resized.
But resizing the iframe while it's setVisible(false) should not trigger resize events in the app.

So we may have a race where the iframe element change sizes before we called setVisible(false). Or something might be broken and we're not calling setVisible(false) on the homescreen anymore... or calling setVisible(true) when we shouldn't.


But I'm just speculating :)

Comment 5

[Chris Lord [:cwiiis]]

Not a blocking issue.

Comment 6

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

The performance is bad in this case, Chris.  I saw the issue from bug 1179723, and the performance will still be bad after that patch lands.  Should I file a new bug in regards to the performance issue?

Comment 7

[Chris Lord [:cwiiis]]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #6)
> The performance is bad in this case, Chris.  I saw the issue from bug
> 1179723, and the performance will still be bad after that patch lands. 
> Should I file a new bug in regards to the performance issue?

Do you mean the performance shortly after hitting this is bad (due to the re-organisation of icons)? If so, we don't need another bug for that. This is on my backlog, I'd like to get it fixed.

Note, that this is a common problem with the system, as opposed to the homescreen - the homescreen is portrait and should not be getting sizechange events to landscape orientations in the background (which is what causes this issue).

Re-titling/changing component and unassigning for now, leaving n?me because I want to fix it, but maybe someone will get there before me.

Comment 8

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

(In reply to Chris Lord [:cwiiis] from comment #7)
> (In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from
> comment #6)
> > The performance is bad in this case, Chris.  I saw the issue from bug
> > 1179723, and the performance will still be bad after that patch lands. 
> > Should I file a new bug in regards to the performance issue?
> 
> Do you mean the performance shortly after hitting this is bad (due to the
> re-organisation of icons)? If so, we don't need another bug for that. This
> is on my backlog, I'd like to get it fixed.

Yes, thanks.  I won't file another bug; I'll just mark this as perf as well then.  I don't think it's really polish, though...

Comment 9

[Chris Lord [:cwiiis]]

App doesn't need to be fullscreen for this to happen.

Taking again, will see what I can do about this.

Comment 10

[Chris Lord [:cwiiis]]

This is the sequence of events of rotating a browser window to landscape, then pressing the home button (the browser window's name is 'Mozilla'):

I/GeckoConsole( 6175): Content JS LOG: Mozilla: resize() 
I/GeckoConsole( 6175): Content JS LOG: Mozilla: _resize() 
I/GeckoConsole( 6175): Content JS LOG: Mozilla: _resize() 
I/GeckoConsole( 6175): Content JS LOG: Default Home Screen: _resize() 
I/GeckoConsole( 6175): Content JS LOG: Default Home Screen: setVisible(true, undefined) 
I/GeckoConsole( 6175): Content JS LOG: Default Home Screen: setVisibleForScreenReader(true) 
I/GeckoConsole( 6175): Content JS LOG: Default Home Screen: showFrame 
I/Default Home Screen( 6388): Content JS LOG: Homescreen window resized to 590x286 
I/GeckoConsole( 6175): Content JS LOG: Default Home Screen: _resize() 
I/GeckoConsole( 6175): Content JS LOG: Default Home Screen: setVisible(true, undefined) 
I/GeckoConsole( 6175): Content JS LOG: Default Home Screen: setVisibleForScreenReader(true) 
I/GeckoConsole( 6175): Content JS LOG: Default Home Screen: setVisible(true, undefined) 
I/GeckoConsole( 6175): Content JS LOG: Default Home Screen: setVisibleForScreenReader(true) 
I/GeckoConsole( 6175): Content JS LOG: Default Home Screen: _resize() 
I/GeckoConsole( 6175): Content JS LOG: Mozilla: _resize() 
I/GeckoConsole( 6175): Content JS LOG: Default Home Screen: _resize() 
I/GeckoConsole( 6175): Content JS LOG: Mozilla: setVisible(false, undefined) 
I/GeckoConsole( 6175): Content JS LOG: Mozilla: setVisibleForScreenReader(false) 
I/GeckoConsole( 6175): Content JS LOG: Mozilla: hideFrame 
I/Default Home Screen( 6388): Content JS LOG: Homescreen window resized to 360x516

It looks like there's a fair amount of redundancy there, but I don't really know what's going on yet.

Comment 11

[Chris Lord [:cwiiis]]

Here's a more detailed, easier to read sequence of events:

www.mozilla.org: resize() 
www.mozilla.org: resize() 
www.mozilla.org: setVisible(true, undefined) 
www.mozilla.org: setVisibleForScreenReader(true) 
www.mozilla.org: resize() 
Default Home Screen: setVisible(false, undefined) 
Default Home Screen: setVisibleForScreenReader(false) 
Default Home Screen: hideFrame 
www.mozilla.org: setVisible(true, undefined) 
www.mozilla.org: setVisibleForScreenReader(true) 
www.mozilla.org: resize() 
www.mozilla.org: _resize() 

[Rotate phone to landscape]

Mozilla: resize() 
Mozilla: _resize() 
Default Home Screen: _handle__orientationchange(evt) 
Mozilla: _handle__orientationchange(evt) 
Mozilla: _resize call from orientationchange 
Mozilla: _resize() 

[Press home button]

Default Home Screen: _resize() 
Default Home Screen: setVisible(true, undefined) 
Default Home Screen: setVisibleForScreenReader(true) 
Default Home Screen: showFrame 

Homescreen window resized to 590x286 

Default Home Screen: _resize() 
Default Home Screen: setVisible(true, undefined) 
Default Home Screen: setVisibleForScreenReader(true) 
Default Home Screen: setVisible(true, undefined) 
Default Home Screen: setVisibleForScreenReader(true) 
Default Home Screen: _resize() 
Default Home Screen: _handle__orientationchange(evt) 
Mozilla: _handle__orientationchange(evt) 
Mozilla: _resize call from orientationchange 
Mozilla: _resize() 
Default Home Screen: _resize() 
Mozilla: setVisible(false, undefined) 
Mozilla: setVisibleForScreenReader(false) 
Mozilla: hideFrame 

Homescreen window resized to 360x516 


It's clear that the associated orientationchange event back to portrait is coming after we've made the frame visible. Now to find out why...

Comment 12

[Etienne Segonzac (:etienne)]

We might want to add some logs here [1] too.
And looks like we have a special case to lock in portrait _before_ we wake up the homescreen [2].

So either it's coming a bit too late, or (slightly more likely I think) we're a bit too eager to setVisible(true) the homescreen somewhere else in the code.
Wonder if this issue is still happening with the patch from bug 1186587...

[1] https://github.com/mozilla-b2g/gaia/blob/db4dabf9b3932fcfccd078fde6bac2d56624d2d9/apps/system/js/app_window.js#L1768
[2] https://github.com/mozilla-b2g/gaia/blob/db4dabf9b3932fcfccd078fde6bac2d56624d2d9/apps/system/js/app_transition_controller.js#L246

Comment 13

[Chris Lord [:cwiiis]]

More logging:

[Launch pinned site]

www.mozilla.org: resize() 
www.mozilla.org: resize() 
www.mozilla.org: handle_opening() 
www.mozilla.org: setVisible(true, undefined) 
www.mozilla.org: setVisibleForScreenReader(true) 
www.mozilla.org: resize() 
Default Home Screen: setVisible(false, undefined) 
Default Home Screen: setVisibleForScreenReader(false) 
Default Home Screen: hideFrame 
www.mozilla.org: handle_opened() 
www.mozilla.org: setVisible(true, undefined) 
www.mozilla.org: setVisibleForScreenReader(true) 
www.mozilla.org: setOrientation() 
www.mozilla.org: lockOrientation(undefined) 
unlocking 
www.mozilla.org: resize() 
www.mozilla.org: _resize() 

[Rotate to landscape]

Mozilla: resize() 
Mozilla: _resize() 
Default Home Screen: _handle__orientationchange(evt) 
Mozilla: _handle__orientationchange(evt) 
Mozilla: _resize call from orientationchange 
Mozilla: _resize() 

[Press home button]

Default Home Screen: _resize() 
Default Home Screen: setVisible(true, undefined) 
Default Home Screen: setVisibleForScreenReader(true) 
Default Home Screen: showFrame 

Homescreen window resized to 590x286 

Default Home Screen: _resize() 
Default Home Screen: handle_opening() 
Default Home Screen: setVisible(true, undefined) 
Default Home Screen: setVisibleForScreenReader(true) 
Default Home Screen: setOrientation() 
Default Home Screen: handle_opened() 
Default Home Screen: setVisible(true, undefined) 
Default Home Screen: setVisibleForScreenReader(true) 
Default Home Screen: setOrientation() 
Default Home Screen: lockOrientation(undefined) 
locking to default 
Default Home Screen: _resize() 
Default Home Screen: _handle__orientationchange(evt) 
Default Home Screen: lockOrientation(undefined) 
locking to default 
Mozilla: _handle__orientationchange(evt) 
Mozilla: _resize call from orientationchange 
Mozilla: _resize() 
Default Home Screen: _resize() 
Mozilla: setVisible(false, undefined) 
Mozilla: setVisibleForScreenReader(false) 
Mozilla: hideFrame 

Homescreen window resized to 360x516

Comment 14

[Chris Lord [:cwiiis]]

(In reply to Etienne Segonzac (:etienne) from comment #12)
> Wonder if this issue is still happening with the patch from bug 1186587...

ftr, this patch makes no difference.

Comment 15

[Chris Lord [:cwiiis]]

This has defeated me. I give up. The code around this is massively complex(/convoluted?) and not something I'm going to understand in a day, or probably even a week, without help.

For what it's worth, this setVisible call seemed superfluous and was getting hit before handle_opening in transition_controller.js: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L2050

But even removing that, I couldn't figure out how to get it to not resize the window. I don't think this is a good use of my time anymore, this requires someone who actually knows what the heck is going on in this code :(

I could add a work-around in homescreen, but I feel that would remove pressure from what is actually a pretty bad performance/correctness bug, so I won't until it's demanded.

Etienne, do you have any more tips, or is there someone who knows how this all works that I could pawn this off on? :)

Comment 16

[Chris Lord [:cwiiis]]

This is my branch with debugging output added to relevant places to help diagnose this: https://github.com/Cwiiis/gaia/tree/bug1214388-landscape-apps-resize-homescreen - in case it's useful for anyone else.

Comment 17

[Etienne Segonzac (:etienne)]

I'll give it a shot :)

Comment 18

[GitHub Autolander Bot]

Attachment: [gaia] etiennesegonzac:bug-1214388 > mozilla-b2g:master

Comment 19

[Alberto Pastor [:albertopq]]

Comment on attachment 8686541 [details] [review]
[gaia] etiennesegonzac:bug-1214388 > mozilla-b2g:master

Works like a charm. Thanks!

Comment 20

[Etienne Segonzac (:etienne)]

https://github.com/mozilla-b2g/gaia/commit/d2c32e3e58c47aa54d06b1eb836c18b158e3917e

Comment 22

[Chris Lord [:cwiiis]]

This doesn't appear to be fixed, according to new reports. Regression or was it just not fixed fully in the first place?

Comment 24

[BMO Automation]

Firefox OS is not being worked on