Bug 681026 - glxtest should wait() for its child to exit - r=joe
authorBenoit Jacob <bjacob@mozilla.com>
Wed, 07 Sep 2011 17:17:44 -0400
changeset 76689 5e90688c720ef15f47ad17d967c3cf5804b85873
parent 76688 bdb0bff93ce85df745b57f9430b3cd6e506bc22f
child 76690 d333f4021aaf7d4096e5d5757ddf8a89ad8ef072
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewersjoe
bugs681026
milestone9.0a1
Bug 681026 - glxtest should wait() for its child to exit - 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
+  [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;
       }
     }
   }