Bug 635475 - nsIdleServiceDaily contructor can cause pointer corruption.

r=sdwilsh a=bsmedberg

widget/src/xpwidgets/nsIdleService.cpp

@@ -99,9 +99,7 @@ nsIdleServiceDaily::Observe(nsISupports *,
   }
 
   // Stop observing idle for today.
-  if (NS_SUCCEEDED(mIdleService->RemoveIdleObserver(this, MAX_IDLE_POLL_INTERVAL))) {
-    mObservesIdle = false;
-  }
+  (void)mIdleService->RemoveIdleObserver(this, MAX_IDLE_POLL_INTERVAL);
 
   // Set the last idle-daily time pref.
   nsCOMPtr<nsIPrefBranch> pref = do_GetService(NS_PREFSERVICE_CONTRACTID);
@@ -117,11 +115,15 @@ nsIdleServiceDaily::Observe(nsISupports *,
   return NS_OK;
 }
 
-nsIdleServiceDaily::nsIdleServiceDaily(nsIdleService* aIdleService)
+nsIdleServiceDaily::nsIdleServiceDaily(nsIIdleService* aIdleService)
   : mIdleService(aIdleService)
-  , mObservesIdle(false)
   , mTimer(do_CreateInstance(NS_TIMER_CONTRACTID))
   , mCategoryObservers(OBSERVER_TOPIC_IDLE_DAILY)
+{
+}
+
+void
+nsIdleServiceDaily::Init()
 {
   // Check time of the last idle-daily notification.  If it was more than 24
   // hours ago listen for idle, otherwise set a timer for 24 hours from now.
@@ -148,19 +150,12 @@ nsIdleServiceDaily::nsIdleServiceDaily(nsIdleService* aIdleService)
   }
 }
 
-void
-nsIdleServiceDaily::Shutdown()
+nsIdleServiceDaily::~nsIdleServiceDaily()
 {
   if (mTimer) {
     mTimer->Cancel();
     mTimer = nsnull;
   }
-  if (mIdleService && mObservesIdle) {
-    if (NS_SUCCEEDED(mIdleService->RemoveIdleObserver(this, MAX_IDLE_POLL_INTERVAL))) {
-      mObservesIdle = false;
-    }
-    mIdleService = nsnull;
-  }
 }
 
 // static
@@ -171,9 +166,7 @@ nsIdleServiceDaily::DailyCallback(nsITimer* aTimer, void* aClosure)
 
   // The one thing we do every day is to start waiting for the user to "have
   // a significant idle time".
-  if (NS_SUCCEEDED(me->mIdleService->AddIdleObserver(me, MAX_IDLE_POLL_INTERVAL))) {
-    me->mObservesIdle = true;
-  }
+  (void)me->mIdleService->AddIdleObserver(me, MAX_IDLE_POLL_INTERVAL);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -184,12 +177,12 @@ nsIdleService::nsIdleService() : mLastIdleReset(0)
                                , mPolledIdleTimeIsValid(false)
 {
   mDailyIdle = new nsIdleServiceDaily(this);
+  mDailyIdle->Init();
 }
 
 nsIdleService::~nsIdleService()
 {
   StopTimer();
-  mDailyIdle->Shutdown();
 }
 
 NS_IMETHODIMP

widget/src/xpwidgets/nsIdleService.h

@@ -77,20 +77,24 @@ public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIOBSERVER
 
-  nsIdleServiceDaily(nsIdleService* aIdleService);
+  nsIdleServiceDaily(nsIIdleService* aIdleService);
 
   /**
-   * This function will make this class release its allocated resources (its
-   * idle timer and/or its normal timer).
+   * Initializes the daily idle observer.
+   * Keep this separated from the constructor, since it could cause pointer
+   * corruption due to AddRef/Release of "this".
    */
-  void Shutdown();
+  void Init();
+
+  virtual ~nsIdleServiceDaily();
 
 private:
   /**
-   * @note This is a normal pointer, or the idle service could keep it self
-   * alive.
+   * @note This is a normal pointer, part to avoid creating a cycle with the
+   * idle service, part to avoid potential pointer corruption due to this class
+   * being instantiated in the constructor of the service itself.
    */
-  nsIdleService* mIdleService;
+  nsIIdleService* mIdleService;
 
   /**
    * Set to true when the instantiated object has a idle observer.
@@ -192,7 +196,7 @@ private:
   /**
    * Object keeping track of the daily idle thingy.
    */
-  nsCOMPtr<nsIdleServiceDaily> mDailyIdle;
+  nsRefPtr<nsIdleServiceDaily> mDailyIdle;
 
   /**
    * Contains the time of the last idle reset or 0 if there haven't been a