Forward modifications of the StyleSet to Servo

RESOLVED FIXED in Firefox 47

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

unspecified
mozilla47
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(3 attachments)

Assignee

Description

3 years ago
Servo needs to know about the active stylesheets, so it either needs to ask Gecko each time, or we need to maintain the state on both sides. Given that modifications to the styleset are comparatively rare, we do the latter.
Assignee

Comment 1

3 years ago
I think we're going to have a fair number of things like this, so I'd rather
put them all in one place, rather that defining them in the header for the first
consumer that uses the type.
Attachment #8723919 - Flags: review?(cam)
Attachment #8723919 - Flags: review?(cam) → review+
Comment on attachment 8723920 [details] [diff] [review]
Part 2 - Introduce a servo-side data structure to represent the style set. v1

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

::: layout/style/ServoBindingHelpers.h
@@ +23,5 @@
>    }
>  };
>  
> +template<>
> +class DefaultDelete<RawServoStyleSet>

I guess from the name (and the comment in UniquePtr.h) that DefaultDelete wasn't intended to be specialized.  Might be worth filing a bug to update the UniquePtr.h comments to explain that it can be specialized (and maybe to change it to a more trait-like interface)?
Attachment #8723920 - Flags: review?(cam) → review+
Comment on attachment 8723921 [details] [diff] [review]
Part 3 - Forward stylesheet management to RawServoStyleSet. v1

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

::: layout/style/ServoStyleSet.cpp
@@ +202,5 @@
>  ServoStyleSet::AddDocStyleSheet(ServoStyleSheet* aSheet,
>                                  nsIDocument* aDocument)
>  {
> +  // XXXbholley: Implement this.
> +  NS_WARNING("stylo: no support for adding doc stylesheets to ServoStyleSet");

FWIW I've been using NS_ERROR rather than NS_WARNING for things like this, just so that once we get to running tests in automation it will cause a failure due to an assertion triggering.

::: layout/style/ServoStyleSheet.h
@@ +62,5 @@
>    void DropSheet();
>  
>    RefPtr<RawServoStyleSheet> mSheet;
> +
> +  nsIDocument* mDocument;

Does this need to be added in this patch?
Attachment #8723921 - Flags: review?(cam) → review+
Assignee

Comment 7

3 years ago
I added a comment to UniquePtr.h at heycam's request. This probably doesn't warrant a review (since it's just a comment), but NI Nathan just to have a peek:

https://hg.mozilla.org/try/diff/ddc5e3e7dc4f/mfbt/UniquePtr.h
Flags: needinfo?(nfroyd)
(In reply to Bobby Holley (busy) from comment #7)
> I added a comment to UniquePtr.h at heycam's request. This probably doesn't
> warrant a review (since it's just a comment), but NI Nathan just to have a
> peek:
> 
> https://hg.mozilla.org/try/diff/ddc5e3e7dc4f/mfbt/UniquePtr.h

Thank you!
Flags: needinfo?(nfroyd)
You need to log in before you can comment on or make changes to this bug.