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

RESOLVED FIXED in Firefox 55

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: emilio, Assigned: emilio)

Tracking

(Blocks: 1 bug)

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

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

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

a year ago
We can do it quite easily, and it seems it's a pretty much free win.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 3

a year ago
mozreview-review
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 4

a year ago
mozreview-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+
(Assignee)

Comment 5

a year ago
(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 :)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8865429 - Attachment is obsolete: true
Comment hidden (mozreview-request)

Comment 8

a year ago
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/42e6c5b52136
Compute at most one text style context per element. r=heycam

Comment 9

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/42e6c5b52136
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.