1075482 - [statusbar] Statusbar causing lots of unnecessary reflows

Status: RESOLVED WONTFIX

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

Description

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Whenever an icon visibility in the statusbar is updated, at least one reflow happens most of the time, since icons can have dynamic sizes. Because of that, we use `getComputedStyle` and clientWidth to get the size, causing reflows.

That happens *very* often, and I believe we could cache the size the first time we retrieve the generated icon size to not have to cause reflows on the next time.

Comment 1

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Attachment: Github PR

This patch solves two issues:
  - Uses offsetWidth instead of (clientWidth + marginLeft + marginRight), avoiding using getComputedStyle so often.
  - Caches the width value the first time it is retrieved.

Comment 2

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Attachment: Github PR

Comment 3

[Etienne Segonzac (:etienne)]

Comment on attachment 8498759 [details] [review]
Github PR

There are only two hard things in Computer Science... :)

We should only go through the |getComputedStyle| call for elements with variable widths (as in variable over time, mainly time/date displays I think).
So we need to invalidate the cache at some point, and I don't have any good heuristic suggestion :/

Guillaume, is what I'm saying correct? Do you have any ideas?

Comment 4

[Guillaume Marty [:gmarty]]

I'm not sure if caching icons width is the easiest way to go because of the invalidation issue you're pointing out.
I've started applying a super easy optimisation for Bug 1074683 (see patch attached). I only call the icon reprioritisation function if an icon visibility changed. Doing so reduced the number of some reflows from 25 down to only 2.

What about applying this optimisation to every icons with a defined width and for the other ones, let's only reprioritise if their content changed.

Comment 5

[Guillaume Marty [:gmarty]]

Also related to https://bugzilla.mozilla.org/show_bug.cgi?id=1074683

Comment 6

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

I see the problem, thanks for explaining.

Regardless of caching, I believe getComputedStyle is not needed if we look at offsetWidth instead of clientWidth, no?

Guillaume, looks indeed like your patch could be easily generalized for all icons.

Comment 7

[Guillaume Marty [:gmarty]]

In bug 1071512 landed, I cache the width of the date element, so this should have a positive impact on the number of reflows. But some work is still required though.

Comment 8

[Guillaume Marty [:gmarty]]

Attachment: Github PR

I experimented with the idea of caching. In this patch, the time DOM element width is cached and refreshed every minute.

We should see significant improvement when using both this patch and the on in Bug 1074683.
Let's not merge this patch for now. Let's see how Bug 1074683 goes and I'll add more optimisations here.

Comment 9

[BMO Automation]

Firefox OS is not being worked on