Bug 681026 - always get data from glxtest process and waitpid() for it - r=joe
☠☠ backed out by b916b514a499 ☠ ☠
authorBenoit Jacob <bjacob@mozilla.com>
Wed, 07 Sep 2011 17:17:44 -0400
changeset 77127 432a30ebd148aaacd1615d4188afab44742c6c52
parent 77126 6dd930769dc95a5d368a6bab32178ce311e208a7
child 77128 b916b514a499416d0065991c50888c81845121d1
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewersjoe
bugs681026
milestone9.0a1
Bug 681026 - always get data from glxtest process and waitpid() for it - r=joe This patch makes GetShouldAccelerate directly call GetData, just before the place where we call GetFeatureStatus, which we know is reached. Initially I considered instead calling GetData from GfxInfo::Init() but that turned out to be a bad idea: Init() is called by the factory constructor, which is called significantly earlier in the startup process. We want to call GetData as late as possible, just when we need it, to maximize chances that the glxtest process be already finished by the time we waitpid() it, so that we don't end up wasting time waiting for it.
widget/public/nsIGfxInfo.idl
widget/src/xpwidgets/GfxInfoBase.h
widget/src/xpwidgets/GfxInfoX11.h
widget/src/xpwidgets/nsBaseWidget.cpp
--- a/widget/public/nsIGfxInfo.idl
+++ b/widget/public/nsIGfxInfo.idl
@@ -138,10 +138,13 @@ interface nsIGfxInfo : nsISupports
   DOMString getFeatureSuggestedDriverVersion(in long aFeature);
 
   /**
    * WebGL info; valid params are "full-renderer", "vendor", "renderer", "version",
    * "shading_language_version", "extensions".  These return info from
    * underlying GL impl that's used to implement WebGL.
    */
   DOMString getWebGLParameter(in DOMString aParam);
+
+  // only useful on X11
+  [noscript, notxpcom] void GetData();
 };
 
--- a/widget/src/xpwidgets/GfxInfoBase.h
+++ b/widget/src/xpwidgets/GfxInfoBase.h
@@ -79,16 +79,19 @@ public:
   // Initialization function. If you override this, you must call this class's
   // version of Init first.
   // We need Init to be called separately from the constructor so we can
   // register as an observer after all derived classes have been constructed
   // and we know we have a non-zero refcount.
   // Ideally, Init() would be void-return, but the rules of
   // NS_GENERIC_FACTORY_CONSTRUCTOR_INIT require it be nsresult return.
   virtual nsresult Init();
+  
+  // only useful on X11
+  virtual void GetData() { }
 
 protected:
 
   virtual nsresult GetFeatureStatusImpl(PRInt32 aFeature, PRInt32* aStatus,
                                         nsAString& aSuggestedDriverVersion,
                                         GfxDriverInfo* aDriverInfo = nsnull) = 0;
 
 private:
--- a/widget/src/xpwidgets/GfxInfoX11.h
+++ b/widget/src/xpwidgets/GfxInfoX11.h
@@ -71,30 +71,31 @@ public:
   NS_SCRIPTABLE NS_IMETHOD GetAdapterDriverVersion2(nsAString & aAdapterDriverVersion);
   NS_SCRIPTABLE NS_IMETHOD GetAdapterDriverDate2(nsAString & aAdapterDriverDate);
   NS_SCRIPTABLE NS_IMETHOD GetIsGPU2Active(PRBool *aIsGPU2Active);
   using GfxInfoBase::GetFeatureStatus;
   using GfxInfoBase::GetFeatureSuggestedDriverVersion;
   using GfxInfoBase::GetWebGLParameter;
 
   virtual nsresult Init();
+  
+  virtual void GetData();
 
 protected:
 
   virtual nsresult GetFeatureStatusImpl(PRInt32 aFeature, PRInt32 *aStatus, nsAString & aSuggestedDriverVersion, GfxDriverInfo* aDriverInfo = nsnull);
 
 private:
   nsCString mVendor;
   nsCString mRenderer;
   nsCString mVersion;
   nsCString mAdapterDescription;
   bool mIsMesa, mIsNVIDIA, mIsFGLRX;
   bool mHasTextureFromPixmap;
   int mMajorVersion, mMinorVersion, mRevisionVersion;
 
   void AddCrashReportAnnotations();
-  void GetData();
 };
 
 } // namespace widget
 } // namespace mozilla
 
 #endif /* __GfxInfoX11_h__ */
--- a/widget/src/xpwidgets/nsBaseWidget.cpp
+++ b/widget/src/xpwidgets/nsBaseWidget.cpp
@@ -822,20 +822,24 @@ nsBaseWidget::GetShouldAccelerate()
 
   nsCOMPtr<nsIXULRuntime> xr = do_GetService("@mozilla.org/xre/runtime;1");
   PRBool safeMode = PR_FALSE;
   if (xr)
     xr->GetInSafeMode(&safeMode);
 
   bool whitelisted = false;
 
-  // bug 655578: on X11 at least, we must always call GetFeatureStatus (even if we don't need that information)
-  // as that's what causes GfxInfo initialization which kills the zombie 'glxtest' process.
   nsCOMPtr<nsIGfxInfo> gfxInfo = do_GetService("@mozilla.org/gfx/info;1");
   if (gfxInfo) {
+    // bug 655578: on X11 at least, we must always call GetData (even if we don't need that information)
+    // as that's what causes GfxInfo initialization which kills the zombie 'glxtest' process.
+    // initially we relied on the fact that GetFeatureStatus calls GetData for us, but bug 681026 showed
+    // that assumption to be unsafe.
+    gfxInfo->GetData();
+
     PRInt32 status;
     if (NS_SUCCEEDED(gfxInfo->GetFeatureStatus(nsIGfxInfo::FEATURE_OPENGL_LAYERS, &status))) {
       if (status == nsIGfxInfo::FEATURE_NO_INFO) {
         whitelisted = true;
       }
     }
   }