1372549 - Avoid generating reflow change hint from list-style-type if display is not list-item

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[Xidorn Quan [:xidorn] UTC+10]

Value of list-style-type and list-style-position only affects elements whose display is list-item. It doesn't make much sense to trigger reflow / reframe if display is not that.

This optimization may not be something very useful for Gecko (since it would only optimize out cases if someone changes these values in an high ancestor from actual list items), but it may benefit Stylo more.


In bug 1371976, I'm going to make counter style get resolved during parallel traversal when possible (when the corresponding item has been queried and cached in the table), and leave it unresolved otherwise. There is a problem, then.

For list items, their counter style would get resolved when they are reflowed, and in following restyles, they would always get the same resolved counter style from traversal, so the comparison would always return no change, which is good.

However, for non-list-item elements, their counter style would be left unresolved. And in the next restyle, if the same counter style has been resolved by some other elements in this document, those elements would get a resolved style this time, which would trigger a reflow in the current settings. It is unfortunate.


If we can avoid generating those change hints for non-list-item elements, we can avoid this problem.

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1372549 part 1 - Slightly simplify DO_STRUCT_DIFFERENCE macro usage.

Review commit: https://reviewboard.mozilla.org/r/148448/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/148448/

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1372549 part 2 - Avoid return reframe / reflow for list-style-{type,position} if display is not list-item.

Review commit: https://reviewboard.mozilla.org/r/148450/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/148450/

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

There is one problem after this change, though, that if someone sets list-style-type directly on ::-moz-list-{bullet,number} pseudo-element, the behavior could be buggy. But I believe it should have already been buggy for some specific combinations.

To fix that, we need to make nsBulletFrame use StyleList of its parent frame rather than that of itself. I don't think this matters too much, though.

Comment 4

[Cameron McCormack (:heycam)]

Comment on attachment 8877109 [details]
Bug 1372549 part 1 - Slightly simplify DO_STRUCT_DIFFERENCE macro usage.

https://reviewboard.mozilla.org/r/148448/#review153284

Comment 5

[Cameron McCormack (:heycam)]

Comment on attachment 8877110 [details]
Bug 1372549 part 2 - Avoid return reframe / reflow for list-style-{type,position} if display is not list-item.

https://reviewboard.mozilla.org/r/148450/#review153292

::: layout/style/nsStyleStruct.cpp:700
(Diff revision 1)
> +  // list-style-position and list-style-type. If the old display struct
> +  // doesn't exist, assume it isn't affected by display value at all,
> +  // and thus these properties should not affecti either.

s/affecti/affect it/

I guess we're also relying on the fact that if the new display value is list-item, that we needed to have a change in display value, and we would return ReconstuctFrame from that.  Can you comment about that here too?

(It seems unlikely to me that PeekStyleDisplay will ever return null, btw.)

::: layout/style/nsStyleStruct.cpp:710
(Diff revision 1)
> -    if (mImageRegion.width == aNewData.mImageRegion.width &&
> -        mImageRegion.height == aNewData.mImageRegion.height) {
> +  } else if (mListStylePosition != aNewData.mListStylePosition ||
> +        mCounterStyle != aNewData.mCounterStyle) {

Nit: fix the indenting here.

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8877110 [details]
Bug 1372549 part 2 - Avoid return reframe / reflow for list-style-{type,position} if display is not list-item.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/148450/diff/1-2/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/90e6e7906b2f
part 1 - Slightly simplify DO_STRUCT_DIFFERENCE macro usage. r=heycam
https://hg.mozilla.org/integration/autoland/rev/ee7918847606
part 2 - Avoid return reframe / reflow for list-style-{type,position} if display is not list-item. r=heycam

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/90e6e7906b2f
https://hg.mozilla.org/mozilla-central/rev/ee7918847606