Make nsRefreshDriver reference-counted instead of being a sub-object of the pres context. (Bug 531585) r=bzbarsky

layout/base/nsPresContext.cpp

@@ -261,6 +261,12 @@ nsPresContext::~nsPresContext()
 
   delete mTransitionManager;
 
+  // Disconnect the refresh driver *after* the transition manager, which
+  // needs it.
+  if (mRefreshDriver) {
+    mRefreshDriver->Disconnect();
+  }
+
   if (mEventManager) {
     // unclear if these are needed, but can't hurt
     mEventManager->NotifyDestroyPresContext(this);
@@ -873,6 +879,12 @@ nsPresContext::Init(nsIDeviceContext* aDeviceContext)
   NS_ADDREF(mEventManager);
 
   mTransitionManager = new nsTransitionManager(this);
+  if (!mTransitionManager)
+    return NS_ERROR_OUT_OF_MEMORY;
+
+  mRefreshDriver = new nsRefreshDriver(this);
+  if (!mRefreshDriver)
+    return NS_ERROR_OUT_OF_MEMORY;
 
   mLangService = do_GetService(NS_LANGUAGEATOMSERVICE_CONTRACTID);
 

layout/base/nsPresContext.h

@@ -72,7 +72,6 @@
 #include "nsContentUtils.h"
 #include "nsIWidget.h"
 #include "mozilla/TimeStamp.h"
-#include "nsRefreshDriver.h"
 
 class nsImageLoader;
 #ifdef IBMBIDI
@@ -103,6 +102,7 @@ class nsUserFontSet;
 struct nsFontFaceRuleContainer;
 class nsObjectFrame;
 class nsTransitionManager;
+class nsRefreshDriver;
 class imgIContainer;
 
 #ifdef MOZ_REFLOW_PERF
@@ -233,13 +233,7 @@ public:
 
   nsTransitionManager* TransitionManager() { return mTransitionManager; }
 
-  nsRefreshDriver* RefreshDriver() { return &mRefreshDriver; }
-
-  static nsPresContext* FromRefreshDriver(nsRefreshDriver* aRefreshDriver) {
-    return reinterpret_cast<nsPresContext*>(
-             reinterpret_cast<char*>(aRefreshDriver) -
-             offsetof(nsPresContext, mRefreshDriver));
-  }
+  nsRefreshDriver* RefreshDriver() { return mRefreshDriver; }
 #endif
 
   /**
@@ -957,7 +951,7 @@ protected:
                                         // from gfx back to layout.
   nsIEventStateManager* mEventManager;  // [STRONG]
   nsILookAndFeel*       mLookAndFeel;   // [STRONG]
-  nsRefreshDriver       mRefreshDriver;
+  nsRefPtr<nsRefreshDriver> mRefreshDriver;
   nsTransitionManager*  mTransitionManager; // owns; it aggregates our refcount
   nsIAtom*              mMedium;        // initialized by subclass ctors;
                                         // weak pointer to static atom

layout/base/nsRefreshDriver.cpp

@@ -56,7 +56,8 @@
 
 using mozilla::TimeStamp;
 
-nsRefreshDriver::nsRefreshDriver()
+nsRefreshDriver::nsRefreshDriver(nsPresContext *aPresContext)
+  : mPresContext(aPresContext)
 {
 }
 
@@ -170,11 +171,7 @@ nsRefreshDriver::ArrayFor(mozFlushType aFlushType)
  * nsISupports implementation
  */
 
-NS_IMPL_ADDREF_USING_AGGREGATOR(nsRefreshDriver,
-                                nsPresContext::FromRefreshDriver(this))
-NS_IMPL_RELEASE_USING_AGGREGATOR(nsRefreshDriver,
-                                 nsPresContext::FromRefreshDriver(this))
-NS_IMPL_QUERY_INTERFACE1(nsRefreshDriver, nsITimerCallback)
+NS_IMPL_ISUPPORTS1(nsRefreshDriver, nsITimerCallback)
 
 /*
  * nsITimerCallback implementation
@@ -185,8 +182,12 @@ nsRefreshDriver::Notify(nsITimer *aTimer)
 {
   UpdateMostRecentRefresh();
 
-  nsPresContext *presContext = nsPresContext::FromRefreshDriver(this);
-  nsCOMPtr<nsIPresShell> presShell = presContext->GetPresShell();
+  if (!mPresContext) {
+    // Things are being destroyed.
+    NS_ABORT_IF_FALSE(!mTimer, "timer should have been stopped");
+    return NS_OK;
+  }
+  nsCOMPtr<nsIPresShell> presShell = mPresContext->GetPresShell();
   if (!presShell) {
     // Things are being destroyed.
     StopTimer();

layout/base/nsRefreshDriver.h

@@ -49,6 +49,8 @@
 #include "nsCOMPtr.h"
 #include "nsTObserverArray.h"
 
+class nsPresContext;
+
 /**
  * An abstract base class to be implemented by callers wanting to be
  * notified at refresh times.  When nothing needs to be painted, callers
@@ -59,16 +61,17 @@ public:
   virtual void WillRefresh(mozilla::TimeStamp aTime) = 0;
 };
 
-/*
- * nsRefreshDriver MUST ONLY be constructed as a sub-object of
- * nsPresContext (since its reference counting methods forward to the
- * pres context of which it is an mRefreshDriver)
- */
-class nsRefreshDriver : private nsITimerCallback {
+class nsRefreshDriver : public nsITimerCallback {
 public:
-  nsRefreshDriver();
+  nsRefreshDriver(nsPresContext *aPresContext);
   ~nsRefreshDriver();
 
+  // nsISupports implementation
+  NS_DECL_ISUPPORTS
+
+  // nsITimerCallback implementation
+  NS_DECL_NSITIMERCALLBACK
+
   /**
    * Return the time of the most recent refresh.  This is intended to be
    * used by callers who want to start an animation now and want to know
@@ -95,13 +98,18 @@ public:
                             mozFlushType aFlushType);
   PRBool RemoveRefreshObserver(nsARefreshObserver *aObserver,
                                mozFlushType aFlushType);
+
+  /**
+   * Tell the refresh driver that it is done driving refreshes and
+   * should stop its timer and forget about its pres context.  This may
+   * be called from within a refresh.
+   */
+  void Disconnect() {
+    StopTimer();
+    mPresContext = nsnull;
+  }
+
 private:
-  // nsISupports implementation
-  NS_DECL_ISUPPORTS_INHERITED
-
-  // nsITimerCallback implementation
-  NS_IMETHOD Notify(nsITimer *aTimer);
-
   typedef nsTObserverArray<nsARefreshObserver*> ObserverArray;
 
   void EnsureTimerStarted();
@@ -113,6 +121,9 @@ private:
   nsCOMPtr<nsITimer> mTimer;
   mozilla::TimeStamp mMostRecentRefresh; // only valid when mTimer non-null
 
+  nsPresContext *mPresContext; // weak; pres context passed in constructor
+                               // and unset in Disconnect
+
   // separate arrays for each flush type we support
   ObserverArray mObservers[3];
 };