Closed Bug 1022094 Opened 5 years ago Closed 5 years ago

[Vertical] Show progress indicator for downloading appcached hosted apps

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(feature-b2g:2.0, b2g-v2.0 fixed, b2g-v2.1 fixed)

VERIFIED FIXED
2.0 S4 (20june)
feature-b2g 2.0
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: jlal, Assigned: jlal)

References

Details

(Keywords: perf, Whiteboard: [c=regression p= s= u=2.0][systemsfe])

User Story

+++ This bug was initially created as a clone of Bug #1020722 +++

The current homescreen has functionality which will allow you to continue an app download (by tapping it) if it fails for some reason this should be ported over to the vertical home screen.

Attachments

(2 files)

No description provided.
The app code here is very minimal (2 lines!) so it's mostly the infrastructure to write tests for hosted apps.
Attachment #8436264 - Flags: review?(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!
Attachment #8436264 - Flags: review?(kgrandon) → feedback+
Comment on attachment 8436264 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20179

it's green & comments addressed
Attachment #8436264 - Flags: feedback+ → review?(kgrandon)
Comment on attachment 8436264 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20179

I like green things.
Attachment #8436264 - Flags: review?(kgrandon) → review+
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?
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
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?
Is it possible to just increase the timeout until we fix bug 1022328?
Which makes me think that TBPL does not use our launcher so bug 1022328 should not change anything on TBPL.
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
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.
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
Keywords: perf
OS: Mac OS X → Gonk (Firefox OS)
Priority: -- → P1
Hardware: x86 → ARM
Whiteboard: [c=regression p= s= u=]
Whiteboard: [c=regression p= s= u=] → [c=regression p= s= u=][systemsfe]
Target Milestone: --- → 2.0 S4 (20june)
QA Whiteboard: [VH-FC-blocking+]
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.
Assignee: nobody → jlal
QA Whiteboard: [VH-FC-blocking+] → [VH-FL-blocking+][VH-FC-blocking+]
Marked as 2.0? blocker as it's a regression from 1.4.
blocking-b2g: --- → 2.0?
Whiteboard: [c=regression p= s= u=][systemsfe] → [c=regression p= s= u=2.0][systemsfe]
Priority: P1 → P2
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
Status: NEW → ASSIGNED
blocking-b2g: 2.0? → 2.0+
Priority: P2 → P1
Summary: [Vertical] Show progress indicator for hosted apps (while the icon is downloading) → [Vertical] Show progress indicator for downloading appcached hosted apps
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
Attachment #8438603 - Flags: review?(kgrandon)
mchang confirmed that the landing in the above commit did not change the perf numbers.
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
Attachment #8438603 - Flags: review?(kgrandon)
We probably should be tagging this with the feature-b2g flag, rather than the blocking flag.

Candice - Can you switch the flags here?
Flags: needinfo?(cserran)
https://github.com/mozilla-b2g/gaia/commit/0e958770822e1b272bb4be40e186a9e179a1c707
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
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.
Updated feature flag
blocking-b2g: 2.0+ → ---
feature-b2g: --- → 2.0
Flags: needinfo?(cserran)
Verified on master. A blue spinner was shown when FoxShop was downloading.
Status: RESOLVED → VERIFIED
Flags: in-moztrap?(jlorenzo)
Flags: in-moztrap?(jlorenzo)
You need to log in before you can comment on or make changes to this bug.