1366247 - stylo: Add sugar for already_AddRefed

Status: RESOLVED INVALID

(Core :: CSS Parsing and Computation, enhancement)

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 1366247 - Add sugar for already_AddRefed and use it for conversion between Atom and nsIAtom pointer.

Review commit: https://reviewboard.mozilla.org/r/141064/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/141064/

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

I don't change methods of RefPtr because binding functions cannot return already_AddRefed, and thus changing that only complicates existing code.

Comment 3

[Manish Goregaokar [:manishearth]]

Comment on attachment 8869425 [details]
Bug 1366247 - Add sugar for already_AddRefed and use it for conversion between Atom and nsIAtom pointer.

https://reviewboard.mozilla.org/r/141064/#review144696

::: servo/components/style/gecko_string_cache/mod.rs:353
(Diff revision 1)
> +        debug_assert!(!ptr.is_null());
> +        unsafe { Atom(WeakAtom::new(ptr)) }
> +    }
> +}
> +
> +impl Into<already_AddRefed<nsIAtom>> for Atom {

usually better to impl from than into (from implies into, into doesn't imply from). coherence should let you do it.

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8869425 [details]
Bug 1366247 - Add sugar for already_AddRefed and use it for conversion between Atom and nsIAtom pointer.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/141064/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd3a293cf053
Add sugar for already_AddRefed and use it for conversion between Atom and nsIAtom pointer. r=manishearth

Comment 6

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b5d15ecde93a
Backed out changeset dd3a293cf053 for making Stylo build, but not run

Comment 7

[Xidorn Quan [:xidorn] UTC+11]

So per emilio, this is not going to work, because already_AddRefed has destructor, which would make the two sides disagree on how arguments are passed.

I guess we still want something to represent addrefed pointer somehow...