Bug 528306 part 1. Don't stop the refresh driver timer on observer removal; instead just stop it if it fires when there are no observers. r=dbaron
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 31 Dec 2009 14:07:57 -0500
changeset 36795 88dd8acfbc0568291754f4c3a1d221f27118db4e
parent 36794 c410241f9d1c1271f2acf4650296b8f7a37a5371
child 36796 6ef8b28b0d9bb82bdb368303049bf8e9a5b75b7c
push id10985
push userbzbarsky@mozilla.com
push dateThu, 31 Dec 2009 19:20:31 +0000
treeherdermozilla-central@2e580c431f4e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs528306
milestone1.9.3a1pre
Bug 528306 part 1. Don't stop the refresh driver timer on observer removal; instead just stop it if it fires when there are no observers. r=dbaron
layout/base/nsRefreshDriver.cpp
--- a/layout/base/nsRefreshDriver.cpp
+++ b/layout/base/nsRefreshDriver.cpp
@@ -89,23 +89,17 @@ nsRefreshDriver::AddRefreshObserver(nsAR
   return success;
 }
 
 PRBool
 nsRefreshDriver::RemoveRefreshObserver(nsARefreshObserver *aObserver,
                                        mozFlushType aFlushType)
 {
   ObserverArray& array = ArrayFor(aFlushType);
-  PRBool success = array.RemoveElement(aObserver);
-
-  if (ObserverCount() == 0) {
-    StopTimer();
-  }
-
-  return success;
+  return array.RemoveElement(aObserver);
 }
 
 void
 nsRefreshDriver::EnsureTimerStarted()
 {
   if (mTimer) {
     // It's already been started.
     return;
@@ -184,18 +178,24 @@ 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();
-  if (!presShell) {
-    // Things are being destroyed.
+  if (!presShell || ObserverCount() == 0) {
+    // Things are being destroyed, or we no longer have any observers.
+    // We don't want to stop the timer when observers are initially
+    // removed, because sometimes observers can be added and removed
+    // often depending on what other things are going on and in that
+    // situation we don't want to thrash our timer.  So instead we
+    // wait until we get a Notify() call when we have no observers
+    // before stopping the timer.
     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
@@ -220,14 +220,10 @@ nsRefreshDriver::Notify(nsITimer *aTimer
       // FIXME: We should probably flush for other sets of observers
       // too.  But we should only flush layout once nsRefreshDriver is
       // the driver for the interruptible layout timer (and we should
       // then Flush_InterruptibleLayout).
       presShell->FlushPendingNotifications(Flush_Style);
     }
   }
 
-  if (ObserverCount() == 0) {
-    StopTimer();
-  }
-
   return NS_OK;
 }