1022094 - [Vertical] Show progress indicator for downloading appcached hosted apps

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Homescreen, defect, P1)

Comment 1

[James Lal [:lightsofapollo]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/20179

The app code here is very minimal (2 lines!) so it's mostly the infrastructure to write tests for hosted apps.

Comment 2

[Kevin Grandon :kgrandon]

Comment on attachment 8436264 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20179

I think it looks good, but I would like to see travis and marionette passing before the R+. Also left a few comments on github. Thanks!

Comment 3

[James Lal [:lightsofapollo]]

Comment on attachment 8436264 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20179

it's green & comments addressed

Comment 4

[Kevin Grandon :kgrandon]

Comment on attachment 8436264 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20179

I like green things.

Comment 5

[Kevin Grandon :kgrandon]

Landed the pull request: https://github.com/mozilla-b2g/gaia/commit/20d0c1e958218365c73222ab2bea9d88ffa132f3

Are we going to keep this bug open to track work, or should we close it?

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Reverted for strong suspicion of causing a big spike in Gaia unit test 7200s timeouts on TBPL (~30% of the time or so).
Master: https://github.com/mozilla-b2g/gaia/commit/eb6ee91f3e0541fa3c873cf90159e953420d61d0

https://tbpl.mozilla.org/php/getParsedLog.php?id=41318057&tree=B2g-Inbound

Comment 7

[James Lal [:lightsofapollo]]

This patch should not have any effect on unit tests? Particularly not in the system app (which this does not touch)... What can I do to get this relanded safely?

Comment 8

[Julien Wajsberg [:julienw]]

Is it possible to just increase the timeout until we fix bug 1022328?

Comment 9

[Julien Wajsberg [:julienw]]

Which makes me think that TBPL does not use our launcher so bug 1022328 should not change anything on TBPL.

Comment 10

[Kevin Grandon :kgrandon]

Looks like the tests were triggered several times to verify (Thanks Ryan?). Just adding some context here so it doesn't get lost and we can help track it down.

The run right before this patch landing: https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=bdda8e2d83e5

This patch landing: https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=539f0d0246b3

The run right before the backout: https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=f78cfe44de9e

And the backout: https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=697abe807996

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Yes, ample retriggers before and after this landed and before and after this was backed out confirm it to be the cause of the timeouts. Sorry.

Comment 12

[Mason Chang [Inactive] [:mchang]]

Just a note that this landing also caused a regression in multiple apps for start up time. All tests on a flame:

Regression in app: Settings
Gaia Rev: 20d0c1e958218365c73222ab2bea9d88ffa132f3 Gecko Rev: 187405:a2f0e0619332ba4ae1f37aab37bff6e71ffa19ab
Previous mean: 458 prev std dev: 23 
Current mean: 526 current std dev: 19 

Regression in app: Phone
Gaia Rev: 20d0c1e958218365c73222ab2bea9d88ffa132f3 Gecko Rev: 187405:a2f0e0619332ba4ae1f37aab37bff6e71ffa19ab
Previous mean: 465 prev std dev: 16
Current mean: 507 current std dev: 25

Regression in app: Camera
Gaia Rev: 20d0c1e958218365c73222ab2bea9d88ffa132f3 Gecko Rev: 187405:a2f0e0619332ba4ae1f37aab37bff6e71ffa19ab
Previous mean: 521 prev std dev: 21 
Current mean: 596 current std dev: 26 

Regression in app: Contacts 
Gaia Rev: 20d0c1e958218365c73222ab2bea9d88ffa132f3 Gecko Rev: 187405:a2f0e0619332ba4ae1f37aab37bff6e71ffa19ab
Previous mean: 715 prev std dev: 21
Current mean: 780 current std dev: 25

Comment 13

[James Lal [:lightsofapollo]]

Putting this in my queue as well... The last patch showed a spinner for _items_ without icons which was not quite right... Should be an easy fix after my icon refactoring patch.

Comment 14

[Johan Lorenzo [:jlorenzo]]

Marked as 2.0? blocker as it's a regression from 1.4.

Comment 15

[Mike Lee [:mlee]]

When this initially landed it caused a startup regression. It was backed out for failing other tests; see comment 11 and comment 12. Since this appears to be targeted for 2.0 we (FxOS Perf) are treating this as blocking.

FxOS Perf Triage

Comment 16

[James Lal [:lightsofapollo]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/20380

This is a _test only_ patch which checks that we show a spinner for appcache installs (this is basically a test of installling/showing appcache based hosted app apps on the verticalhome) The code for this functionality hits the same codepath as the code added to handle the packaged app case in https://github.com/lightsofapollo/gaia/commit/a16f730ad2512270b3da9f48b3428b3d6152a1bb

Comment 17

[James Lal [:lightsofapollo]]

Corrected commit https://github.com/mozilla-b2g/gaia/commit/5438f4773400a77f4d04f9e120a3c488c699ef7b

Comment 18

[James Lal [:lightsofapollo]]

mchang confirmed that the landing in the above commit did not change the perf numbers.

Comment 19

[Kevin Grandon :kgrandon]

Comment on attachment 8438603 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20380

I'll leave an R+, just go ahead and fix the linter issues first please. Also looks like you had a green run on gaia-try (minus linters): https://tbpl.mozilla.org/?tree=Gaia-Try&showall=1&rev=2384c9d71eda

Comment 20

[Jason Smith [:jsmith]]

We probably should be tagging this with the feature-b2g flag, rather than the blocking flag.

Candice - Can you switch the flags here?

Comment 21

[James Lal [:lightsofapollo]]

https://github.com/mozilla-b2g/gaia/commit/0e958770822e1b272bb4be40e186a9e179a1c707

Comment 22

[Kevin Grandon :kgrandon]

I am pretty sure just the test needs uplifting: https://github.com/mozilla-b2g/gaia/commit/2bb66630315299ca947e40fcec23c9f7ea012111

Oops - I just saw Jasons comment in comment 20. If it's not blocking status, it's test only so a=npotb (this also technically blocks a blocking bug). Let me know if there's any problems.

Comment 23

[Candice Serran (:cserran)]

Updated feature flag

Comment 24

[Johan Lorenzo [:jlorenzo]]

Verified on master. A blue spinner was shown when FoxShop was downloading.