Closed Bug 921683 Opened 6 years ago Closed 6 years ago

dynamic creation of contact list groups causes reflow


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

Gonk (Firefox OS)
Not set


(b2g-v1.2 fixed)

1.2 C3(Oct25)
Tracking Status
b2g-v1.2 --- fixed


(Reporter: bkelly, Assigned: bkelly)



(Keywords: perf, Whiteboard: [c= p=3 s=2013.10.04 u=1.2],QARegressExclude)


(1 file)

Profiling during bug 914913 shows that there is a small reflow after the dynamic creation of the list group DOM structures in list.js's renderGroupHeader().

While we might get a small win by making this static, we can probably defer the creation of the group list DOM until we have a contact that needs to go in that list.  This should avoid both the reflow and help reduce the DOM.
blocking-b2g: --- → koi?
Whiteboard: [c= p=3 s= u=] → [c= p=3 s=2013.10.04 u=]
triage: koi+ for improving contact performacne
blocking-b2g: koi? → koi+
Working code is here:

I need to fix the tests, though, as they are all pretty much written to expect the group list DOM to already exist.  Sadness.  :-(
Also, I believe this profile confirms that this change does indeed prevent the reflow after the contacts init:
My contacts-dynamic-group branch linked in comment 2 has been updated with fixes for most of the tests.

One test is still failing.  Debugging suggests this might actually be a bug in tag_visibility_monitor, so it will take longer to figure out.
Finally tracked down that last test issue.  It was not a bug in visibility monitor, but instead a problem due to the test DOM missing some CSS which forced scrolling to occur.

Here is the pull request.  This defers the group creation until its first needed.  Profiling shows that this finally removes the reflow after the contacts.List.init() function.

Again, it currently hardcodes the expected names of the groups in order to provide ordering.  I did this partly in anticipation of adding more scripts.
Attachment #813806 - Flags: review?(jmcf)
Attachment #813806 - Attachment mime type: text/plain → text/html
Comment on attachment 813806 [details]
Pull request at

good work Ben

Attachment #813806 - Flags: review?(jmcf) → review+
Finally got travis green.  Landed:

Thanks for the review Jose!
Closed: 6 years ago
Resolution: --- → FIXED
Uplifted cbb6174cd51222fbc3d3b1d4e12620666aceca38 to:
v1.2: e9533a66b441b902a77966da87144f89e1db808e
Whiteboard: [c= p=3 s=2013.10.04 u=] → [c= p=3 s=2013.10.04 u=],QARegressExclude
Target Milestone: --- → 1.2 C3(Oct25)
Whiteboard: [c= p=3 s=2013.10.04 u=],QARegressExclude → [c= p=3 s=2013.10.04 u=1.2],QARegressExclude
Blocks: 914942
We also need to uplift this patch to v1-train in order to uplift bug 914942.
Do we need to nom for leo? then?
is v1-train still an active branch?
Bug 914942 is leo+ and it depends on this patch now…

I think we should uplift this patch if it’s straight-forward; otherwise I can still work on a leo-specific patch for bug 914942.
I think this should probably uplift cleanly.  I'm more wondering who can give us leo+ ?  Is there a manager you are working with aware of the effort that needs this uplift?
John, are you the right person to ask?
Flags: needinfo?(jhford)
I'll go ahead an nom for leo? based on comment 9.  Just need someone to make a call on it.
blocking-b2g: koi+ → leo?
Also needinfo Preeti about whether we can leo+ this bug since it blocks bug 914942.
Flags: needinfo?(praghunath)
Hi Ben,

leo has already shipped so we cannot leo+ this bug.

We can however 1.1HD+ it. Let me know.
(In reply to Preeti Raghunath(:Preeti) from comment #17)
> leo has already shipped so we cannot leo+ this bug.
> We can however 1.1HD+ it. Let me know.

Would that let us uplift it to v1-train?  Is there going to be a 1.1.1 release and which branch/flag would that use?

Thanks for the quick reply.
See bug 914942 comment 20, I think I should probably write another patch rather than request a bunch of dependency uplifts.
Ok. Dropping leo? nom based on comment 19.  Note, I could not put the flag back at koi+.
blocking-b2g: leo? → ---
Flags: needinfo?(praghunath)
Flags: needinfo?(jhford)
You need to log in before you can comment on or make changes to this bug.