1081321 - v2.1 Task Manager slow to open from a browser window loading heavy page

Status: VERIFIED FIXED

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

Description

[Sam Foster [:sfoster] (he/him)]

STR: 

1. Open a large/heavy page in a browser window. E.g. http://rakkordo.com/
2. long-press the home holdhome while the browser loading indicator is still showing - to show the task-manager

Expected results: 
Task manager shows immediately

Actual result
Task manager shows after significant delay. Delay may be a function of connection speed. 

Can you fix the nits Etienne pointed out?

Comment 1

[Sam Foster [:sfoster] (he/him)]

[Blocking Requested - why for this release]: Bad UX, symptoms are worse for slower connections which are expected in target market

This is a follow-up from bug 1078859 which are related issues - iframe.getScreenshot doesnt return the screenshot until the window has (at least partially) loaded. For the normal (not 'show browser windows' case), in the holdhome event handler we request a screenshot of the active window and wait to show the task manager when that calls-back.

Comment 2

[KTucker [:KTucker][Inactive 3/4/2016]]

Adding qawanted for branch checks.

Comment 3

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue reproduces on 2.1 KK Flame.  

Actual Results: The task manager takes signficantly longer to open when opening a large website.

Environmental Variables (shallow flash):
Device: Flame 2.1
BuildID: 20141011094725
Gaia: f5d4ff60ffed8961f7d0380ada9d0facfdfd56b1
Gecko: 65c0a4f2b0e9
Version: 34.0a2 (2.1) 
Firmware Version: L1TC10011800
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0


Does not reproduce:

Environmental Variables (shallow flash):
Device: Flame 2.2
BuildID: 20141011031924
Gaia: 95f580a1522ffd0f09302372b78200dab9b6f322
Gecko: 3f6a51950eb5
Version: 35.0a1 (2.2) 
Firmware Version: L1TC10011800
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Environmental Variables(shallow flash):
Device: Flame 2.0
BuildID: 20141010074705
Gaia: 6effca669c5baaf6cd7a63c91b71a02c6bd953b3
Gecko: 54ec9cb26b59
Version: 32.0 (2.0) 
Firmware Version: L1TC10011800
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 4

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Bug 1061324 seems to have fixed this issue.

B2g-inbound Regression Window

Last Broken 
Environmental Variables:
Device: Flame 2.2
BuildID: 20140910060514
Gaia: 2f71bd60ff9e50c9821ef96d6a67fbd51f721f2a
Gecko: 661f503ea524
Version: 35.0a1 (2.2) 
Firmware Version: L1TC00011230
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

First Working 
Environmental Variables:
Device: Flame 2.2
BuildID: 20140910061313
Gaia: b3698e7e93b5945b5aa5f857dd01932894f57a85
Gecko: b80b5054f126
Version: 35.0a1 (2.2) 
Firmware Version: L1TC00011230
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Last Broken gaia / First Working gecko - Issue DOES occur
Gaia: 2f71bd60ff9e50c9821ef96d6a67fbd51f721f2a
Gecko: b80b5054f126

First Working gaia / Last Broken gekko - Issue does NOT occur
Gaia: b3698e7e93b5945b5aa5f857dd01932894f57a85
Gecko: 661f503ea524

Gaia Pushlog: https://github.com/mozilla-b2g/gaia/compare/2f71bd60ff9e50c9821ef96d6a67fbd51f721f2a...b3698e7e93b5945b5aa5f857dd01932894f57a85

Comment 5

[Joshua Mitchell (Inactive)]

The patch from Bug 1061324 seems to have fixed this issue in 2.1 but this issue is still present in 2.0 and 2.2 -  can you take a look Vivien?

Comment 6

[Aus Lacroix [:aus]]

I'll have a look today. I'm pretty sure I know how to fix this. However, for the record, based on the code itself, the issue should be *PRESENT* in 2.1 and 2.0 but *NOT* present in 2.2.

This is what I'm seeing. :)

Comment 7

[Aus Lacroix [:aus]]

:epang, could I get a placeholder color to use as the background when we do not yet have a screenshot available?

Comment 8

[Eric Pang [:epang] UX]

