1466609 - Followups to various recent bugs.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[Emilio Cobos Álvarez (:emilio)]

Just things I told xidorn I'll do in followups :)

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1466609: #[parse(..)].

I need to admit I'm ambivalent about this one :).

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

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1466609: Make the threadsafe refcounting macros more reusable.

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

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1466609: Make clearing atoms slightly more ergonomic.

I prefer to do it this way because Atom has inline paths for static atoms and
such.

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

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8983138 [details]
Bug 1466609: #[parse(..)].

https://reviewboard.mozilla.org/r/248972/#review255640

::: servo/components/style/parser.rs:164
(Diff revision 1)
> -///    the value conditional on `function`, which will be invoked with a
> +///    value conditional on `function`, which will be invoked with a
>  ///    `&ParserContext` reference.

I think it would probably be clearer to write down the function signature directly, like:
> which should match type `fn(&ParserContext) -> bool`.

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8983139 [details]
Bug 1466609: Make the threadsafe refcounting macros more reusable.

https://reviewboard.mozilla.org/r/248974/#review255646

::: servo/components/style/gecko_bindings/sugar/refptr.rs:278
(Diff revision 1)
>  // Companion of NS_DECL_THREADSAFE_FFI_REFCOUNTING.
>  //
>  // Gets you a free RefCounted impl implemented via FFI.
>  macro_rules! impl_threadsafe_refcount {
> -    ($t:ty, $addref:ident, $release:ident) => {
> +    ($t:ty, $addref:path, $release:path) => {
>          impl_refcount!($t, $addref, $release);

We can actually fold `impl_refcount` into this macro nowadays since there are no longer any non-threadsafe refcount... but it's not a big deal, and maybe we will add another in the future.

Comment 6

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8983140 [details]
Bug 1466609: Make clearing atoms slightly more ergonomic.

https://reviewboard.mozilla.org/r/248976/#review255648

Thanks!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9889304ebdea
Move some parsing-only attributes to use #[parse(..)] instead of #[css(..)]. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/4607eea0048d
Make the threadsafe refcounting macros more reusable. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/f7785e8b5002
Make clearing atoms slightly more ergonomic. r=xidorn

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/9889304ebdea
https://hg.mozilla.org/mozilla-central/rev/4607eea0048d
https://hg.mozilla.org/mozilla-central/rev/f7785e8b5002