Fix a memory error when a timer task deleles its

original timer in the receiver method. This happens
in the events of following sequence:
  - A TimerTask is created on message loop
  - When TimerTask::Run is called, it nullifies
    timer_->delayed_task.
  - The receiver method is dispatched, and inside
    the  method, the timer_ is deleted. Since 
    timer_->delayed_task being null, the timer_
    destructor will not orphan the task.
  - After the method is returned, message loop
    deletes the task which will deref the
    dangling pointer to timer_.
    
I also tried to add a unit test to this. The best
I can come up with is making the test process
crash/fail in full page heap or purify environment.

BUG=1570948

Review URL: http://codereview.chromium.org/20111

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@9368 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 45 insertions, 0 deletions

diff --git a/base/timer.h b/base/timer.h
index 9aa084b..698d59d 100644
--- a/base/timer.h
+++ b/base/timer.h
@@ -168,6 +168,10 @@ class BaseTimer : public BaseTimer_Helper {
         // that the Timer has already taken care of properly setting the task.
         if (self->delayed_task_ == this)
           self->delayed_task_ = NULL;
+        // By now the delayed_task_ in the Timer does not point to us anymore.
+        // We should reset our own timer_ because the Timer can not do this
+        // for us in its destructor. 
+        timer_ = NULL;
       }
     }
 
diff --git a/base/timer_unittest.cc b/base/timer_unittest.cc
index 1fb44a3..8c1d58e 100644
--- a/base/timer_unittest.cc
+++ b/base/timer_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/message_loop.h"
+#include "base/scoped_ptr.h"
 #include "base/task.h"
 #include "base/timer.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -28,6 +29,26 @@ class OneShotTimerTester {
   base::OneShotTimer<OneShotTimerTester> timer_;
 };
 
+class OneShotSelfDeletingTimerTester {
+ public:
+  OneShotSelfDeletingTimerTester(bool* did_run) :
+      did_run_(did_run),
+      timer_(new base::OneShotTimer<OneShotSelfDeletingTimerTester>()) {
+  }
+  void Start() {
+    timer_->Start(TimeDelta::FromMilliseconds(10), this,
+                  &OneShotSelfDeletingTimerTester::Run);
+  }
+ private:
+  void Run() {
+    *did_run_ = true;
+    timer_.reset();
+    MessageLoop::current()->Quit();
+  }
+  bool* did_run_;
+  scoped_ptr<base::OneShotTimer<OneShotSelfDeletingTimerTester> > timer_;
+};
+
 class RepeatingTimerTester {
  public:
   RepeatingTimerTester(bool* did_run) : did_run_(did_run), counter_(10) {
@@ -82,6 +103,18 @@ void RunTest_OneShotTimer_Cancel(MessageLoop::Type message_loop_type) {
   EXPECT_TRUE(did_run_b);
 }
 
+void RunTest_OneShotSelfDeletingTimer(MessageLoop::Type message_loop_type) {
+  MessageLoop loop(message_loop_type);
+
+  bool did_run = false;
+  OneShotSelfDeletingTimerTester f(&did_run);
+  f.Start();
+
+  MessageLoop::current()->Run();
+
+  EXPECT_TRUE(did_run);
+}
+
 void RunTest_RepeatingTimer(MessageLoop::Type message_loop_type) {
   MessageLoop loop(message_loop_type);
 
@@ -134,6 +167,14 @@ TEST(TimerTest, OneShotTimer_Cancel) {
   RunTest_OneShotTimer_Cancel(MessageLoop::TYPE_IO);
 }
 
+// If underline timer does not handle properly, we will crash or fail
+// in full page heap or purify environment.
+TEST(TimerTest, OneShotSelfDeletingTimer) {
+  RunTest_OneShotSelfDeletingTimer(MessageLoop::TYPE_DEFAULT);
+  RunTest_OneShotSelfDeletingTimer(MessageLoop::TYPE_UI);
+  RunTest_OneShotSelfDeletingTimer(MessageLoop::TYPE_IO);
+}
+
 TEST(TimerTest, RepeatingTimer) {
   RunTest_RepeatingTimer(MessageLoop::TYPE_DEFAULT);
   RunTest_RepeatingTimer(MessageLoop::TYPE_UI);