Closed Bug 1251496 Opened 5 years ago Closed 5 years ago

Forward modifications of the StyleSet to Servo

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: bholley, Assigned: bholley)

References

Details

Attachments

(3 files)

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.
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+
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.