Often the previews are missing, and sometimes they do show up after a few seconds of waiting (3 - 5).

Hi Alive, could you help to check this one? Thanks!

No, I don't understand what I could do, isn't it expected to be slow on tarako? Screenshoting "IS" expensive. The only way to improve it is from gecko side, but AFAIK it's not landed yet. What do we want to achieve in this issue?

My question is that the behavior is unpredictable: * sometimes you can see 4 apps in card view, all with correct preview * sometimes with only 1 app in card view, there is no preview, just a floating [x] button * sometimes with 2 apps in card view, one has preview and other doesn't The user experience could improve, without using more memory. Some possible mitigations: * improvement for the worst case: add an outline of where preview would be. this is better than a floating [x] button. * make behavior more deterministic by throwing away previews of older loaded apps first. this improves likelihood that app that is OOM-killed last will still have preview. survival of the fittest :D

already 1.3T+, remove [tarako] whiteboard

We should just have a fallback with the icon. How does this sound?

Flagging Francis to advise. Francis, you seemed closest to this but I'm not certain. Please reassign as appropriate.

From Product perspective, showing App icon is ok.

(In reply to Marvin Khoo [:Marvin_Khoo] from comment #7) > From Product perspective, showing App icon is ok. Sounds a reasonable tradeoff to do at the moment. Showing the app icon centered on a background that is a bit different from the color of the cards view in order to see the card sounds doable.

lets give UX a day or two to confirm, if no further concerns, lets move forward with using the app icon for tarako

For systemsfe sprint planning.

Displaying an icon is what we're intending in any case for the case of "zombie" sheets, so this approach would be consistent with that. Hung, could you provide some visual specs for this in Task manager? Thanks.

Forwarding to Eric to have a look. Thanks

(In reply to Hung Nguyen from comment #12) > Forwarding to Eric to have a look. > > Thanks Hi, here are links to the specs for zombie apps in the task manager (the right image in each set shows this in each spec) Portrait Spec: https://mozilla.box.com/s/nwr2w7g32sbkfjx0efau Landscape Spec: https://mozilla.box.com/s/7wsuppddj8mewzkch5yr Let me know if there are any questions, thanks!

(In reply to Eric Pang [:epang] from comment #13) > (In reply to Hung Nguyen from comment #12) > > Forwarding to Eric to have a look. > > > > Thanks > > Hi, here are links to the specs for zombie apps in the task manager (the > right image in each set shows this in each spec) > > Portrait Spec: https://mozilla.box.com/s/nwr2w7g32sbkfjx0efau > Landscape Spec: https://mozilla.box.com/s/7wsuppddj8mewzkch5yr > > Let me know if there are any questions, thanks! Ignore my previous post, I didn't realize this was for tarako. Will attached updated specs shortly, thanks!

Hi, here is a link to an updated spec for 1.3 https://mozilla.box.com/s/957mlao74kubgfwlqvn1 When a 'zombie' app appears the task screen is white and the app icon is displayed centered. Let me know if there are any questions, thanks!

Note to self: <insert brain dump here about the first steps to resolve this issue>

Plan of attack to resolve this issue: First, turn on application icons on top of cards: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/cards_view.js#L12 This will likely not work at first as this code is probably bit-rotten. Let's go ahead and get that working. Step two is removing display and capture of screenshots. This is slightly more involved but here are some starting points. https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/cards_view.js#L207 is where we create the div that holds the screenshot itself. https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/cards_view.js#L226 is where we grab the iframe that we want to screenshot. And finally, here is where we either re-use the screenshot or take a new one: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/cards_view.js#L297

Is there any (not just for tarako) case where we'll need to show *both* the icon and the screenshot in the card view? Or is it an either/or

(In reply to Sam Foster [:sfoster] from comment #18) > Is there any (not just for tarako) case where we'll need to show *both* the > icon and the screenshot in the card view? Or is it an either/or Yes, there are cases in which this may happen. In 1.5, we're hoping that the cards view will also include killed applications with a screenshot, however, we also want to have fallback to application icon or simply have the application icon in an overlay. I should mention that the divergence of the code between the 1.3/1.3t branch and 1.4/master is substantial. The new App Window Manager changed how we do things and the Edge Gestures and Sheets were taken into account for 1.4. Your patch is likely to have to land first on master and then be ported to 1.3t.

In the case that we can't get an icon from app.manifest.icons or app.icon, is there a default icon should we use for the card view preview?

I wont have a device to test on until the end of the week at the earliest, so I can't carry this over the finish line. If someone else wants to run with this patch, let me know if I can help and I'm happy to iterate, help back-port etc.

I thought the action item of this bug is to provide a switch to only show icon splash in card views on low-end devices? How is not having an tarako with you stops you from doing that?

Tim, I don't have any device currrently - low or high end. I don't feel comfortable (and have been advised against) requesting review and committing code that's not been tested outside the browser. I will look into how this will back-port to 1.3 though.

Sam joined our group this week and he needs 'a' device. He should have one by tomorrow.

Hey Sam, If you're working on this and waiting for a device, can you assign this to yourself please?

If backporting this is a PITA, look way back to comment #5 & #7. We would settle for a simple app icon for 1.3t. In fact, given the timeframe (tarako freeze imminent) might be better to scale back to just that change first, if it'd be quick and easy to do.

As it stands the patch is pretty trivial. Its just un-bitrotting existing code to display an app icon on the card view, and adding a little logic to allow either the screenshot or appicon (or both in theory, but that would need UX work.) I do need to know how to set this flag though - how will we configure and make available the values for DISPLAY_APP_SCREENSHOT and DISPLAY_APP_ICON. These are hard-coded right now -see https://github.com/sfoster/gaia/blob/5f503f65441d30a70b7a180a086374830064443d/apps/system/js/cards_view.js#L12

(In reply to Sam Foster [:sfoster] from comment #28) > As it stands the patch is pretty trivial. Its just un-bitrotting existing > code to display an app icon on the card view, and adding a little logic to > allow either the screenshot or appicon (or both in theory, but that would > need UX work.) > > I do need to know how to set this flag though - how will we configure and > make available the values for DISPLAY_APP_SCREENSHOT and DISPLAY_APP_ICON. > These are hard-coded right now -see > https://github.com/sfoster/gaia/blob/ > 5f503f65441d30a70b7a180a086374830064443d/apps/system/js/cards_view.js#L12 This patch will only land on 1.3t, so you can just set the flags to the appropriate value. Does that also mean that if we set: DISPLAY_APP_SCREENSHOT = false and DISPLAY_APP_ICON = true we don't *take* any screenshot?

I think it's OK for these to remain hard-coded for 1.3t (with display icon on) for expediency. On 1.4 and master (1.5), it would be nice to have a Setting to toggle this on and off at runtime and add tests for this as well (unit tests are fine). See apps/system/test/unit/cards_view_test.js. Here we will want screenshots on and icons off. Default values for Settings are located here: https://github.com/mozilla-b2g/gaia/blob/master/build/config/common-settings.json (these can be overridden at build time so partners can tweak this as they wish, and so can developers) This setting does not need to be directly user accessible but should be present in the Developer Menu. See https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/elements/developer.html and https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/developer.js for adding Developer Menu options. FYI, the developer menu can be turned on by going into Settings -> Device Information -> More Information -> Developer Menu checkbox (at the bottom). Reading and writing of Settings is also demonstrated in developer.js. Hope this all makes sense. If not, I'm on IRC. :)

tarako will never use 1.4, so we can focus on just doing the right thing on 1.5

> Does that also mean that if we set: > DISPLAY_APP_SCREENSHOT = false and DISPLAY_APP_ICON = true we don't *take* > any screenshot? Correct. The screenshot gets taken in the onviewport handler at https://github.com/sfoster/gaia/blob/5f503f65441d30a70b7a180a086374830064443d/apps/system/js/cards_view.js#L282, and with this patch we return early if DISPLAY_APP_SCREENSHOT is false. This is the same code path that existed already for when the screenshot was already taken, so nothing to new here - I just added this extra condition. I use the same check to avoid getting object references we wont need. In other words, the patch doesn't attempt to address the much harder performance problem with taking and displaying screenshots, it just uses the lighter-weight app icon strategy agreed in comment #9.

great, I think we should just do that.

Hi Sam, I've attached the default icon for apps. Let me know if anything else is needed. Thanks!

Tested on 1.3 on Peak.

Comment on attachment 8395103 [details] Patch against 1.3t to use app icon not screenshot for card view preview Alive, Vivien, whichever of you can get to this one first can clear the other persons review flag. :) We're hoping to land this ASAP. I've done a first review and it looks good to me (and works on my Peak, as expected).

Comment on attachment 8395103 [details] Patch against 1.3t to use app icon not screenshot for card view preview I am asked to let any workaround passes on v1.3t so I r+ this any way. Note: the commit message needs a bug number.

Thanks Alive. Commit message is updated to include bug number. There is follow up work to add settings support and unit tests for this in master/1.5 - and I believe for 1.4 uplift, so I guess this bug should remain open

Follow up for Settings and unit tests filed as bug 987237

Fixed unit test failure - moved early return in onviewport to allow the rotation class to get set when using the app icon vs. screenshot in card views. Carrying r+ from alive

Comment on attachment 8395103 [details] Patch against 1.3t to use app icon not screenshot for card view preview NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] [Bug caused by] (feature/regressing bug #): 1.3t only - screenshots too slow to take and load on Tarako/low-end devices [User impact] if declined: Cards view screenshot previews may take seconds to show up - or not at all [Testing completed]: Patch tested in browser and on Peak, unit tests passing on travis build [Risk to taking this patch] (and alternatives if risky): Low-risk [String changes made]: None Note: this patch is intended for 1.3t only.

A side effect. FYI. Bug 988128. Thanks!

Verified it. Thanks for the help. * Build Info. - Gaia d8ff994bd96c37ba9a93c343932a5441a78a0eec - Gecko 6db37b3f76b4fe2aa6f8fb5ae9e036ed99344772 - BuildID 20140327060053 - Version 28.1

It can be reproduced on latest v1.4 Dolphin build. Attach the screenshot. - https://drive.google.com/file/d/0B3SBjlVFSBc3TTdhcW5sZFdSUGs/edit?usp=sharing

Luke, can you help on uplifting the patch to v1.4? thanks.

I've cherry-picked this patch to v1.4 branch here. Let's wait for tests.

Comment on attachment 8457900 [details] cherry-pick to v1.4 branch Hi Thomas, Travis and TBPL are passed! However, this patch just hard-codes to replace the screenshots with app icons in cards view. It can resolve the blank screenshot issue (see comment 45) but I can't affirm if it's effective against the loading time of cards view.

Hi James: can you also verify if this patch can reduce the launch time in v1.4?

Yang, please verify this 1.3t patch on 1.4 dolphin.

(In reply to Luke Chang [:lchang] from comment #48) > Comment on attachment 8457900 [details] > cherry-pick to v1.4 branch > > Hi Thomas, > > Travis and TBPL are passed! > > However, this patch just hard-codes to replace the screenshots with app > icons in cards view. It can resolve the blank screenshot issue (see comment > 45) but I can't affirm if it's effective against the loading time of cards > view. Hi,Luke I'm not sure which time should I verify ?What do you mean by saying loading time of cards view? Does it mean the time cost from long press the home button to the cards view appears? I'm also confused on launch time From comment 49.Does launch time mean launch app from cards view? Could you tell me which time should I verify? Thank you!

Hi Yang, One of the original symptom this bug mentioned is that sometimes it takes more than 3 seconds to show the cards view after long pressing home button. I heard from Thomas that you're trying to uplift this v1.3t patch to v1.4 but I'm not sure what symptom you're encountering in dolphin. In comment 48, I meant that the uplifted patch can resolve the screenshot-missing issue but I have no idea if it can resolve loading time as well. So, I'd like to know if the patch I uplifted can really fulfill your need and we can further request the approval to merge this patch to v1.4 if necessary.

(In reply to Luke Chang [:lchang] from comment #52) Hi,Luke I flash the latest version on my dolphin, and I can't reproduce the issue about loading time you mentioned. So I also have no idea if it can resolve loading time as well.But as in comment 45,v1.4 does have the issue about screenshot-missing.So I think this patch should merge to v1.4. What do you think then?^_^

Hi Thomas, According to comment 53, do we need the approval for uplifting this patch?

(In reply to Luke Chang [:lchang] from comment #54) > Hi Thomas, > > According to comment 53, do we need the approval for uplifting this patch? I just find a method to reproduce the loading time issue.500 contacts in contact app,launch contact app,then long press the home button,the card view loads very slow.I'll try the patch to see if it is helpful to the issue.

Hi,Luke I've applied the patch on v1.4,and use |APP=system make install-gaia| to install system app. 1.With 500 contacts in contact app.Open the contact app,and long press home button,it takes more than 4 seconds to load the cards view. 2.The icons of dialer and contacts couldn't be displayed.Please see the attachment for more detail. So I think maybe the patch needs to be improved.

Hi Yang, (In reply to yang.zhao from comment #56) > 1.With 500 contacts in contact app.Open the contact app,and long press > home button,it takes more than 4 seconds to load the cards view. I guess this issue is not only caused by cards view and can't be fixed by this patch. I suggest filing another bug to track this issue. > 2.The icons of dialer and contacts couldn't be displayed.Please see the > attachment for more detail. I've updated my patch for this issue. Please check attachment 8460712 [details] again. Thanks.

(In reply to Luke Chang [:lchang] from comment #57) > Hi Yang, > > (In reply to yang.zhao from comment #56) > > 1.With 500 contacts in contact app.Open the contact app,and long press > > home button,it takes more than 4 seconds to load the cards view. > > I guess this issue is not only caused by cards view and can't be fixed by > this patch. I suggest filing another bug to track this issue. This issue only happens when long press the home button in a short time after cold launch for contact > > 2.The icons of dialer and contacts couldn't be displayed.Please see the > > attachment for more detail. > > I've updated my patch for this issue. Please check attachment 8460712 [details] > [details] again. Thanks. I've tried the attachment 8457900 [details],this new patch is ok for the dialer/contacts icon. Maybe we should uplift it to v1.4 first.

Maybe we should uplift it to v1.4 first.