stylo: Need to expose restyled element count to nsIDOMWindowUtils

NEW
Unassigned

Status

()

Core
CSS Parsing and Computation
P2
normal
3 months ago
5 days ago

People

(Reporter: emilio, Unassigned)

Tracking

(Blocks: 2 bugs)

Firefox Tracking Flags

(Not tracked)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

3 months ago
I was checking out the remaining test failures in test_media_queries_dynamic.html after bug 1328652, and turns out we don't change nsPresContext::mElementsRestyled, so tests fail (even though the result is correct).

We should probably do this.
Priority: -- → P2
Blocks: 1321197
FWIW, we talked about this in taipei, and I think we concluded that the tests really just wanted to know whether any elements were restyled at all, so we might be able to simplify things a bit. Otherwise we'd need to turn traversal statistics on by default, which isn't so bad, except for aggregating the statistics for all the worker threads might be expensive.
(Reporter)

Comment 2

5 days ago
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #1)
> FWIW, we talked about this in taipei, and I think we concluded that the
> tests really just wanted to know whether any elements were restyled at all,
> so we might be able to simplify things a bit. Otherwise we'd need to turn
> traversal statistics on by default, which isn't so bad, except for
> aggregating the statistics for all the worker threads might be expensive.

We could also do it in the post-traversal I guess, and we wouldn't have to worry about worker threads and all that.
Comment hidden (mozreview-request)

Comment 4

5 days ago
mozreview-review
Comment on attachment 8870425 [details]
Bug 1347270: Count restyled elements in ServoRestyleManager.

https://reviewboard.mozilla.org/r/141870/#review145546

As trivial as this is, I'm not really happy about keeping inaccurate statistics.

How about we just remove the field and have the getter return the restyle generation? That seems to be all the tests needs.

::: layout/base/ServoRestyleManager.cpp:325
(Diff revision 1)
>    // which consumes the new style and expects the old style to be on the frame.
>    //
>    // XXXbholley: We should teach the frame constructor how to clear the dirty
>    // descendants bit to avoid the traversal here.
>    if (changeHint & nsChangeHint_ReconstructFrame) {
> +    mPresContext->RestyledElement();

This isn't really accurate - we restyled all the elements in the subtree too.

::: layout/base/ServoRestyleManager.cpp:368
(Diff revision 1)
>    if (recreateContext) {
>      MOZ_ASSERT(styleFrame || displayContentsNode);
> +    mPresContext->RestyledElement();

This also isn't accurate, because we may still have restyled other things and ended up with the same style context.
Attachment #8870425 - Flags: review?(bobbyholley) → review-
You need to log in before you can comment on or make changes to this bug.