Redo fix from yesterday....

Redo fix from yesterday.

Post Mortem on why yesterday's fix was bogus:

The motivation for this change was because a separate CL had a OneShotTimer<> as
part of a Singleton class.  The Singleton is cleaned up in the AtExit manager,
which currently runs after the MessageLoop is destroyed.  The following
sequence left a dangling pointer:

   1) Start a OneShotTimer
          - Creates a task and registers on the MessageLoop
   2) Destroy MessageLoop   (this deletes all Pending Tasks)
   3) Destroy your OneShotTimer
          - Tries to reference a dangling pointer for the already-deleted task.

The fix was to modify the Task such that at destruction it would zero-out the
wrapper's pointer to the Task.  Now step 3 would not reference a bogus pointer.

Unfortunately, the fix is broken.  The timers work by having a wrapper Timer
class (BaseTimer_Helper) and a Task.  The Task has a pointer back to its wrapper
and the wrapper has a pointer to the task.   For the case where a timer is
re-started from within its Run() loop, things fail:

    1) Start a RepeatingTimer
    2) Timer fires via the MessageLoop
            MessageLoop calls Run()
    3) Run() restarts the timer, which creates a new Task but reuses the
       same BaseTimer_Helper.
             BaseTimer_Helper's timer pointer now points to the NEW timer
    4) The MessageLoop destroys the Task
             The new code zeroes out the task pointer in the
             BaseTimer_Helper, breaking the newly restarted timer.



This fix is the same as yesterday's fix, except:
  1) More comments
  2) Added extra check at line 169.  Don't reset the Task pointer when 
     destroying the Task if the Timer does not point to the Task being
     destroyed.

I'm confident, but not 100% positive that this fixes the problem.  I did
reproduce *something* using PageHeap which now seems to be gone, but I
can't quite confirm.




Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=6251

[Nicolas Sylvain, 2008-12-02 22:54:41]

LGTM.

Should we be worried about threading/timing issue?

[jar (doing other things), 2008-12-02 23:14:43]

LGTM