Bug 1995268 - Make StylesheetContents more obviously immutable. r=nicoburns,oriol,#style
ClosedPublic
Actions

Authored by emilio on Oct 20 2025, 12:20 PM.

Details

Reviewers

nicoburns
Oriol

Group Reviewers

Restricted Project

Commits

rFIREFOXAUTOLANDad632bac51fd: Bug 1995268 - Make StylesheetContents more obviously immutable. r=nicoburns…

Bugzilla Bug ID

1995268

Summary

I want to update the UrlExtraData of the stylesheet contents when
cloning for uniqueness.

This removes an API that servo uses but that's easily replaceable, see:
https://github.com/servo/servo/pull/40024

Diff Detail

Repository: rFIREFOXAUTOLAND firefox-autoland

Event Timeline

emilio created this revision.Oct 20 2025, 12:20 PM

Herald added a project: secure-revision. · View Herald TranscriptOct 20 2025, 12:20 PM

phab-bot published this revision for review.Oct 20 2025, 12:20 PM

phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".

phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".

phab-bot removed a project: secure-revision.

Herald added a subscriber: zrhoffman. · View Herald TranscriptOct 20 2025, 12:20 PM

Harbormaster completed remote builds in B862747: Diff 1141218.Oct 20 2025, 12:32 PM

Blitz is using this too: https://github.com/DioxusLabs/blitz/blob/29f42ab026ef4c35154c5147c09b7269b04fd4c6/packages/blitz-dom/src/net.rs#L110
But probably it shouldn't be hard to replace?

servo/components/style/stylesheets/stylesheet.rs
476	This comment was already obsolete, but can be removed while you are at it.

This revision is now accepted and ready to land.Oct 20 2025, 12:36 PM

This change makes sense to me. Removing the RwLocks is nice. The only bit I didn't understand was this comment:

I want to update the UrlExtraData of the stylesheet contents when cloning for uniqueness.

Does this mean "I only want to update UrlExtraData when/by cloning (and not by mutating)?" (copy-on-write semantics)
As written, it makes it sounds to me like you are adding a mutation to UrlExtraData where there wasn't previously one.

In D269200#9313505, @Oriol wrote:

Blitz is using this too: https://github.com/DioxusLabs/blitz/blob/29f42ab026ef4c35154c5147c09b7269b04fd4c6/packages/blitz-dom/src/net.rs#L110
But probably it shouldn't be hard to replace?

Blitz already migrated off this api in response to Stylo#250

emilio updated this revision to Diff 1141252.Oct 20 2025, 1:07 PM

In D269200#9313509, @nicoburns wrote:

This change makes sense to me. Removing the RwLocks is nice. The only bit I didn't understand was this comment:

I want to update the UrlExtraData of the stylesheet contents when cloning for uniqueness.

Does this mean "I only want to update UrlExtraData when/by cloning (and not by mutating)?" (copy-on-write semantics)
As written, it makes it sounds to me like you are adding a mutation to UrlExtraData where there wasn't previously one.

Yeah, to fix bug 1994008 I want for the StylesheetContents copy-on-write to pull the "right" base URI directly.

Harbormaster completed remote builds in B862781: Diff 1141252.Oct 20 2025, 1:22 PM