1281393 - stylo: Decide style system backend type for documents earlier

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Cameron McCormack (:heycam)]

Attachment: patch

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.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=62051343b69d

Comment 1

[Cameron McCormack (:heycam)]

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

Test failures to look into.

Comment 2

[Cameron McCormack (:heycam)]

Attachment: patch v1.1

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d5816dc3bb05

Comment 3

[Bobby Holley (:bholley)]

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.

Comment 4

[Bobby Holley (:bholley)]

ISTR this landing?

Comment 5

[Bobby Holley (:bholley)]

This landed with a wrong bug number (bug 1285474).