1267560 - get style structs from ServoComputedValues rather than the rule node, when using the Servo-backed style system

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Cameron McCormack (:heycam)]

Attachment: patch

Uses the functions in https://github.com/servo/servo/pull/10848.

Comment 1

[Bobby Holley (:bholley)]

Comment on attachment 8745240 [details] [diff] [review]
patch

Review of attachment 8745240 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with those fixes.

::: layout/style/nsStyleContext.h
@@ +519,5 @@
>  
>    void ApplyStyleFixups(bool aSkipParentDisplayBasedStyleFixup);
>  
>    const void* StyleStructFromServoComputedValues(nsStyleStructID aSID) {
> +#ifdef MOZ_STYLO

Why does this need to be ifdef MOZ_STYLO? I don't see any reason for it here, and it would be nice to minimize ifdefs.

@@ +589,5 @@
> +      } else {                                                          \
> +        newData =                                                       \
> +          Servo_GetStyle##name_(mSource.AsServoComputedValues());       \
> +        /* the Servo-backed StyleContextSource owns the struct */       \
> +        AddStyleBit(NS_STYLE_INHERIT_BIT(name_));                       \

If this bit really just means "somebody else owns the struct", then can we get a second patch to rename it to NS_STYLE_STRUCT_BORROWED_BIT or something instead? Otherwise it's quite confusing, since servo will be using this bit for both inherit and reset structs.

Comment 2

[Cameron McCormack (:heycam)]

It was to avoid link errors, but really I should add stubs to ServoBindings.cpp instead.  I'll do that.

Comment 3

[Cameron McCormack (:heycam)]

(In reply to Bobby Holley (busy) from comment #1)
> If this bit really just means "somebody else owns the struct", then can we
> get a second patch to rename it to NS_STYLE_STRUCT_BORROWED_BIT or something
> instead? Otherwise it's quite confusing, since servo will be using this bit
> for both inherit and reset structs.

You're right, it is confusing.  The use of those bits in nsStyleContext::mBits will disappear in bug 1188721, but I've filed bug 1267866 to rename it.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4211911ef5df

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4211911ef5df