Closed Bug 967277 Opened 11 years ago Closed 11 years ago

[Contacts] Scrolling the contacts app causes reflow

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::Contacts
Platform:
ARM
Gonk (Firefox OS)
Type:
defect

Tracking

(blocking-b2g:1.3+, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

Status:
RESOLVED FIXED
Milestone:
1.4 S1 (14feb)
Project Flags:
blocking-b2g 1.3+
Tracking Flags:
Tracking Status
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: BenWa, Assigned: vingtetun)

References

Details

(Keywords: perf, Whiteboard: [c=handeye p= s=2014.02.14 u=1.3] [ETA=2/14])

Attachments

(2 files)

bug967277.patch
(deleted), patch
arcturus
: review+
Details | Diff | Splinter Review
bug967277.followup.import.patch
(deleted), patch
arcturus
: review+
Details | Diff | Splinter Review
Reflowing shouldn't happen while scrolling but yet we do it while scrolling. I heard there might be people working on this bug I can't find the bug so filing this to track that work. Profiles show this Layout phase taking 50-100ms which we need to cutdown at all costs.
I attached one of the reasons for reflows. I suspect that we are also reflowing because of the code that looks for images. Will try to investigate.
Depends on: 942460
Attached patch bug967277.patch (deleted) — — Splinter Review
With this patch and the position: sticky patch I don't see any reflows anymore when the contacts list is panned.
Attachment #8369775 - Flags: review?(francisco.jordano)
Keywords: perf
Does bug 916315 apply here? Will position sticky ship with 1.3? If not we should able to come up with an alternative that doesn't cause expensive reflows.
(In reply to Benoit Girard (:BenWa) from comment #3) > Does bug 916315 apply here? Will position sticky ship with 1.3? If not we > should able to come up with an alternative that doesn't cause expensive > reflows. position: sticky seems to works OK with our app testcases. If we can find a way to limit it to certified app for 1.3 it would make me happy and I will confident to turn it on for those. I'm mostly worried about enabling it for the rest of the world as of today as people can run into issues that are not covered by our testcases.
Assignee: nobody → 21
(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #2) > Created attachment 8369775 [details] [diff] [review] > bug967277.patch > > With this patch and the position: sticky patch I don't see any reflows > anymore when the contacts list is panned. I just tested this on trunk with this patch (and a few others that should help performance) and I'm still seeing expensive reflows: http://people.mozilla.org/~bgirard/cleopatra/#report=feacfdfc1f931f13f5cc171f4a5ee219d392086e Now I'm also seeing: |scrollHandler() @ tag_visibility_monitor.js:150| Perhaps I'm profiling the wrong thing here.
(In reply to Benoit Girard (:BenWa) from comment #5) > (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #2) > > Created attachment 8369775 [details] [diff] [review] > > bug967277.patch > > > > With this patch and the position: sticky patch I don't see any reflows > > anymore when the contacts list is panned. > > I just tested this on trunk with this patch (and a few others that should > help performance) and I'm still seeing expensive reflows: > > http://people.mozilla.org/~bgirard/cleopatra/ > #report=feacfdfc1f931f13f5cc171f4a5ee219d392086e > Did you profile with position: sticky activated in the app ? It should removed some reflows. > Now I'm also seeing: > |scrollHandler() @ tag_visibility_monitor.js:150| > I can look at the visibility_monitor.js stuff too. I used my dogfood devices to see reflows and I probably does not have enough contacts to trigger visibility monitor.I will find a few more friends.
Comment on attachment 8369775 [details] [diff] [review] bug967277.patch Review of attachment 8369775 [details] [diff] [review]: ----------------------------------------------------------------- Just tried and definitely we save some reflows. Thanks a lot @21!
Attachment #8369775 - Flags: review?(francisco.jordano) → review+
Status: NEW → ASSIGNED
OS: Mac OS X → Gonk (Firefox OS)
Priority: -- → P1
Hardware: x86 → ARM
Whiteboard: [c=handeye p= s= u=][ETA=2/14]
Target Milestone: --- → 1.4 S1 (14feb)
blocking-b2g: --- → 1.3?
1.3+ as this blocks existing 1.3 blocker Bug 942750: Gaia Checkerboarding.
blocking-b2g: 1.3? → 1.3+
Whiteboard: [c=handeye p= s= u=][ETA=2/14] → [c=handeye p= s= u=1.3][ETA=2/14]
Whiteboard: [c=handeye p= s= u=1.3][ETA=2/14] → [c=handeye p= s=2014.02.14 u=1.3] [ETA=2/14]
Attached patch bug967277.followup.import.patch (deleted) — — Splinter Review
Seems like something was not caught by tests for imports.
Attachment #8371644 - Flags: review?(francisco.jordano)
Comment on attachment 8371644 [details] [diff] [review] bug967277.followup.import.patch Review of attachment 8371644 [details] [diff] [review]: ----------------------------------------------------------------- Tested, now working for the importer lists (fb, gmail and hotmail)
Attachment #8371644 - Flags: review?(francisco.jordano) → review+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: