Bootstrapping OMTP requires two pieces: some boilerplate to create a thread and establish ownership of the thread, and hooking up the feature to the config/about:support/crash annotation systems. This bug is for the latter part: adding the config pref, adding decision logic about whether or not to enable OMTP, and then communicating the decision to about:support and other processes. The exact steps are roughly: 1. Add a line to gfxFeature.h [2]. "OMTP" and "Off Main Thread Painting" are probably fine for a feature name/description. gfxFeature is used to track the decision logic of enabling/disabling features, and gets communicated automatically to about:support. 2. Add a line to gfxVars [3]. These are variables that are automatically communicated to other processes. We'll want something like "UseOMTP", so content processes know whether or not to create a painting thread. 3. Add some logic to gfxPlatform [4] to set up all this state. InitWebrenderConfig() is a good, albeit complicated example of what we want. The logic should go something like this: - Create a "ScopedGfxFeatureReporter" for OMTP. - If in a content process, early return. - Check whether a "layers.omtp.enabled" pref is set to true - with a default of false. - If it's false, for now just early return. We won't show anything in about:support. - Otherwise the pref is true. Set the gfxFeature status to DisableByDefault, and then UserEnabled. This will make it appear in about:support that the feature is not normally enabled and the user wants it on. - If InSafeMode() is true, set the feature to forcefully disabled. - At the end, if the gfxFeature status is still enabled, set the reporter to successful. (This will attach a note to crash reports, so we can correlate crashes with features.) [1] http://searchfox.org/mozilla-central/source/gfx/thebes/gfxPrefs.h [2] http://searchfox.org/mozilla-central/source/gfx/config/gfxFeature.h [3] http://searchfox.org/mozilla-central/source/gfx/config/gfxVars.h [4] http://searchfox.org/mozilla-central/rev/1a0d9545b9805f50a70de703a3c04fc0d22e3839/gfx/thebes/gfxPlatform.cpp#695

Nit: Remove double-space

Comment on attachment 8874608 [details] [diff] [review] bug1369542.patch Review of attachment 8874608 [details] [diff] [review]: ----------------------------------------------------------------- Looks good but needs one more revision for the InSafeMode thing. ::: gfx/thebes/gfxPlatform.cpp @@ +2401,5 @@ > +gfxPlatform::InitOMTP() > +{ > + bool prefEnabled = Preferences::GetBool("layers.omtp.enabled", false); > + > + if (!prefEnabled) { nit: Add a comment saying something like, "We don't want to report anything for this feature when turned off since it is still early in development." @@ +2431,5 @@ > + gfxVars::SetUseOMTP(true); > + reporter.SetSuccessful(); > + } > + > + if (InSafeMode()) { This block should come after the UserEnable, and before the IsEnabled+SetSuccessful block, since it can turn OMTP off. ::: gfx/thebes/gfxPlatform.h @@ +826,5 @@ > > void InitCompositorAccelerationPrefs(); > void InitGPUProcessPrefs(); > void InitWebRenderConfig(); > + void InitOMTP(); nit: rename to InitOMTPConfig

Address both nits, move the InSafeMode check/block, and actually call the InitOMTPConfig() method.

Comment on attachment 8875053 [details] [diff] [review] bug1369542.patch Review of attachment 8875053 [details] [diff] [review]: ----------------------------------------------------------------- Looks good!

Try submission link for bug 1369549 (https://bugzilla.mozilla.org/show_bug.cgi?id=1369549#c12) also applies to this bug, so when that try finishes (assuming no errors) this should be able to be checked in.

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/8adb463b2631 Hook up OMTP to config/about:support/crash annotation system. r=dvander