1253078 - Switch default environmental variable behavior for stylo builds

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Bobby Holley (:bholley)]

Right now, when doing a stylo build, you also need to pass MOZ_STYLO=1 as an environmental variable to enable it. This is yet another thing that people need to remember to do, and is likely to be a source of confusion going forward.

I do see some value in being able to dynamically turn off the runtime stylo behavior on an existing stylo build, but let's make that explicit, rather than the default.

Comment 1

[Bobby Holley (:bholley)]

Attachment: Switch MOZ_STYLO environmental variable to MOZ_DISABLE_STYLO. v1

Comment 2

[Daniel Holbert [:dholbert]]

Comment on attachment 8725919 [details] [diff] [review]
Switch MOZ_STYLO environmental variable to MOZ_DISABLE_STYLO. v1

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

r=me; just one nit:

::: layout/base/nsPresContext.h
@@ +1088,5 @@
>    static bool StyloEnabled()
>    {
>  #ifdef MOZ_STYLO
> +    static bool disabled = PR_GetEnv("MOZ_DISABLE_STYLO");
> +    return !disabled;

Maybe worth adding a comment (outside the ifdef) to sum up the intended behavior here, in English, since the multiple variables + double negation of "!disabled" requires a little bit of staring to figure out.

Something like:
  // Stylo is primarily enabled/disabled at build-time.  (And in stylo-enabled
  // builds, we also allow it to be disabled at run-time via an env variable.)

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1a1d1594492d

Comment 4

[Daniel Holbert [:dholbert]]

Comment looks good -- thanks!

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1a1d1594492d