Switch default environmental variable behavior for stylo builds

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
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

3 years ago
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 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.)
Attachment #8725919 - Flags: review?(dholbert) → review+
Comment looks good -- thanks!
Flags: in-testsuite-

Comment 5

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1a1d1594492d
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.