Give FeatureState a public interface. (bug 1254899 part 8, r=milan)
☠☠ backed out by df82a3088812 ☠ ☠
authorDavid Anderson <danderson@mozilla.com>
Wed, 27 Apr 2016 22:54:26 -0700
changeset 357319 e8abac801a515dbaa2210c0a78907b9e8f6301e6
parent 357318 e9c311c68e4aa25901a874471778e1b8b98ed7ea
child 357320 993906e53329b6eb88b3f0a26507cdae0ae17be0
push id16755
push useryura.zenevich@gmail.com
push dateThu, 28 Apr 2016 15:12:20 +0000
reviewersmilan
bugs1254899
milestone49.0a1
Give FeatureState a public interface. (bug 1254899 part 8, r=milan)
gfx/config/gfxConfig.cpp
gfx/config/gfxConfig.h
gfx/config/gfxFeature.cpp
gfx/config/gfxFeature.h
gfx/src/gfxTelemetry.cpp
gfx/src/gfxTelemetry.h
gfx/thebes/gfxPlatform.cpp
--- a/gfx/config/gfxConfig.cpp
+++ b/gfx/config/gfxConfig.cpp
@@ -5,139 +5,119 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 #include "gfxConfig.h"
 
 namespace mozilla {
 namespace gfx {
 
 static gfxConfig sConfig;
 
-// When querying the current stae of a feature, assert that we initialized some
-// default value for the feature already. This ensures that the config/init
-// order is correct.
-/* static */ void
-gfxConfig::AssertStatusInitialized(Feature aFeature)
+/* static */ FeatureState&
+gfxConfig::GetFeature(Feature aFeature)
 {
-  sConfig.GetState(aFeature).AssertInitialized();
+  return sConfig.GetState(aFeature);
 }
 
 /* static */ bool
 gfxConfig::IsEnabled(Feature aFeature)
 {
-  AssertStatusInitialized(aFeature);
-
-  FeatureStatus status = GetValue(aFeature);
-  return status == FeatureStatus::Available || status == FeatureStatus::ForceEnabled;
+  const FeatureState& state = sConfig.GetState(aFeature);
+  return state.IsEnabled();
 }
 
 /* static */ bool
 gfxConfig::IsDisabledByDefault(Feature aFeature)
 {
-  AssertStatusInitialized(aFeature);
-
   const FeatureState& state = sConfig.GetState(aFeature);
   return state.DisabledByDefault();
 }
 
 /* static */ bool
 gfxConfig::IsForcedOnByUser(Feature aFeature)
 {
-  AssertStatusInitialized(aFeature);
-
   const FeatureState& state = sConfig.GetState(aFeature);
   return state.IsForcedOnByUser();
 }
 
 /* static */ FeatureStatus
 gfxConfig::GetValue(Feature aFeature)
 {
-  AssertStatusInitialized(aFeature);
-
   const FeatureState& state = sConfig.GetState(aFeature);
   return state.GetValue();
 }
 
 /* static */ bool
 gfxConfig::SetDefault(Feature aFeature,
                       bool aEnable,
                       FeatureStatus aDisableStatus,
                       const char* aDisableMessage)
 {
   FeatureState& state = sConfig.GetState(aFeature);
-  if (!aEnable) {
-    state.DisableByDefault(aDisableStatus, aDisableMessage);
-    return false;
-  }
+  return state.SetDefault(aEnable, aDisableStatus, aDisableMessage);
+}
+
+/* static */ void
+gfxConfig::DisableByDefault(Feature aFeature,
+                            FeatureStatus aDisableStatus,
+                            const char* aDisableMessage)
+{
+  FeatureState& state = sConfig.GetState(aFeature);
+  state.DisableByDefault(aDisableStatus, aDisableMessage);
+}
+
+/* static */ void
+gfxConfig::EnableByDefault(Feature aFeature)
+{
+  FeatureState& state = sConfig.GetState(aFeature);
   state.EnableByDefault();
-  return true;
 }
 
 /* static */ bool
 gfxConfig::InitOrUpdate(Feature aFeature,
                         bool aEnable,
                         FeatureStatus aDisableStatus,
                         const char* aDisableMessage)
 {
   FeatureState& state = sConfig.GetState(aFeature);
-  if (!state.IsInitialized()) {
-    return SetDefault(aFeature, aEnable, aDisableStatus, aDisableMessage);
-  }
-  return MaybeSetFailed(aFeature, aEnable, aDisableStatus, aDisableMessage);
+  return state.InitOrUpdate(aEnable, aDisableStatus, aDisableMessage);
 }
 
 /* static */ void
 gfxConfig::SetFailed(Feature aFeature, FeatureStatus aStatus, const char* aMessage)
 {
-  AssertStatusInitialized(aFeature);
-
-  // We should never bother setting a runtime status to "enabled," since it could
-  // override an explicit user decision to disable it.
-  MOZ_ASSERT(IsFeatureStatusFailure(aStatus));
-
   FeatureState& state = sConfig.GetState(aFeature);
-  state.SetRuntime(aStatus, aMessage);
+  state.SetFailed(aStatus, aMessage);
 }
 
 /* static */ void
 gfxConfig::Disable(Feature aFeature, FeatureStatus aStatus, const char* aMessage)
 {
-  AssertStatusInitialized(aFeature);
-
-  // We should never bother setting an environment status to "enabled," since
-  // it could override an explicit user decision to disable it.
-  MOZ_ASSERT(IsFeatureStatusFailure(aStatus));
-
   FeatureState& state = sConfig.GetState(aFeature);
-  state.SetEnvironment(aStatus, aMessage);
+  state.Disable(aStatus, aMessage);
 }
 
 /* static */ void
 gfxConfig::UserEnable(Feature aFeature, const char* aMessage)
 {
-  AssertStatusInitialized(aFeature);
+  FeatureState& state = sConfig.GetState(aFeature);
+  state.UserEnable(aMessage);
+}
 
-  FeatureState& state = sConfig.GetState(aFeature);
-  state.SetUser(FeatureStatus::Available, aMessage);
-}
 /* static */ void
 gfxConfig::UserForceEnable(Feature aFeature, const char* aMessage)
 {
-  AssertStatusInitialized(aFeature);
-
   FeatureState& state = sConfig.GetState(aFeature);
-  state.SetUser(FeatureStatus::ForceEnabled, aMessage);
+  state.UserForceEnable(aMessage);
 }
 
 /* static */ void
 gfxConfig::UserDisable(Feature aFeature, const char* aMessage)
 {
-  AssertStatusInitialized(aFeature);
-
   FeatureState& state = sConfig.GetState(aFeature);
-  state.SetUser(FeatureStatus::Disabled, aMessage);
+  state.UserDisable(aMessage);
 }
 
 /* static */ bool
 gfxConfig::UseFallback(Fallback aFallback)
 {
   return sConfig.UseFallbackImpl(aFallback);
 }
 
--- a/gfx/config/gfxConfig.h
+++ b/gfx/config/gfxConfig.h
@@ -20,16 +20,19 @@ namespace gfx {
 //   - An environment value, determined by system/hardware factors or nsIGfxInfo.
 //   - A runtime value, determined by any failures encountered after enabling
 //     the feature.
 //
 // Each state change for a feature is recorded in this class.
 class gfxConfig
 {
 public:
+  // Return the full state history of a feature.
+  static FeatureState& GetFeature(Feature aFeature);
+
   // Query whether a parameter is enabled, taking into account any user or
   // runtime overrides. The algorithm works as follow:
   //
   //  1. If a runtime decision disabled the feature, return false.
   //  2. If the user force-enabled the feature, return true.
   //  3. If the environment disabled the feature, return false.
   //  4. If the user specified a decision, return it.
   //  5. Return the base setting for the feature.
@@ -52,16 +55,20 @@ public:
   //  5. Return the default status.
   static FeatureStatus GetValue(Feature aFeature);
 
   // Initialize the base value of a parameter. The return value is aEnable.
   static bool SetDefault(Feature aFeature,
                          bool aEnable,
                          FeatureStatus aDisableStatus,
                          const char* aDisableMessage);
+  static void DisableByDefault(Feature aFeature,
+                               FeatureStatus aDisableStatus,
+                               const char* aDisableMessage);
+  static void EnableByDefault(Feature aFeature);
 
   // Set a environment status that overrides both the default and user
   // statuses; this should be used to disable features based on system
   // or hardware problems that can be determined up-front. The only
   // status that can override this decision is the user force-enabling
   // the feature.
   static void Disable(Feature aFeature,
                       FeatureStatus aStatus,
@@ -138,18 +145,16 @@ private:
   const FeatureState& GetState(Feature aFeature) const {
     MOZ_ASSERT(size_t(aFeature) < kNumFeatures);
     return mFeatures[size_t(aFeature)];
   }
 
   bool UseFallbackImpl(Fallback aFallback) const;
   void EnableFallbackImpl(Fallback aFallback);
 
-  static void AssertStatusInitialized(Feature aFeature);
-
 private:
   static const size_t kNumFeatures = size_t(Feature::NumValues);
 
 private:
   FeatureState mFeatures[kNumFeatures];
   uint64_t mFallbackBits;
 };
 
--- a/gfx/config/gfxFeature.cpp
+++ b/gfx/config/gfxFeature.cpp
@@ -4,43 +4,138 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 #include "gfxFeature.h"
 #include "prprf.h"
 
 namespace mozilla {
 namespace gfx {
 
+bool
+FeatureState::IsEnabled() const
+{
+  return IsFeatureStatusSuccess(GetValue());
+}
+
 FeatureStatus
 FeatureState::GetValue() const
 {
+  AssertInitialized();
+
   if (mRuntime.mStatus != FeatureStatus::Unused) {
     return mRuntime.mStatus;
   }
   if (mUser.mStatus == FeatureStatus::ForceEnabled) {
     return FeatureStatus::ForceEnabled;
   }
   if (mEnvironment.mStatus != FeatureStatus::Unused) {
     return mEnvironment.mStatus;
   }
   if (mUser.mStatus != FeatureStatus::Unused) {
     return mUser.mStatus;
   }
   return mDefault.mStatus;
 }
 
 bool
+FeatureState::SetDefault(bool aEnable,
+                         FeatureStatus aDisableStatus,
+                         const char* aDisableMessage)
+{
+  if (!aEnable) {
+    DisableByDefault(aDisableStatus, aDisableMessage);
+    return false;
+  }
+  EnableByDefault();
+  return true;
+}
+
+bool
+FeatureState::InitOrUpdate(bool aEnable,
+                           FeatureStatus aDisableStatus,
+                           const char* aDisableMessage)
+{
+  if (!IsInitialized()) {
+    return SetDefault(aEnable, aDisableStatus, aDisableMessage);
+  }
+  return MaybeSetFailed(aEnable, aDisableStatus, aDisableMessage);
+}
+
+void
+FeatureState::UserEnable(const char* aMessage)
+{
+  AssertInitialized();
+  SetUser(FeatureStatus::Available, aMessage);
+}
+
+void
+FeatureState::UserForceEnable(const char* aMessage)
+{
+  AssertInitialized();
+  SetUser(FeatureStatus::ForceEnabled, aMessage);
+}
+
+void
+FeatureState::UserDisable(const char* aMessage)
+{
+  AssertInitialized();
+  SetUser(FeatureStatus::Disabled, aMessage);
+}
+
+void
+FeatureState::Disable(FeatureStatus aStatus, const char* aMessage)
+{
+  AssertInitialized();
+
+  // We should never bother setting an environment status to "enabled," since
+  // it could override an explicit user decision to disable it.
+  MOZ_ASSERT(IsFeatureStatusFailure(aStatus));
+
+  SetEnvironment(aStatus, aMessage);
+}
+
+void
+FeatureState::SetFailed(FeatureStatus aStatus, const char* aMessage)
+{
+  AssertInitialized();
+
+  // We should never bother setting a runtime status to "enabled," since it could
+  // override an explicit user decision to disable it.
+  MOZ_ASSERT(IsFeatureStatusFailure(aStatus));
+
+  SetRuntime(aStatus, aMessage);
+}
+
+bool
+FeatureState::MaybeSetFailed(bool aEnable, FeatureStatus aStatus, const char* aMessage)
+{
+  if (!aEnable) {
+    SetFailed(aStatus, aMessage);
+    return false;
+  }
+  return true;
+}
+
+bool
+FeatureState::MaybeSetFailed(FeatureStatus aStatus, const char* aMessage)
+{
+  return MaybeSetFailed(IsFeatureStatusSuccess(aStatus), aStatus, aMessage);
+}
+
+bool
 FeatureState::DisabledByDefault() const
 {
+  AssertInitialized();
   return mDefault.mStatus != FeatureStatus::Available;
 }
 
 bool
 FeatureState::IsForcedOnByUser() const
 {
+  AssertInitialized();
   return mUser.mStatus == FeatureStatus::ForceEnabled;
 }
 
 void
 FeatureState::EnableByDefault()
 {
   // User/runtime decisions should not have been made yet.
   MOZ_ASSERT(mUser.mStatus == FeatureStatus::Unused);
--- a/gfx/config/gfxFeature.h
+++ b/gfx/config/gfxFeature.h
@@ -22,21 +22,41 @@ enum class Feature : uint32_t {
 #define MAKE_ENUM(name, type, desc) name,
   GFX_FEATURE_MAP(MAKE_ENUM)
 #undef MAKE_ENUM
   NumValues
 };
 
 class FeatureState
 {
+  friend class gfxConfig;
+
  public:
+  bool IsEnabled() const;
   FeatureStatus GetValue() const;
 
   void EnableByDefault();
   void DisableByDefault(FeatureStatus aStatus, const char* aMessage);
+  bool SetDefault(bool aEnable, FeatureStatus aDisableStatus, const char* aDisableMessage);
+
+  bool InitOrUpdate(bool aEnable,
+                    FeatureStatus aDisableStatus,
+                    const char* aMessage);
+  void UserEnable(const char* aMessage);
+  void UserForceEnable(const char* aMessage);
+  void UserDisable(const char* aMessage);
+  void Disable(FeatureStatus aStatus, const char* aMessage);
+  void ForceDisable(FeatureStatus aStatus, const char* aMessage) {
+    SetFailed(aStatus, aMessage);
+  }
+  void SetFailed(FeatureStatus aStatus, const char* aMessage);
+  bool MaybeSetFailed(bool aEnable, FeatureStatus aStatus, const char* aMessage);
+  bool MaybeSetFailed(FeatureStatus aStatus, const char* aMessage);
+
+ private:
   void SetUser(FeatureStatus aStatus, const char* aMessage);
   void SetEnvironment(FeatureStatus aStatus, const char* aMessage);
   void SetRuntime(FeatureStatus aStatus, const char* aMessage);
   bool IsForcedOnByUser() const;
   bool DisabledByDefault() const;
   bool IsInitialized() const {
     return mDefault.mStatus != FeatureStatus::Unused;
   }
--- a/gfx/src/gfxTelemetry.cpp
+++ b/gfx/src/gfxTelemetry.cpp
@@ -41,10 +41,17 @@ FeatureStatusToString(FeatureStatus aSta
 bool
 IsFeatureStatusFailure(FeatureStatus aStatus)
 {
   return !(aStatus == FeatureStatus::Unused ||
            aStatus == FeatureStatus::Available ||
            aStatus == FeatureStatus::ForceEnabled);
 }
 
+bool
+IsFeatureStatusSuccess(FeatureStatus aStatus)
+{
+  return aStatus == FeatureStatus::Available ||
+         aStatus == FeatureStatus::ForceEnabled;
+}
+
 } // namespace gfx
 } // namespace mozilla
--- a/gfx/src/gfxTelemetry.h
+++ b/gfx/src/gfxTelemetry.h
@@ -45,13 +45,14 @@ enum class FeatureStatus
   ForceEnabled,
 
   // This feature was disabled due to the startup crash guard.
   CrashedOnStartup
 };
 
 const char* FeatureStatusToString(FeatureStatus aStatus);
 bool IsFeatureStatusFailure(FeatureStatus aStatus);
+bool IsFeatureStatusSuccess(FeatureStatus aStatus);
 
 } // namespace gfx
 } // namespace mozilla
 
 #endif // gfx_src_gfxTelemetry_h__
--- a/gfx/thebes/gfxPlatform.cpp
+++ b/gfx/thebes/gfxPlatform.cpp
@@ -2132,44 +2132,42 @@ gfxPlatform::InitAcceleration()
   sLayersAccelerationPrefsInitialized = true;
 }
 
 void
 gfxPlatform::InitCompositorAccelerationPrefs()
 {
   const char *acceleratedEnv = PR_GetEnv("MOZ_ACCELERATED");
 
+  FeatureState& feature = gfxConfig::GetFeature(Feature::HW_COMPOSITING);
+
   // Base value - does the platform allow acceleration?
-  if (gfxConfig::SetDefault(Feature::HW_COMPOSITING,
-                            AccelerateLayersByDefault(),
-                            FeatureStatus::Blocked,
-                            "Acceleration blocked by platform"))
+  if (feature.SetDefault(AccelerateLayersByDefault(),
+                         FeatureStatus::Blocked,
+                         "Acceleration blocked by platform"))
   {
     if (gfxPrefs::LayersAccelerationDisabledDoNotUseDirectly()) {
-      gfxConfig::UserDisable(Feature::HW_COMPOSITING, "Disabled by pref");
+      feature.UserDisable("Disabled by pref");
     } else if (acceleratedEnv && *acceleratedEnv == '0') {
-      gfxConfig::UserDisable(Feature::HW_COMPOSITING, "Disabled by envvar");
+      feature.UserDisable("Disabled by envvar");
     }
   } else {
     if (acceleratedEnv && *acceleratedEnv == '1') {
-      gfxConfig::UserEnable(Feature::HW_COMPOSITING, "Enabled by envvar");
+      feature.UserEnable("Enabled by envvar");
     }
   }
 
   // This has specific meaning elsewhere, so we always record it.
   if (gfxPrefs::LayersAccelerationForceEnabledDoNotUseDirectly()) {
-    gfxConfig::UserForceEnable(Feature::HW_COMPOSITING, "Force-enabled by pref");
+    feature.UserForceEnable("Force-enabled by pref");
   }
 
   // Safe mode trumps everything.
   if (InSafeMode()) {
-    gfxConfig::ForceDisable(
-      Feature::HW_COMPOSITING,
-      FeatureStatus::Blocked,
-      "Acceleration blocked by safe-mode");
+    feature.ForceDisable(FeatureStatus::Blocked, "Acceleration blocked by safe-mode");
   }
 }
 
 bool
 gfxPlatform::CanUseDirect3D9()
 {
   // this function is called from the compositor thread, so it is not
   // safe to init the prefs etc. from here.