stylo: Decide style system backend type for documents earlier




3 years ago
2 years ago


(Reporter: heycam, Assigned: heycam)


(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)



(1 attachment, 1 obsolete attachment)



3 years ago
Created attachment 8764145 [details] [diff] [review]

Currently we decide whether to use a Servo- or a Gecko-backed style system at the time we create the nsDocumentViewer.  This assumes that the document is being presented (which turns out not to be true for all documents that get Loaders, style sets, etc., which need to know the backend type), resulting in many warnings on startup.  This patch makes the document the ultimate decider of the backend type, rather than the nsDocumentViewer, but still delays that decision until the first GetStyleBackendType call on it.  This still allows us to choose, for now, to use Servo for documents that will be in content docshells without having the docshell set on the document yet, since we can call doc->IsContentDocument() to get the same information.
Attachment #8764145 - Flags: review?(bobbyholley)

Comment 1

3 years ago
Comment on attachment 8764145 [details] [diff] [review]

Test failures to look into.
Attachment #8764145 - Flags: review?(bobbyholley)

Comment 2

3 years ago
Created attachment 8764242 [details] [diff] [review]
patch v1.1
Assignee: nobody → cam
Attachment #8764145 - Attachment is obsolete: true
Attachment #8764242 - Flags: review?(bobbyholley)
Comment on attachment 8764242 [details] [diff] [review]
patch v1.1

Review of attachment 8764242 [details] [diff] [review]:

r=me with that.

::: dom/base/nsIDocument.h
@@ +1071,5 @@
>      return mCSSLoader;
>    }
> +  mozilla::StyleBackendType GetStyleBackendType() const {
> +    if (mStyleBackendType == mozilla::StyleBackendType(0)) {

This mechanism feels too hacky to me. How about just storing a Maybe<StyleBackendType> and checking whether it's been initialized? Then we can nix the "= 1" in the enum class.

@@ +1077,5 @@
> +    }
> +    MOZ_ASSERT(mStyleBackendType != mozilla::StyleBackendType(0));
> +    return mStyleBackendType;
> +  }
> +  void UpdateStyleBackendType();

Let's call this ComputeStyleBackendType. And add a comment explaining why we need to do this lazily?

::: layout/style/StyleBackendType.h
@@ +13,5 @@
>   * Enumeration that represents one of the two supported style system backends.
>   */
>  enum class StyleBackendType : int
>  {
> +  Gecko = 1,

Per above let's revert this.
Attachment #8764242 - Flags: review?(bobbyholley) → review+
ISTR this landing?
Flags: needinfo?(cam)
Summary: decide style system backend type for documents earlier → stylo: Decide style system backend type for documents earlier
This landed with a wrong bug number (bug 1285474).
Last Resolved: 2 years ago
Flags: needinfo?(cam)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.