Closed Bug 1362991 Opened 3 years ago Closed 3 years ago

stylo: Compute a single text style context for all the text children of the same element

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

We can do it quite easily, and it seems it's a pretty much free win.
Comment on attachment 8865429 [details]
Bug 1362991: Pass StyleSet by reference in ServoRestyleManager.

https://reviewboard.mozilla.org/r/137078/#review140380

I don't think we should do this, as it goes against the prevailing style of the codebase.  We tend to use references just for outparams (and inout), and even that goes against an existing coding style rule.  If we do want to move towards a codebase that uses references instead of pointers, it deserves a discussion on dev-platform resulting in something explicit in the style guide.
Attachment #8865429 - Flags: review?(cam) → review-
Comment on attachment 8865430 [details]
Bug 1362991: Compute at most one text style context per element.

https://reviewboard.mozilla.org/r/137080/#review140392

::: layout/base/ServoRestyleManager.cpp:142
(Diff revision 1)
>   * This is currently used to properly compute change hints when the parent
>   * element of this node is a display: contents node.

Please update this comment to say that we also use it to avoid performing duplicate on sibling text nodes.
Attachment #8865430 - Flags: review?(cam) → review+
(In reply to Cameron McCormack (:heycam) from comment #3)
> Comment on attachment 8865429 [details]
> Bug 1362991: Pass StyleSet by reference in ServoRestyleManager.
> 
> https://reviewboard.mozilla.org/r/137078/#review140380
> 
> I don't think we should do this, as it goes against the prevailing style of
> the codebase.  We tend to use references just for outparams (and inout), and
> even that goes against an existing coding style rule.  If we do want to move
> towards a codebase that uses references instead of pointers, it deserves a
> discussion on dev-platform resulting in something explicit in the style
> guide.

I sent https://groups.google.com/forum/#!topic/mozilla.dev.platform/_jfnwDvcvN4

Will revert this for now :)
Attachment #8865429 - Attachment is obsolete: true
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/42e6c5b52136
Compute at most one text style context per element. r=heycam
https://hg.mozilla.org/mozilla-central/rev/42e6c5b52136
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.