Closed Bug 1550377 Opened 1 year ago Closed 1 year ago

Use ManuallyDrop in the rust representation of style structs.

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

Details

Attachments

(1 file)

This allows us to not run destructors of any members of nsStyle*, which in turn allows us to:

  • Remove the hack that replaced all nsStrings for nsStringReprs.
  • Remove ns{,C}StringRepr (followup)
  • Add members with destructors to the style structs (you see where I'm going :)).

We destroy them manually, so it's the right thing to do.

This allows us to not run destructors of any members of nsStyle*, which in turn allows us to:

  • Remove the hack that replaced all nsStrings for nsStringReprs.
  • Remove ns{,C}StringRepr (followup)
  • Add members with destructors to the style structs (you see where I'm going :)).
Blocks: 1550554

(In reply to Emilio Cobos Álvarez (:emilio) from comment #1)

  • Add members with destructors to the style structs (you see where I'm going :)).

Can you say what your desired end state is here? That would help in reviewing the direction of these patches. I'm guessing it's towards using more and more Rust types for the style struct members, or perhaps defining the style structs entirely on the Rust side?

Flags: needinfo?(emilio)

Yes. Generally this allows to have rust-defined slices and such in style structs (see bug 1550554 for an example where this allows us to avoid doing silly copies for box shadows and such and clean up a bunch of stuff).

But I think regardless of that, this patch is necessary. What we have right now is a real footgun (see the nsStringRepr stuff to avoid double-freeing strings in style structs on drop).

Defining eventually all the structs in rust would be great. For now I'm just trying to remove the most complex glue code as time and such permits.

Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2d0e8bded97a
Use ManuallyDrop for style structs. r=jwatt
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/21897fb59491
Use ManuallyDrop for style structs. r=jwatt
Flags: needinfo?(emilio)
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.