849441 - [B2g] [Webapps] The wrapper loads slowly causing the user to see the background

Status: RESOLVED FIXED

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

Description

[Deepa Subramanian]

Description:
Go to everything.me page. Click on one of the categories listed and tap on any app. While the app is loading  the background screen image at the bottom is seen first and then the wrapper loads up. Clicking on home button, the user can see the same background change as the wrapper unloads. This happens very quickly.

Repro Steps:
1) Updated to Unagi Build ID: 20130308070202
2) Go to everything.me
3) Click on one of the categories 
4) Open an app (while app loading user sees at screen background at the bottom) 
6) Tap on the home button to leave the app
7) User sees the background screen changing at the bottom again

Expected:
The user is able to see the wrapper load and unload properly.

Actual:
The user is unable to see the loading and unloading of the wrapper appropriately.
Repro frequency: 100%


Environmental  Variables:
Verified on Unagi Build ID: 20130308070202
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/94927529a55f
Gaia: 87801b12cd3aac153fae84844da8f1e6455ea679
Also, Repros in Master Build ID: 20130308031058
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/mozilla-central/rev/cb432984d5ce
Gaia: c7ac21c48e3718a178340a56b17a0508ae903aba

Other Notes:
Three screenshots attached

Comment 1

[Deepa Subramanian]

Attachment: Bug 849441 - attachment 1

Comment 2

[Deepa Subramanian]

Attachment: Bug 849441 - attachment 2

Comment 3

[Cristian Rodriguez (:crdlc)]

Attachment: Patch v1

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 723453 [details]
Patch v1

It's sad that window manager doesn't comes with unit tests.

Please manually test your patch thoroughly before merging the patch.

Comment 5

[Cristian Rodriguez (:crdlc)]

Comment on attachment 723453 [details]
Patch v1

Please, could you test it on master? Thanks a lot

Comment 6

[Cristian Rodriguez (:crdlc)]

If feedback is +, I will land on master, thanks Tim

Comment 7

[Deepa Subramanian]

The user is not able to see the wrapper load and unload properly while opening webapps. 

This issue was found and tested on both in Leo and in Master (also found in the description).

Master Build ID:20130312030649
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/mozilla-central/rev/519c7cf5605b
Gaia: ee236c942f2b9abe9065c2fb967b715770a43d9c

And also on 
Unagi Build ID: 20130312070202
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/99680a32f297
Gaia: de3e5b9205e6cb1a6bd0858a98d159272ad96d11

Comment 8

[Cristian Rodriguez (:crdlc)]

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

No risk, very simple and it improves the user experience a lot

Comment 9

[Julien Wajsberg [:julienw]]

there will likely be conflicts because of Bug 844990 that removed some of the touched code. Crdlc could you please try to uplift it to v1-train yourself ?

Comment 10

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Julien Wajsberg [:julienw] from comment #9)
> there will likely be conflicts because of Bug 844990 that removed some of
> the touched code. Crdlc could you please try to uplift it to v1-train
> yourself ?

Yeah, good catch.

IMO This is also a regression from 842627 because:
The adding class action to wrapper element is also put in windowOpened function, which is delayed until mozbrowserloadend(on ev.me this means wait until all network transmit......) is fired.

Comment 11

[(inactive after 6/18) Alive Kuo [:alive]]

Since 844990 already remove the something from 842627, I suggest that we don't need to do this patch :|

Comment 12

[Julien Wajsberg [:julienw]]

It would indeed make sense to revert this and see if Bug 844990 is enough to fix this.

Comment 13

[(inactive after 6/18) Alive Kuo [:alive]]

This doesn't happen in current v1-train ;) 
(with patch of bug 844990, without this bug's patch).

Comment 14

[Cristian Rodriguez (:crdlc)]

It doesn't happen neither loading wrapper nor closing it? If it is so, it is not needed anymore

Comment 15

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to crdlc from comment #14)
> It doesn't happen neither loading wrapper nor closing it? If it is so, it is
> not needed anymore

Ya, the wrapper-footer appears/disappears immediately..

Comment 16

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Alive Kuo [:alive] (PTO during 3/27~4/7) from comment #15)
> (In reply to crdlc from comment #14)
> > It doesn't happen neither loading wrapper nor closing it? If it is so, it is
> > not needed anymore
> 
> Ya, the wrapper-footer appears/disappears immediately..

Wait, the wrapper type app disappears too slow, investigating.

Comment 17

[(inactive after 6/18) Alive Kuo [:alive]]

OK, when an app is closing(to homescreen),
we will have about ~1sec to wait next-paint event to be fired.

This is a general issue in WM, and timdream tells that we have a at most 300ms timer to cancel the event callback if the event is not fired in 300ms "before".

So I think this is another regression in recently performance work in Window Manager.

I still recommend to back out the change in this bug and set dup to 844990 and open another bug for app closing too slow one.

Thanks!

Comment 18

[(inactive after 6/18) Alive Kuo [:alive]]

The slow app closing transition is caused by bug 831391's patch.

Comment 19

[(inactive after 6/18) Alive Kuo [:alive]]

So what we should do in this bug should be:
delay the |removeClass('visible')| like what https://bugzilla.mozilla.org/attachment.cgi?id=723453 does "only".

Comment 20

[Cristian Rodriguez (:crdlc)]

Sorry, what do you mean? Open other bug only to implement 

if ('wrapper' in closeFrame.dataset) {
+        wrapperHeader.classList.remove('visible');
+        wrapperFooter.classList.remove('visible');
+      }

and

-  -moz-transition-delay: .5s; /* Delay opening a wrapper */
-  -moz-transition-property: visibility;

true?

thanks

Comment 21

[Cristian Rodriguez (:crdlc)]

v1-train:

https://github.com/mozilla-b2g/gaia/commit/51db440a62e36e7fcbfe5727f9f899ef7b0c5132