Bug 1550023 - Don't allow the qualified pref to disable WR for already released populations. r=jrmuizel a=RyanVM
authorKartikaya Gupta <kgupta@mozilla.com>
Wed, 10 Jul 2019 11:27:02 +0000
changeset 544507 ea96619eb18345b62d6280e61f877bc358af1c91
parent 544506 c69f2d22e51ef9131843e2806ff20aa16a0de2f8
child 544508 676c09f04f1182b40a829bcaaefe09e9f7040f44
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel, RyanVM
bugs1550023
milestone69.0
Bug 1550023 - Don't allow the qualified pref to disable WR for already released populations. r=jrmuizel a=RyanVM
gfx/thebes/gfxPlatform.cpp
--- a/gfx/thebes/gfxPlatform.cpp
+++ b/gfx/thebes/gfxPlatform.cpp
@@ -2613,32 +2613,39 @@ static bool CalculateWrQualifiedPrefValu
 
 static void HardwareTooOldForWR(FeatureState& aFeature) {
   aFeature.Disable(
       FeatureStatus::BlockedDeviceTooOld, "Device too old",
       NS_LITERAL_CSTRING("FEATURE_FAILURE_DEVICE_TOO_OLD"));
 }
 
 static void UpdateWRQualificationForNvidia(FeatureState& aFeature,
-    int32_t aDeviceId) {
+                                           int32_t aDeviceId,
+                                           bool* aOutGuardedByQualifiedPref) {
   // 0x6c0 is the lowest Fermi device id. Unfortunately some Tesla
   // devices that don't support D3D 10.1 have higher deviceIDs. They
   // will be included, but blocked by ANGLE.
   bool supported = aDeviceId >= 0x6c0;
 
   if (!supported) {
     HardwareTooOldForWR(aFeature);
     return;
   }
 
-  // Any additional Nvidia checks go here
+  // Any additional Nvidia checks go here. Make sure to leave
+  // aOutGuardedByQualifiedPref as true unless the hardware is qualified
+  // for users on the release channel.
+
+  // Nvidia devices with device id >= 0x6c0 got WR in release Firefox 67.
+  *aOutGuardedByQualifiedPref = false;
 }
 
 static void UpdateWRQualificationForAMD(FeatureState& aFeature,
-    int32_t aDeviceId) {
+                                        int32_t aDeviceId,
+                                        bool* aOutGuardedByQualifiedPref) {
   // AMD deviceIDs are not very well ordered. This
   // condition is based off the information in gpu-db
   bool supported =
       (aDeviceId >= 0x6600 && aDeviceId < 0x66b0) ||
       (aDeviceId >= 0x6700 && aDeviceId < 0x6720) ||
       (aDeviceId >= 0x6780 && aDeviceId < 0x6840) ||
       (aDeviceId >= 0x6860 && aDeviceId < 0x6880) ||
       (aDeviceId >= 0x6900 && aDeviceId < 0x6a00) ||
@@ -2646,29 +2653,37 @@ static void UpdateWRQualificationForAMD(
       (aDeviceId >= 0x9830 && aDeviceId < 0x9870) ||
       (aDeviceId >= 0x9900 && aDeviceId < 0x9a00);
 
   if (!supported) {
     HardwareTooOldForWR(aFeature);
     return;
   }
 
-  // we have a desktop CAYMAN, SI, CIK, VI, or GFX9 device
-  // so treat the device as qualified unless it is not Windows
-  // and not nightly.
-#if !defined(XP_WIN) && !defined(NIGHTLY_BUILD)
-  aFeature.Disable(
-      FeatureStatus::BlockedReleaseChannelAMD,
-      "Release channel and AMD",
-      NS_LITERAL_CSTRING("FEATURE_FAILURE_RELEASE_CHANNEL_AMD"));
-#endif  // !XPWIN && !NIGHTLY_BUILD
+  // we have a desktop CAYMAN, SI, CIK, VI, or GFX9 device.
+
+#if defined(XP_WIN)
+  // These devices got WR in release Firefox 68.
+  *aOutGuardedByQualifiedPref = false;
+#elif defined(NIGHTLY_BUILD)
+  // Qualify on Linux Nightly, but leave *aOutGuardedByQualifiedPref as true
+  // to indicate users on release don't have it yet, and it's still guarded
+  // by the qualified pref.
+#else
+  // Disqualify everywhere else
+  aFeature.Disable(FeatureStatus::BlockedReleaseChannelAMD,
+                   "Release channel and AMD",
+                   NS_LITERAL_CSTRING("FEATURE_FAILURE_RELEASE_CHANNEL_AMD"));
+#endif
 }
 
 static void UpdateWRQualificationForIntel(FeatureState& aFeature,
-    int32_t aDeviceId, int32_t aScreenPixels) {
+                                          int32_t aDeviceId,
+                                          int32_t aScreenPixels,
+                                          bool* aOutGuardedByQualifiedPref) {
   const uint16_t supportedDevices[] = {
       // skylake gt2+
       0x1912,
       0x1913,
       0x1915,
       0x1916,
       0x1917,
       0x191a,
@@ -2788,30 +2803,34 @@ static void UpdateWRQualificationForInte
         NS_LITERAL_CSTRING("FEATURE_FAILURE_SCREEN_SIZE_UNKNOWN"));
     return;
   }
 #endif
 
 #if (defined(XP_WIN) || (defined(MOZ_WIDGET_GTK) && defined(NIGHTLY_BUILD)))
   // Qualify Intel graphics cards on Windows to release and on Linux nightly
   // (subject to device whitelist and screen size checks above).
+  // Leave *aOutGuardedByQualifiedPref as true to indicate no existing
+  // release users have this yet, and it's still guarded by the qualified pref.
 #else
   // Disqualify everywhere else
   aFeature.Disable(
       FeatureStatus::BlockedReleaseChannelIntel,
       "Release channel and Intel",
       NS_LITERAL_CSTRING("FEATURE_FAILURE_RELEASE_CHANNEL_INTEL"));
 #endif
 }
 
 static FeatureState& WebRenderHardwareQualificationStatus(
-    const IntSize& aScreenSize, bool aHasBattery) {
+    const IntSize& aScreenSize, bool aHasBattery,
+    bool* aOutGuardedByQualifiedPref) {
   FeatureState& featureWebRenderQualified =
       gfxConfig::GetFeature(Feature::WEBRENDER_QUALIFIED);
   featureWebRenderQualified.EnableByDefault();
+  MOZ_ASSERT(aOutGuardedByQualifiedPref && *aOutGuardedByQualifiedPref);
 
   if (Preferences::HasUserValue(WR_ROLLOUT_HW_QUALIFIED_OVERRIDE)) {
     if (!Preferences::GetBool(WR_ROLLOUT_HW_QUALIFIED_OVERRIDE)) {
       featureWebRenderQualified.Disable(
           FeatureStatus::BlockedOverride, "HW qualification pref override",
           NS_LITERAL_CSTRING("FEATURE_FAILURE_WR_QUALIFICATION_OVERRIDE"));
     }
     return featureWebRenderQualified;
@@ -2845,37 +2864,46 @@ static FeatureState& WebRenderHardwareQu
     featureWebRenderQualified.Disable(
         FeatureStatus::BlockedDeviceUnknown, "Bad device id",
         NS_LITERAL_CSTRING("FEATURE_FAILURE_BAD_DEVICE_ID"));
     return featureWebRenderQualified;
   }
 
   const int32_t screenPixels = aScreenSize.width * aScreenSize.height;
 
-  if (adapterVendorID == u"0x10de") { // Nvidia
-    UpdateWRQualificationForNvidia(featureWebRenderQualified, deviceID);
+  if (adapterVendorID == u"0x10de") {  // Nvidia
+    UpdateWRQualificationForNvidia(featureWebRenderQualified, deviceID,
+                                   aOutGuardedByQualifiedPref);
   } else if (adapterVendorID == u"0x1002") {  // AMD
-    UpdateWRQualificationForAMD(featureWebRenderQualified, deviceID);
+    UpdateWRQualificationForAMD(featureWebRenderQualified, deviceID,
+                                aOutGuardedByQualifiedPref);
   } else if (adapterVendorID == u"0x8086") {  // Intel
     UpdateWRQualificationForIntel(featureWebRenderQualified, deviceID,
-                                  screenPixels);
+                                  screenPixels, aOutGuardedByQualifiedPref);
   } else {
     featureWebRenderQualified.Disable(
         FeatureStatus::BlockedVendorUnsupported, "Unsupported vendor",
         NS_LITERAL_CSTRING("FEATURE_FAILURE_UNSUPPORTED_VENDOR"));
   }
 
   if (!featureWebRenderQualified.IsEnabled()) {
-    // One of the checks above failed, early exit
+    // One of the checks above failed, early exit. If this happens then
+    // this population must still be guarded by the qualified pref.
+    MOZ_ASSERT(*aOutGuardedByQualifiedPref);
     return featureWebRenderQualified;
   }
 
   // We leave checking the battery for last because we would like to know
   // which users were denied WebRender only because they have a battery.
   if (aHasBattery) {
+    // We never released WR to the battery populations, so let's keep the pref
+    // guard for these populations. That way we can do a gradual rollout to
+    // the battery population using the pref.
+    *aOutGuardedByQualifiedPref = true;
+
     // For AMD/Intel devices, if we have a battery, ignore it if the
     // screen is small enough. Note that we always check for a battery
     // with NVIDIA because we do not have a limited/curated set of devices
     // to support WebRender on.
     const int32_t kMaxPixelsBattery = 1920 * 1200;  // WUXGA
     if ((adapterVendorID == u"0x8086" || adapterVendorID == u"0x1002") &&
         screenPixels > 0 && screenPixels <= kMaxPixelsBattery) {
 #ifndef NIGHTLY_BUILD
@@ -2917,36 +2945,45 @@ void gfxPlatform::InitWebRenderConfig() 
     // later in this function. For other processes we still want to report
     // the state of the feature for crash reports.
     if (UseWebRender()) {
       reporter.SetSuccessful();
     }
     return;
   }
 
+  bool guardedByQualifiedPref = true;
   FeatureState& featureWebRenderQualified =
-      WebRenderHardwareQualificationStatus(GetScreenSize(), HasBattery());
+      WebRenderHardwareQualificationStatus(GetScreenSize(), HasBattery(),
+                                           &guardedByQualifiedPref);
   FeatureState& featureWebRender = gfxConfig::GetFeature(Feature::WEBRENDER);
 
   featureWebRender.DisableByDefault(
       FeatureStatus::OptIn, "WebRender is an opt-in feature",
       NS_LITERAL_CSTRING("FEATURE_FAILURE_DEFAULT_OFF"));
 
   const bool wrQualifiedAll = CalculateWrQualifiedPrefValue();
 
   // envvar works everywhere; note that we need this for testing in CI.
   // Prior to bug 1523788, the `prefEnabled` check was only done on Nightly,
   // so as to prevent random users from easily enabling WebRender on
   // unqualified hardware in beta/release.
   if (envvarEnabled) {
     featureWebRender.UserEnable("Force enabled by envvar");
   } else if (prefEnabled) {
     featureWebRender.UserEnable("Force enabled by pref");
-  } else if (wrQualifiedAll && featureWebRenderQualified.IsEnabled()) {
-    featureWebRender.UserEnable("Qualified enabled by pref ");
+  } else if (featureWebRenderQualified.IsEnabled()) {
+    // If the HW is qualified, we enable if either the HW has been qualified
+    // on the release channel (i.e. it's no longer guarded by the qualified
+    // pref), or if the qualified pref is enabled.
+    if (!guardedByQualifiedPref) {
+      featureWebRender.UserEnable("Qualified in release");
+    } else if (wrQualifiedAll) {
+      featureWebRender.UserEnable("Qualified enabled by pref");
+    }
   }
 
   // If the user set the pref to force-disable, let's do that. This will
   // override all the other enabling prefs (gfx.webrender.enabled,
   // gfx.webrender.all, and gfx.webrender.all.qualified).
   if (StaticPrefs::gfx_webrender_force_disabled() ||
       (WebRenderEnvvarDisabled() && !InMarionetteRolloutTest())) {
     featureWebRender.UserDisable(