Closed Bug 967277 Opened 7 years ago Closed 7 years ago

[Contacts] Scrolling the contacts app causes reflow

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

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

RESOLVED FIXED
1.4 S1 (14feb)
blocking-b2g 1.3+
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)

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.patchSplinter 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]
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.