Backout revisions fa5326c011b8, 8b22441911b0, and cfa10b01b1f6 (bug 531585) on suspicion of causing random orange bug 536382.
authorL. David Baron <dbaron@dbaron.org>
Tue, 22 Dec 2009 15:46:23 -0500
changeset 36566 f32e7f33b01530d2dba7594c3899a1922da92f3c
parent 36523 cfa10b01b1f657b7ac20ccaef5a666ffc334881c
child 36567 313bf48d30acfa747dd86bc614ee3b75b68dd432
push idunknown
push userunknown
push dateunknown
bugs531585, 536382
milestone1.9.3a1pre
backs outfa5326c011b8d22f132d59d191b8f3316be5dda9
8b22441911b036039ea3927cb2859ac4d67ae8b5
cfa10b01b1f657b7ac20ccaef5a666ffc334881c
Backout revisions fa5326c011b8, 8b22441911b0, and cfa10b01b1f6 (bug 531585) on suspicion of causing random orange bug 536382.
layout/base/nsPresContext.cpp
layout/base/nsPresContext.h
layout/base/nsRefreshDriver.cpp
layout/base/nsRefreshDriver.h
layout/style/nsTransitionManager.cpp
layout/style/nsTransitionManager.h
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@@ -254,23 +254,17 @@ nsPresContext::nsPresContext(nsIDocument
 nsPresContext::~nsPresContext()
 {
   for (PRUint32 i = 0; i < IMAGE_LOAD_TYPE_COUNT; ++i)
     mImageLoaders[i].Enumerate(destroy_loads, nsnull);
 
   NS_PRECONDITION(!mShell, "Presshell forgot to clear our mShell pointer");
   SetShell(nsnull);
 
-  if (mRefreshDriver) {
-    mRefreshDriver->Disconnect();
-  }
-
-  if (mTransitionManager) {
-    mTransitionManager->Disconnect();
-  }
+  delete mTransitionManager;
 
   if (mEventManager) {
     // unclear if these are needed, but can't hurt
     mEventManager->NotifyDestroyPresContext(this);
     mEventManager->SetPresContext(nsnull);
 
     NS_RELEASE(mEventManager);
   }
@@ -874,22 +868,16 @@ nsPresContext::Init(nsIDeviceContext* aD
 
   mEventManager = new nsEventStateManager();
   if (!mEventManager)
     return NS_ERROR_OUT_OF_MEMORY;
 
   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);
 
   // Register callbacks so we're notified when the preferences change
   nsContentUtils::RegisterPrefCallback("font.",
                                        nsPresContext::PrefChangedCallback,
                                        this);
   nsContentUtils::RegisterPrefCallback("browser.display.",
--- a/layout/base/nsPresContext.h
+++ b/layout/base/nsPresContext.h
@@ -67,16 +67,17 @@
 #include "gfxRect.h"
 #include "nsRegion.h"
 #include "nsTArray.h"
 #include "nsAutoPtr.h"
 #include "nsThreadUtils.h"
 #include "nsContentUtils.h"
 #include "nsIWidget.h"
 #include "mozilla/TimeStamp.h"
+#include "nsRefreshDriver.h"
 
 class nsImageLoader;
 #ifdef IBMBIDI
 class nsBidiPresUtils;
 #endif // IBMBIDI
 
 struct nsRect;
 
@@ -97,17 +98,16 @@ class nsIAtom;
 struct nsStyleBackground;
 struct nsStyleBorder;
 class nsIRunnable;
 class gfxUserFontSet;
 class nsUserFontSet;
 struct nsFontFaceRuleContainer;
 class nsObjectFrame;
 class nsTransitionManager;
-class nsRefreshDriver;
 class imgIContainer;
 
 #ifdef MOZ_REFLOW_PERF
 class nsIRenderingContext;
 #endif
 
 enum nsWidgetType {
   eWidgetType_Button  	= 1,
@@ -228,17 +228,23 @@ public:
 #ifdef _IMPL_NS_LAYOUT
   nsStyleSet* StyleSet() { return GetPresShell()->StyleSet(); }
 
   nsFrameManager* FrameManager()
     { return GetPresShell()->FrameManager(); } 
 
   nsTransitionManager* TransitionManager() { return mTransitionManager; }
 
-  nsRefreshDriver* RefreshDriver() { return mRefreshDriver; }
+  nsRefreshDriver* RefreshDriver() { return &mRefreshDriver; }
+
+  static nsPresContext* FromRefreshDriver(nsRefreshDriver* aRefreshDriver) {
+    return reinterpret_cast<nsPresContext*>(
+             reinterpret_cast<char*>(aRefreshDriver) -
+             offsetof(nsPresContext, mRefreshDriver));
+  }
 #endif
 
   /**
    * Rebuilds all style data by throwing out the old rule tree and
    * building a new one, and additionally applying aExtraHint (which
    * must not contain nsChangeHint_ReconstructFrame) to the root frame.
    * Also rebuild the user font set.
    */
@@ -946,18 +952,18 @@ protected:
   nsCOMPtr<nsIDocument> mDocument;
   nsIDeviceContext*     mDeviceContext; // [STRONG] could be weak, but
                                         // better safe than sorry.
                                         // Cannot reintroduce cycles
                                         // since there is no dependency
                                         // from gfx back to layout.
   nsIEventStateManager* mEventManager;  // [STRONG]
   nsILookAndFeel*       mLookAndFeel;   // [STRONG]
-  nsRefPtr<nsRefreshDriver> mRefreshDriver;
-  nsRefPtr<nsTransitionManager> mTransitionManager;
+  nsRefreshDriver       mRefreshDriver;
+  nsTransitionManager*  mTransitionManager; // owns; it aggregates our refcount
   nsIAtom*              mMedium;        // initialized by subclass ctors;
                                         // weak pointer to static atom
 
   nsILinkHandler*       mLinkHandler;   // [WEAK]
   nsIAtom*              mLangGroup;     // [STRONG]
 
   nsRefPtrHashtable<nsVoidPtrHashKey, nsImageLoader>
                         mImageLoaders[IMAGE_LOAD_TYPE_COUNT];
--- a/layout/base/nsRefreshDriver.cpp
+++ b/layout/base/nsRefreshDriver.cpp
@@ -39,31 +39,29 @@
  * Code to notify things that animate before a refresh, at an appropriate
  * refresh rate.  (Perhaps temporary, until replaced by compositor.)
  */
 
 #include "nsRefreshDriver.h"
 #include "nsPresContext.h"
 #include "nsComponentManagerUtils.h"
 #include "prlog.h"
-#include "nsAutoPtr.h"
 
 /*
  * TODO:
  * Once this is hooked in to suppressing updates when the presentation
  * is not visible, we need to hook it up to FlushPendingNotifications so
  * that we flush when necessary.
  */
 
 #define REFRESH_INTERVAL_MILLISECONDS 20
 
 using mozilla::TimeStamp;
 
-nsRefreshDriver::nsRefreshDriver(nsPresContext *aPresContext)
-  : mPresContext(aPresContext)
+nsRefreshDriver::nsRefreshDriver()
 {
 }
 
 nsRefreshDriver::~nsRefreshDriver()
 {
   NS_ABORT_IF_FALSE(ObserverCount() == 0,
                     "observers should have unregistered");
   NS_ABORT_IF_FALSE(!mTimer, "timer should be gone");
@@ -167,55 +165,43 @@ nsRefreshDriver::ArrayFor(mozFlushType a
       return *static_cast<ObserverArray*>(nsnull);
   }
 }
 
 /*
  * nsISupports implementation
  */
 
-NS_IMPL_ISUPPORTS1(nsRefreshDriver, nsITimerCallback)
+NS_IMPL_ADDREF_USING_AGGREGATOR(nsRefreshDriver,
+                                nsPresContext::FromRefreshDriver(this))
+NS_IMPL_RELEASE_USING_AGGREGATOR(nsRefreshDriver,
+                                 nsPresContext::FromRefreshDriver(this))
+NS_IMPL_QUERY_INTERFACE1(nsRefreshDriver, nsITimerCallback)
 
 /*
  * nsITimerCallback implementation
  */
 
 NS_IMETHODIMP
 nsRefreshDriver::Notify(nsITimer *aTimer)
 {
   UpdateMostRecentRefresh();
 
-  if (!mPresContext) {
-    // Things are being destroyed.
-    NS_ABORT_IF_FALSE(!mTimer, "timer should have been stopped");
-    return NS_OK;
-  }
-  nsCOMPtr<nsIPresShell> presShell = mPresContext->GetPresShell();
+  nsPresContext *presContext = nsPresContext::FromRefreshDriver(this);
+  nsCOMPtr<nsIPresShell> presShell = presContext->GetPresShell();
   if (!presShell) {
     // Things are being destroyed.
     StopTimer();
     return NS_OK;
   }
 
-  /*
-   * The timer holds a reference to |this| while calling |Notify|.
-   * However, implementations of |WillRefresh| are permitted to destroy
-   * the pres context, which will cause our |mPresContext| to become
-   * null.  If this happens, we must stop notifying observers.
-   */
   for (PRUint32 i = 0; i < NS_ARRAY_LENGTH(mObservers); ++i) {
     ObserverArray::EndLimitedIterator etor(mObservers[i]);
     while (etor.HasMore()) {
-      nsRefPtr<nsARefreshObserver> obs = etor.GetNext();
-      obs->WillRefresh(mMostRecentRefresh);
-      
-      if (!mPresContext || !mPresContext->GetPresShell()) {
-        StopTimer();
-        return NS_OK;
-      }
+      etor.GetNext()->WillRefresh(mMostRecentRefresh);
     }
     if (i == 0) {
       // This is the Flush_Style case.
       // FIXME: Maybe we should only flush if the WillRefresh calls did
       // something?  It's probably ok as-is, though, especially as we
       // hook up more things here (or to the replacement of this class).
       // FIXME: We should probably flush for other sets of observers
       // too.  But we should only flush layout once nsRefreshDriver is
--- a/layout/base/nsRefreshDriver.h
+++ b/layout/base/nsRefreshDriver.h
@@ -44,48 +44,36 @@
 #define nsRefreshDriver_h_
 
 #include "mozilla/TimeStamp.h"
 #include "mozFlushType.h"
 #include "nsITimer.h"
 #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
  * may not be notified.
  */
 class nsARefreshObserver {
 public:
-  // AddRef and Release signatures that match nsISupports.  Implementors
-  // must implement reference counting, and those that do implement
-  // nsISupports will already have methods with the correct signature.
-  //
-  // The refresh driver does NOT hold references to refresh observers
-  // except while it is notifying them.  
-  NS_IMETHOD_(nsrefcnt) AddRef(void) = 0;
-  NS_IMETHOD_(nsrefcnt) Release(void) = 0;
-
   virtual void WillRefresh(mozilla::TimeStamp aTime) = 0;
 };
 
-class nsRefreshDriver : public nsITimerCallback {
+/*
+ * 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 {
 public:
-  nsRefreshDriver(nsPresContext *aPresContext);
+  nsRefreshDriver();
   ~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
    * what time to consider the start of the animation.  (This helps
    * ensure that multiple animations started during the same event off
    * the main event loop have the same start time.)
    */
   mozilla::TimeStamp MostRecentRefresh() const;
@@ -97,47 +85,36 @@ public:
    * The flush type affects:
    *   + the order in which the observers are notified (lowest flush
    *     type to highest, in order registered)
    *   + (in the future) which observers are suppressed when the display
    *     doesn't require current position data or isn't currently
    *     painting, and, correspondingly, which get notified when there
    *     is a flush during such suppression
    * and it must be either Flush_Style, Flush_Layout, or Flush_Display.
-   *
-   * The refresh driver does NOT own a reference to these observers;
-   * they must remove themselves before they are destroyed.
    */
   PRBool AddRefreshObserver(nsARefreshObserver *aObserver,
                             mozFlushType aFlushType);
   PRBool RemoveRefreshObserver(nsARefreshObserver *aObserver,
                                mozFlushType aFlushType);
+private:
+  // nsISupports implementation
+  NS_DECL_ISUPPORTS_INHERITED
 
-  /**
-   * 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;
-  }
+  // nsITimerCallback implementation
+  NS_IMETHOD Notify(nsITimer *aTimer);
 
-private:
   typedef nsTObserverArray<nsARefreshObserver*> ObserverArray;
 
   void EnsureTimerStarted();
   void StopTimer();
   PRUint32 ObserverCount() const;
   void UpdateMostRecentRefresh();
   ObserverArray& ArrayFor(mozFlushType aFlushType);
 
   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];
 };
 
 #endif /* !defined(nsRefreshDriver_h_) */
--- a/layout/style/nsTransitionManager.cpp
+++ b/layout/style/nsTransitionManager.cpp
@@ -742,17 +742,19 @@ nsTransitionManager::AddElementTransitio
 
   PR_INSERT_BEFORE(aElementTransitions, &mElementTransitions);
 }
 
 /*
  * nsISupports implementation
  */
 
-NS_IMPL_ISUPPORTS1(nsTransitionManager, nsIStyleRuleProcessor)
+NS_IMPL_ADDREF_USING_AGGREGATOR(nsTransitionManager, mPresContext)
+NS_IMPL_RELEASE_USING_AGGREGATOR(nsTransitionManager, mPresContext)
+NS_IMPL_QUERY_INTERFACE1(nsTransitionManager, nsIStyleRuleProcessor)
 
 /*
  * nsIStyleRuleProcessor implementation
  */
 
 nsresult
 nsTransitionManager::WalkTransitionRule(RuleProcessorData* aData,
                                         nsCSSPseudoElements::Type aPseudoType)
@@ -844,20 +846,16 @@ nsTransitionManager::MediumFeaturesChang
 {
   *aRulesChanged = PR_FALSE;
   return NS_OK;
 }
 
 /* virtual */ void
 nsTransitionManager::WillRefresh(mozilla::TimeStamp aTime)
 {
-  NS_ABORT_IF_FALSE(mPresContext,
-                    "refresh driver should not notify additional observers "
-                    "after pres context has been destroyed");
-
   // Trim transitions that have completed, and post restyle events for
   // frames that are still transitioning.
   {
     PRCList *next = PR_LIST_HEAD(&mElementTransitions);
     while (next != &mElementTransitions) {
       ElementTransitions *et = static_cast<ElementTransitions*>(next);
       next = PR_NEXT_LINK(next);
 
--- a/layout/style/nsTransitionManager.h
+++ b/layout/style/nsTransitionManager.h
@@ -47,30 +47,27 @@
 #include "nsCSSPseudoElements.h"
 
 class nsStyleContext;
 class nsPresContext;
 class nsCSSPropertySet;
 struct nsTransition;
 struct ElementTransitions;
 
+/**
+ * Must be created only as a sub-object of an nsPresContext (since its
+ * reference counting methods assume that).
+ */
 class nsTransitionManager : public nsIStyleRuleProcessor,
                             public nsARefreshObserver {
 public:
   nsTransitionManager(nsPresContext *aPresContext);
   ~nsTransitionManager();
 
   /**
-   * Notify the transition manager that the pres context is going away.
-   */
-  void Disconnect() {
-    mPresContext = nsnull;
-  }
-
-  /**
    * StyleContextChanged 
    *
    * To be called from nsFrameManager::ReResolveStyleContext when the
    * style of an element has changed, to initiate transitions from that
    * style change.
    *
    * It may return a "cover rule" (see CoverTransitionStartStyleRule) to
    * cover up some of the changes for the duration of the restyling of
@@ -80,17 +77,17 @@ public:
    * returned cover rule as the most specific rule.
    */
   already_AddRefed<nsIStyleRule>
     StyleContextChanged(nsIContent *aElement,
                         nsStyleContext *aOldStyleContext,
                         nsStyleContext *aNewStyleContext);
 
   // nsISupports
-  NS_DECL_ISUPPORTS
+  NS_DECL_ISUPPORTS_INHERITED
 
   // nsIStyleRuleProcessor
   NS_IMETHOD RulesMatching(ElementRuleProcessorData* aData);
   NS_IMETHOD RulesMatching(PseudoElementRuleProcessorData* aData);
   NS_IMETHOD RulesMatching(AnonBoxRuleProcessorData* aData);
 #ifdef MOZ_XUL
   NS_IMETHOD RulesMatching(XULTreeRuleProcessorData* aData);
 #endif
@@ -118,12 +115,12 @@ private:
                                             nsCSSPseudoElements::Type aPseudoType,
                                             PRBool aCreateIfNeeded);
   void AddElementTransitions(ElementTransitions* aElementTransitions);
   void TransitionsRemoved();
   nsresult WalkTransitionRule(RuleProcessorData* aData,
 			      nsCSSPseudoElements::Type aPseudoType);
 
   PRCList mElementTransitions;
-  nsPresContext *mPresContext; // weak (non-null from ctor to Disconnect)
+  nsPresContext *mPresContext;
 };
 
 #endif /* !defined(nsTransitionManager_h_) */