Bug 1521370 - Do not attempt to retry crash guard once a crash occurred. r=mattwoodrow
authorJean-Yves Avenard <jyavenard@mozilla.com>
Wed, 13 Mar 2019 02:26:55 +0000
changeset 521659 6ee4b21eb0e0
parent 521658 a6724ca6cb91
child 521660 15e52cb0872d
push id10867
push userdvarga@mozilla.com
push dateThu, 14 Mar 2019 15:20:45 +0000
treeherdermozilla-beta@abad13547875 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow
bugs1521370
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1521370 - Do not attempt to retry crash guard once a crash occurred. r=mattwoodrow Should the configuration had changed at startup, a crash guard would have always been re-attempted even if a new crash occurred. Differential Revision: https://phabricator.services.mozilla.com/D22623
gfx/src/DriverCrashGuard.cpp
gfx/src/DriverCrashGuard.h
--- a/gfx/src/DriverCrashGuard.cpp
+++ b/gfx/src/DriverCrashGuard.cpp
@@ -215,17 +215,16 @@ void DriverCrashGuard::ActivateGuard() {
     if (!mGuardFile || !NS_SUCCEEDED(mGuardFile->OpenANSIFileDesc("w", &fp))) {
       return;
     }
     fclose(fp);
   }
 }
 
 void DriverCrashGuard::NotifyCrashed() {
-  CheckOrRefreshEnvironment();
   SetStatus(DriverInitStatus::Crashed);
   FlushPreferences();
   LogCrashRecovery();
 }
 
 bool DriverCrashGuard::RecoverFromCrash() {
   MOZ_ASSERT(XRE_IsParentProcess());
 
@@ -245,28 +244,34 @@ bool DriverCrashGuard::RecoverFromCrash(
 }
 
 // Return true if the caller should proceed to guard for crashes. False if
 // the environment has not changed. We persist the "changed" status across
 // calls, so that after an environment changes, all guards for the new
 // session are activated rather than just the first.
 bool DriverCrashGuard::CheckOrRefreshEnvironment() {
   // Our result can be cached statically since we don't check live prefs.
-  static bool sBaseInfoChanged = false;
-  static bool sBaseInfoChecked = false;
+  // We need to cache once per crash guard type.
+  // The first call to CheckOrRefrechEnvironment will always return true should
+  // the configuration had changed, following calls will return false.
+  static bool sBaseInfoChanged[NUM_CRASH_GUARD_TYPES];
+  static bool sBaseInfoChecked[NUM_CRASH_GUARD_TYPES];
 
-  if (!sBaseInfoChecked) {
+  const uint32_t type = uint32_t(mType);
+  if (!sBaseInfoChecked[type]) {
     // None of the prefs we care about, so we cache the result statically.
-    sBaseInfoChecked = true;
-    sBaseInfoChanged = UpdateBaseEnvironment();
+    sBaseInfoChecked[type] = true;
+    sBaseInfoChanged[type] = UpdateBaseEnvironment();
   }
 
   // Always update the full environment, even if the base info didn't change.
-  return UpdateEnvironment() || sBaseInfoChanged ||
-         GetStatus() == DriverInitStatus::Unknown;
+  bool result = UpdateEnvironment() || sBaseInfoChanged[type] ||
+                GetStatus() == DriverInitStatus::Unknown;
+  sBaseInfoChanged[type] = false;
+  return result;
 }
 
 bool DriverCrashGuard::UpdateBaseEnvironment() {
   bool changed = false;
   if (mGfxInfo) {
     nsString value;
 
     // Driver properties.
@@ -385,41 +390,40 @@ void D3D11LayersCrashGuard::Initialize()
   // If no telemetry states have been recorded, this will set the state to okay.
   // Otherwise, it will have no effect.
   RecordTelemetry(TelemetryState::Okay);
 }
 
 bool D3D11LayersCrashGuard::UpdateEnvironment() {
   // Our result can be cached statically since we don't check live prefs.
   static bool checked = false;
-  static bool changed = false;
 
   if (checked) {
-    return changed;
+    // We no longer need to bypass the crash guard.
+    return false;
   }
 
   checked = true;
 
+  bool changed = false;
   // Feature status.
 #if defined(XP_WIN)
   bool d2dEnabled = gfxPrefs::Direct2DForceEnabled() ||
                     (!gfxPrefs::Direct2DDisabled() &&
                      FeatureEnabled(nsIGfxInfo::FEATURE_DIRECT2D));
   changed |= CheckAndUpdateBoolPref("feature-d2d", d2dEnabled);
 
   bool d3d11Enabled = gfxConfig::IsEnabled(Feature::D3D11_COMPOSITING);
   changed |= CheckAndUpdateBoolPref("feature-d3d11", d3d11Enabled);
+  if (changed) {
+    RecordTelemetry(TelemetryState::EnvironmentChanged);
+  }
 #endif
 
-  if (!changed) {
-    return false;
-  }
-
-  RecordTelemetry(TelemetryState::EnvironmentChanged);
-  return true;
+  return changed;
 }
 
 void D3D11LayersCrashGuard::LogCrashRecovery() {
   RecordTelemetry(TelemetryState::RecoveredFromCrash);
   gfxCriticalNote << "D3D11 layers just crashed; D3D11 will be disabled.";
 }
 
 void D3D11LayersCrashGuard::LogFeatureDisabled() {
@@ -443,38 +447,28 @@ void D3D11LayersCrashGuard::RecordTeleme
   Telemetry::Accumulate(Telemetry::GRAPHICS_DRIVER_STARTUP_TEST,
                         int32_t(aState));
   sTelemetryStateRecorded = true;
 }
 
 D3D9VideoCrashGuard::D3D9VideoCrashGuard(dom::ContentParent* aContentParent)
     : DriverCrashGuard(CrashGuardType::D3D9Video, aContentParent) {}
 
-bool D3D9VideoCrashGuard::UpdateEnvironment() {
-  // We don't care about any extra preferences here.
-  return false;
-}
-
 void D3D9VideoCrashGuard::LogCrashRecovery() {
   gfxCriticalNote << "DXVA2D3D9 just crashed; hardware video will be disabled.";
 }
 
 void D3D9VideoCrashGuard::LogFeatureDisabled() {
   gfxCriticalNote
       << "DXVA2D3D9 video decoding is disabled due to a previous crash.";
 }
 
 D3D11VideoCrashGuard::D3D11VideoCrashGuard(dom::ContentParent* aContentParent)
     : DriverCrashGuard(CrashGuardType::D3D11Video, aContentParent) {}
 
-bool D3D11VideoCrashGuard::UpdateEnvironment() {
-  // We don't care about any extra preferences here.
-  return false;
-}
-
 void D3D11VideoCrashGuard::LogCrashRecovery() {
   gfxCriticalNote
       << "DXVA2D3D11 just crashed; hardware video will be disabled.";
 }
 
 void D3D11VideoCrashGuard::LogFeatureDisabled() {
   gfxCriticalNote
       << "DXVA2D3D11 video decoding is disabled due to a previous crash.";
@@ -498,24 +492,26 @@ void GLContextCrashGuard::Initialize() {
   return;
 #endif
 
   DriverCrashGuard::Initialize();
 }
 
 bool GLContextCrashGuard::UpdateEnvironment() {
   static bool checked = false;
-  static bool changed = false;
 
   if (checked) {
-    return changed;
+    // We no longer need to bypass the crash guard.
+    return false;
   }
 
   checked = true;
 
+  bool changed = false;
+
 #if defined(XP_WIN)
   changed |= CheckAndUpdateBoolPref("gfx.driver-init.webgl-angle-force-d3d11",
                                     gfxPrefs::WebGLANGLEForceD3D11());
   changed |= CheckAndUpdateBoolPref("gfx.driver-init.webgl-angle-try-d3d11",
                                     gfxPrefs::WebGLANGLETryD3D11());
   changed |= CheckAndUpdateBoolPref("gfx.driver-init.webgl-angle-force-warp",
                                     gfxPrefs::WebGLANGLEForceWARP());
   changed |= CheckAndUpdateBoolPref(
@@ -535,21 +531,16 @@ void GLContextCrashGuard::LogCrashRecove
 
 void GLContextCrashGuard::LogFeatureDisabled() {
   gfxCriticalNote << "GLContext remains enabled despite a previous crash.";
 }
 
 WMFVPXVideoCrashGuard::WMFVPXVideoCrashGuard(dom::ContentParent* aContentParent)
     : DriverCrashGuard(CrashGuardType::WMFVPXVideo, aContentParent) {}
 
-bool WMFVPXVideoCrashGuard::UpdateEnvironment() {
-  // We don't care about any extra preferences here.
-  return false;
-}
-
 void WMFVPXVideoCrashGuard::LogCrashRecovery() {
   gfxCriticalNote
       << "WMF VPX decoder just crashed; hardware video will be disabled.";
 }
 
 void WMFVPXVideoCrashGuard::LogFeatureDisabled() {
   gfxCriticalNote
       << "WMF VPX video decoding is disabled due to a previous crash.";
--- a/gfx/src/DriverCrashGuard.h
+++ b/gfx/src/DriverCrashGuard.h
@@ -83,17 +83,25 @@ class DriverCrashGuard {
   };
 
   typedef std::function<void(const char* aName, const char* aPrefName)>
       CrashGuardCallback;
   static void ForEachActiveCrashGuard(const CrashGuardCallback& aCallback);
 
  protected:
   virtual void Initialize();
-  virtual bool UpdateEnvironment() = 0;
+  // UpdateEnvironment needs to return true should we need to attempt the
+  // operation once again.
+  // It should return true once only so that in case of a crash, we won't
+  // needlessly attempt the operation over and over again leading to continual
+  // crashes. several times
+  virtual bool UpdateEnvironment() {
+    // We don't care about any extra preferences here.
+    return false;
+  }
   virtual void LogCrashRecovery() = 0;
   virtual void LogFeatureDisabled() = 0;
 
   // Helper functions.
   bool FeatureEnabled(int aFeature, bool aDefault = true);
   bool CheckAndUpdatePref(const char* aPrefName,
                           const nsAString& aCurrentValue);
   bool CheckAndUpdateBoolPref(const char* aPrefName, bool aCurrentValue);
@@ -140,27 +148,25 @@ class D3D11LayersCrashGuard final : publ
   void RecordTelemetry(TelemetryState aState);
 };
 
 class D3D9VideoCrashGuard final : public DriverCrashGuard {
  public:
   explicit D3D9VideoCrashGuard(dom::ContentParent* aContentParent = nullptr);
 
  protected:
-  bool UpdateEnvironment() override;
   void LogCrashRecovery() override;
   void LogFeatureDisabled() override;
 };
 
 class D3D11VideoCrashGuard final : public DriverCrashGuard {
  public:
   explicit D3D11VideoCrashGuard(dom::ContentParent* aContentParent = nullptr);
 
  protected:
-  bool UpdateEnvironment() override;
   void LogCrashRecovery() override;
   void LogFeatureDisabled() override;
 };
 
 class GLContextCrashGuard final : public DriverCrashGuard {
  public:
   explicit GLContextCrashGuard(dom::ContentParent* aContentParent = nullptr);
   void Initialize() override;
@@ -171,17 +177,16 @@ class GLContextCrashGuard final : public
   void LogFeatureDisabled() override;
 };
 
 class WMFVPXVideoCrashGuard final : public DriverCrashGuard {
  public:
   explicit WMFVPXVideoCrashGuard(dom::ContentParent* aContentParent = nullptr);
 
  protected:
-  bool UpdateEnvironment() override;
   void LogCrashRecovery() override;
   void LogFeatureDisabled() override;
 };
 
 }  // namespace gfx
 }  // namespace mozilla
 
 #endif  // gfx_src_DriverCrashGuard_h__