(In reply to Ghislain Aus Lacroix [:aus] from comment #7)
> :epang, could I get a placeholder color to use as the background when we do
> not yet have a screenshot available?

Hey Aus, we can use #f4f4f4 as a placeholder, thanks!

Comment 9

[Joshua Mitchell (Inactive)]

(In reply to Joshua Mitchell [:Joshua_M] from comment #5)
> The patch from Bug 1061324 seems to have fixed this issue in 2.1 but this
> issue is still present in 2.0 and 2.2 -  can you take a look Vivien?

What I MEANT to say was Bug1061324 seems to have fixed the issue in 2.2 but the fix is not present in 2.1 -   looked at too many windows over the weekend - sorry for the sloppy work

Comment 10

[Aus Lacroix [:aus]]

Attachment: Pull Request - Do not wait more than 400ms for screenshot. Do not screenshot homescreen or browser window.

Comment 11

[Aus Lacroix [:aus]]

(In reply to Joshua Mitchell [:Joshua_M] from comment #9)
> (In reply to Joshua Mitchell [:Joshua_M] from comment #5)
> > The patch from Bug 1061324 seems to have fixed this issue in 2.1 but this
> > issue is still present in 2.0 and 2.2 -  can you take a look Vivien?
> 
> What I MEANT to say was Bug1061324 seems to have fixed the issue in 2.2 but
> the fix is not present in 2.1 -   looked at too many windows over the
> weekend - sorry for the sloppy work

No worries :) Thanks for clarifying!

Comment 12

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8504264 [details] [review]
Pull Request - Do not wait more than 400ms for screenshot. Do not screenshot homescreen or browser window.

This seems to do what it says on the tin. I've seen a few old screenshots while testing this - for example if you start a new page loading in the browser, and long-tap home when the page is rendered above the fold but still loading, you see a screenshot from the previous page. I've also seen the new tab (search app) page in the task manager instead of the page I had just loaded from there. But I dont think these are new issues introduced by the patch. 

With all the churn in this component we may want to step back and get a new ui-review at some point, there just seems to be a lot of abrupt changes and disjointed bebavior (that is, if there's any possibility of improving the user's experience in the 2.1 timeframe).

Comment 13

[Sam Foster [:sfoster] (he/him)]

Francis: do you have thoughts on what should happen here for 2.1? see Comment #12. If we dont immediately/quickly get a screenshot we show the task manager anyway to avoid blocking and having the UI appear unresponsive. I believe we'll then show whatever we already have (which might be a screenshot from the previous page), and failing that a plain background color.

Comment 14

[Sam Foster [:sfoster] (he/him)]

Attachment: Pull Request - Do not wait more than 400ms for screenshot. Do not screenshot homescreen or browser window.

I've rebased Aus' pull request and caught the hex vs. rgb nit from Kevin. I spent a little time looking at alternatives, like showing the icon until the screenshot shows up. There are still issues where you'll get the screenshot of the previous page if you are navigating in a browser window when you long-tap home to see the task manager. It seems to me that we really need screenshot update/availablility to be event-driven rather than callback driven, and I'm not sure we want to take that on in this bug.

Comment 15

[Etienne Segonzac (:etienne)]

Comment on attachment 8506556 [details] [review]
Pull Request - Do not wait more than 400ms for screenshot. Do not screenshot homescreen or browser window.

Do we really need the timeout if we're ignoring browser sheets?

Also, I'd like a small spy in the |display cardsview via holdhome >| to cover the change we end up with :)

Comment 16

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8506556 [details] [review]
Pull Request - Do not wait more than 400ms for screenshot. Do not screenshot homescreen or browser window.

Updated PR to remove the timeout and just skip the getScreenshot on holdhome for browser windows and homescreen. Added a couple unit tests to assert the expected behavior.

Comment 17

[Etienne Segonzac (:etienne)]

Comment on attachment 8506556 [details] [review]
Pull Request - Do not wait more than 400ms for screenshot. Do not screenshot homescreen or browser window.

thanks!

Comment 18

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8506556 [details] [review]
Pull Request - Do not wait more than 400ms for screenshot. Do not screenshot homescreen or browser window.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Task Manager
[User impact] if declined: On slow connections and/or heavy pages, the home button my seem unresponsive to long-tap when loading a page in a browser window
[Testing completed]: manual testing on Flame/2.1, Gaia-Try at https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=05fc9acb65be + new unit tests to cover expected behavior
[Risk to taking this patch] (and alternatives if risky): Low risk, just streamlines existing behavior in Task Manager
[String changes made]: None

Comment 19

[Sam Foster [:sfoster] (he/him)]

(In reply to Sam Foster [:sfoster] from comment #13)
> Francis: do you have thoughts on what should happen here for 2.1? see
> Comment #12. If we dont immediately/quickly get a screenshot we show the
> task manager anyway to avoid blocking and having the UI appear unresponsive.
> I believe we'll then show whatever we already have (which might be a
> screenshot from the previous page), and failing that a plain background
> color.

I went with the simplest case - if there's no screenshot we'll just get the flat background-color #f4f4f4. As this has been re-worked to use the horizontal strip of cards in 2.2 I'm not sure it makes sense to try and get too fancy here.

Comment 20

[Gregor Wagner [:gwagner]]

NI fabrice since its 2.1 only.

Comment 21

[Sam Foster [:sfoster] (he/him)]

Merged to v2.1: https://github.com/mozilla-b2g/gaia/commit/76070c98dd0068324938c79bede50fe6d90bd996

Comment 22

[Kevin Grandon :kgrandon]

The 2.1 patch comes with a unit test which should suffice for now. I think we can skip worrying about an integration tests for 2.2 here.

Comment 23

[Yeojin Chung [:YeojinC]]

This issue is verified fixed on Flame 2.1.
No significant delay is observed when the task manager is opened from a heavy web page.

Flame 2.1 

Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141022001201
Gaia: 3d9cc667f4e929861a9a77c41096bbf5a9c1bde0
Gecko: 928b18f7d8ff
Gonk: 05aa7b98d3f891b334031dc710d48d0d6b82ec1d
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0