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

VERIFIED FIXED in Firefox OS v2.1

Status

Firefox OS
Gaia::System::Window Mgmt
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: sfoster, Assigned: sfoster)

Tracking

({regression})

unspecified
2.1 S7 (24Oct)
x86_64
Gonk (Firefox OS)
regression
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking-b2g:2.1+, b2g-v2.0 unaffected, b2g-v2.1 verified, b2g-v2.2 unaffected)

Details

(Whiteboard: [systemsfe][p=1])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
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?
(Assignee)

Comment 1

3 years ago
[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.
blocking-b2g: --- → 2.1?
Summary: Task Manager slow to open from a browser window loading heavy page → v2.1 Task Manager slow to open from a browser window loading heavy page
Adding qawanted for branch checks.
Keywords: qawanted
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
QA Whiteboard: [QAnalyst-Triage?]
status-b2g-v2.0: --- → unaffected
status-b2g-v2.1: --- → affected
status-b2g-v2.2: --- → unaffected
Flags: needinfo?(jmitchell)
Keywords: qawanted
Keywords: regression
QA Contact: jmercado
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: regressionwindow-wanted
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
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: regressionwindow-wanted
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?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell) → needinfo?(21)
blocking-b2g: 2.1? → 2.1+

Comment 6

3 years ago
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. :)
Assignee: nobody → aus
Status: NEW → ASSIGNED
Flags: needinfo?(21)
Target Milestone: --- → 2.1 S7 (Oct24)

Comment 7

3 years ago
:epang, could I get a placeholder color to use as the background when we do not yet have a screenshot available?
Flags: needinfo?(epang)
(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!
Flags: needinfo?(epang)
(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

3 years ago
Created attachment 8504264 [details] [review]
Pull Request - Do not wait more than 400ms for screenshot. Do not screenshot homescreen or browser window.
Attachment #8504264 - Flags: review?(alive)

Updated

3 years ago
Attachment #8504264 - Flags: review?(alive) → feedback?(sfoster)

Comment 11

3 years ago
(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!

Updated

3 years ago
Whiteboard: [systemsfe] → [systemsfe][p=1]
(Assignee)

Comment 12

3 years ago
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).
Attachment #8504264 - Flags: feedback?(sfoster) → feedback+
(Assignee)

Comment 13

3 years ago
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.
Assignee: aus → sfoster
Flags: needinfo?(fdjabri)
(Assignee)

Comment 14

3 years ago
Created attachment 8506556 [details] [review]
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.
Attachment #8504264 - Attachment is obsolete: true
Attachment #8506556 - Flags: review?(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 :)
Attachment #8506556 - Flags: review?(etienne)
(Assignee)

Comment 16

3 years ago
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.
Attachment #8506556 - Flags: review?(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!
Attachment #8506556 - Flags: review?(etienne) → review+
(Assignee)

Comment 18

3 years ago
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
Attachment #8506556 - Flags: approval-gaia-v2.1?(fabrice)
(Assignee)

Comment 19

3 years ago
(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.
Flags: needinfo?(fdjabri)
NI fabrice since its 2.1 only.
Flags: needinfo?(fabrice)
Flags: needinfo?(fabrice)
Attachment #8506556 - Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1+
(Assignee)

Comment 21

3 years ago
Merged to v2.1: https://github.com/mozilla-b2g/gaia/commit/76070c98dd0068324938c79bede50fe6d90bd996
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-b2g-v2.1: affected → fixed
Resolution: --- → FIXED
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.
Flags: in-testsuite+
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
status-b2g-v2.1: fixed → verified
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.