Switch SharedTimerWin over to using PostDelayedTask. I made some tweaks to the

PostDelayedTask implementation to ensure that perf is still good.  This
involved recording the intended fire time of PostDelayedTask on the Task object
so that it can be used to properly determine the delay passed to the StartTimer
call.  With this change, I am able to service timers (call DoDelayedWork) more
often from within the MessagePump implementations.

R=mbelshe
BUG=1346553

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

9 files changed, 84 insertions, 89 deletions

diff --git a/base/message_loop.cc b/base/message_loop.cc
index cd44b92..410394f 100644
--- a/base/message_loop.cc
+++ b/base/message_loop.cc
@@ -66,7 +66,11 @@ MessageLoop::MessageLoop(Type type)
 
   // TODO(darin): Choose the pump based on the requested type.
 #if defined(OS_WIN)
-  pump_ = new base::MessagePumpWin();
+  if (type_ == TYPE_DEFAULT) {
+    pump_ = new base::MessagePumpDefault();
+  } else {
+    pump_ = new base::MessagePumpWin();
+  }
 #else
   pump_ = new base::MessagePumpDefault();
 #endif
@@ -180,10 +184,17 @@ void MessageLoop::Quit() {
 void MessageLoop::PostDelayedTask(const tracked_objects::Location& from_here,
                                   Task* task, int delay_ms) {
   task->SetBirthPlace(from_here);
-  DCHECK(delay_ms >= 0);
-  DCHECK(!task->is_owned_by_message_loop());
-  task->set_posted_task_delay(delay_ms);
-  DCHECK(task->is_owned_by_message_loop());
+
+  DCHECK(!task->owned_by_message_loop_);
+  task->owned_by_message_loop_ = true;
+
+  if (delay_ms > 0) {
+    task->delayed_run_time_ =
+        Time::Now() + TimeDelta::FromMilliseconds(delay_ms);
+  } else {
+    DCHECK(delay_ms == 0) << "delay should not be negative";
+  }
+
   PostTaskInternal(task);
 }
 
@@ -231,7 +242,7 @@ bool MessageLoop::RunTimerTask(Timer* timer) {
   HistogramEvent(kTimerEvent);
 
   Task* task = timer->task();
-  if (task->is_owned_by_message_loop()) {
+  if (task->owned_by_message_loop_) {
     // We constructed it through PostDelayedTask().
     DCHECK(!timer->repeating());
     timer->set_task(NULL);
@@ -249,7 +260,7 @@ bool MessageLoop::RunTimerTask(Timer* timer) {
 
 void MessageLoop::DiscardTimer(Timer* timer) {
   Task* task = timer->task();
-  if (task->is_owned_by_message_loop()) {
+  if (task->owned_by_message_loop_) {
     DCHECK(!timer->repeating());
     timer->set_task(NULL);
     delete timer;  // We constructed it through PostDelayedTask().
@@ -292,7 +303,7 @@ void MessageLoop::RunTask(Task* task) {
   HistogramEvent(kTaskRunEvent);
   // task may self-delete during Run() if we don't happen to own it.
   // ...so check *before* we Run, since we can't check after.
-  bool we_own_task = task->is_owned_by_message_loop();
+  bool we_own_task = task->owned_by_message_loop_;
   task->Run();
   if (we_own_task)
     task->RecycleOrDelete();  // Relinquish control, and probably delete.
@@ -331,12 +342,16 @@ void MessageLoop::ReloadWorkQueue() {
 
   while (!new_task_list.Empty()) {
     Task* task = new_task_list.Pop();
-    DCHECK(task->is_owned_by_message_loop());
-
-    if (task->posted_task_delay() > 0)
-      timer_manager_.StartTimer(task->posted_task_delay(), task, false);
-    else
+    DCHECK(task->owned_by_message_loop_);
+
+    // TODO(darin): We should probably postpone starting the timer until we
+    // process the work queue as starting the timer here breaks the FIFO
+    // ordering of tasks.
+    if (!task->delayed_run_time_.is_null()) {
+      timer_manager_.StartTimer(new Timer(task->delayed_run_time_, task));
+    } else {
       work_queue_.Push(task);
+    }
   }
 }
 
diff --git a/base/message_pump.h b/base/message_pump.h
index 4b53f4e..da4d214 100644
--- a/base/message_pump.h
+++ b/base/message_pump.h
@@ -54,21 +54,22 @@ class MessagePump : public RefCountedThreadSafe<MessagePump> {
   //     bool did_work = DoInternalWork();
   //     if (should_quit_)
   //       break;
+  //
   //     did_work |= delegate_->DoWork();
   //     if (should_quit_)
   //       break;
-  //     if (did_work)
-  //       continue;
   //
-  //     did_work = delegate_->DoDelayedWork();
+  //     did_work |= delegate_->DoDelayedWork();
   //     if (should_quit_)
   //       break;
+  //
   //     if (did_work)
   //       continue;
   //
   //     did_work = delegate_->DoIdleWork();
   //     if (should_quit_)
   //       break;
+  //
   //     if (did_work)
   //       continue;
   //
@@ -80,13 +81,10 @@ class MessagePump : public RefCountedThreadSafe<MessagePump> {
   // completion (for example).  WaitForWork is a private method that simply
   // blocks until there is more work of any type to do.
   //
-  // Notice that the run loop alternates between calling DoInternalWork and
-  // calling the delegate's DoWork method.  This helps ensure that neither work
-  // queue starves the other.  However, DoDelayedWork may be starved.  The
-  // implementation may decide to periodically let some DoDelayedWork calls go
-  // through if either DoInternalWork or DoWork is dominating the run loop.
-  // This can be important for message pumps that are used to drive animations,
-  // for example.
+  // Notice that the run loop cycles between calling DoInternalWork, DoWork,
+  // and DoDelayedWork methods.  This helps ensure that neither work queue
+  // starves the other.  This is important for message pumps that are used to
+  // drive animations, for example.
   //
   // Notice also that after each callout to foreign code, the run loop checks
   // to see if it should quit.  The Quit method is responsible for setting this
diff --git a/base/message_pump_default.cc b/base/message_pump_default.cc
index 0460b28..c88f465 100644
--- a/base/message_pump_default.cc
+++ b/base/message_pump_default.cc
@@ -20,26 +20,18 @@ void MessagePumpDefault::Run(Delegate* delegate) {
     bool did_work = delegate->DoWork();

     if (!keep_running_)

       break;

-    if (did_work)

-      continue;

-

-    // TODO(darin): Delayed work will be starved if DoWork continues to return

-    // true.  We should devise a better strategy.

-    //

-    // It is tempting to call DoWork followed by DoDelayedWork before checking

-    // did_work, but we need to make sure that any tasks that were dispatched

-    // prior to a timer actually run before the timer.  Getting that right may

-    // require some additional changes.

 

-    did_work = delegate->DoDelayedWork(&delayed_work_time_);

+    did_work |= delegate->DoDelayedWork(&delayed_work_time_);

     if (!keep_running_)

       break;

+

     if (did_work)

       continue;

 

     did_work = delegate->DoIdleWork();

     if (!keep_running_)

       break;

+

     if (did_work)

       continue;

 

diff --git a/base/message_pump_win.cc b/base/message_pump_win.cc
index 289e089..7b053c3 100644
--- a/base/message_pump_win.cc
+++ b/base/message_pump_win.cc
@@ -273,10 +273,7 @@ void MessagePumpWin::DoRunLoop() {
     if (state_->should_quit)
       break;
 
-    if (more_work_is_plausible)
-      continue;
-
-    more_work_is_plausible =
+    more_work_is_plausible |=
         state_->delegate->DoDelayedWork(&delayed_work_time_);
     // If we did not process any delayed work, then we can assume that our
     // existing WM_TIMER if any will fire when delayed work should run.  We
diff --git a/base/task.h b/base/task.h
index 42277d4..99bec4e 100644
--- a/base/task.h
+++ b/base/task.h
@@ -11,6 +11,7 @@
 #include "base/logging.h"
 #include "base/non_thread_safe.h"
 #include "base/revocable_store.h"
+#include "base/time.h"
 #include "base/tracked.h"
 #include "base/tuple.h"
 
@@ -24,6 +25,8 @@ class TimerManager;
 
 class Task;
 
+// TODO(darin): Eliminate TaskBase.  This data is ML specific and is not
+// applicable to other uses of Task.
 class TaskBase : public tracked_objects::Tracked {
  public:
   TaskBase() { Reset(); }
@@ -53,28 +56,23 @@ class TaskBase : public tracked_objects::Tracked {
   // derived classes should attempt this feat, as a replacement for creating a
   // new instance.
   void Reset() {
-    posted_task_delay_ = -1;
     priority_ = 0;
+    delayed_run_time_ = Time();
     next_task_ = NULL;
     nestable_ = true;
+    owned_by_message_loop_ = false;
   }
 
  private:
   friend class base::TimerManager;  // To check is_owned_by_message_loop().
-  friend class MessageLoop;   // To maintain posted_task_delay().
-
-  // Access methods used ONLY by friends in MessageLoop and TimerManager
-  int posted_task_delay() const { return posted_task_delay_; }
-  bool is_owned_by_message_loop() const { return 0 <= posted_task_delay_; }
-  void set_posted_task_delay(int delay) { posted_task_delay_ = delay; }
+  friend class MessageLoop;         // To maintain posted_task_delay().
 
   // Priority for execution by MessageLoop. 0 is default. Higher means run
   // sooner, and lower (including negative) means run less soon.
   int priority_;
 
-  // Slot to hold delay if the task was passed to PostTask().  If it was not
-  // passed to PostTask, then the delay is negative (the default).
-  int posted_task_delay_;
+  // The time when a delayed task should be run.
+  Time delayed_run_time_;
 
   // When tasks are collected into a queue by MessageLoop, this member is used
   // to form a null terminated list.
@@ -84,6 +82,9 @@ class TaskBase : public tracked_objects::Tracked {
   // only in the top level message loop.
   bool nestable_;
 
+  // True if this Task is owned by the message loop.
+  bool owned_by_message_loop_;
+
   DISALLOW_COPY_AND_ASSIGN(TaskBase);
 };
 
diff --git a/base/timer.cc b/base/timer.cc
index 09a13f6..5cf6002 100644
--- a/base/timer.cc
+++ b/base/timer.cc
@@ -32,6 +32,19 @@ Timer::Timer(int delay, Task* task, bool repeating)
   Reset();
 }
 
+Timer::Timer(Time fire_time, Task* task)
+    : task_(task),
+      fire_time_(fire_time),
+      repeating_(false) {
+  timer_id_ = timer_id_counter_.GetNext();
+
+  // TODO(darin): kill off this stuff
+  creation_time_ = Time::Now();
+  delay_ = static_cast<int>((fire_time_ - creation_time_).InMilliseconds());
+  DCHECK(delay_ >= 0);
+  DHISTOGRAM_COUNTS(L"Timer.Durations", delay_);
+}
+
 void Timer::Reset() {
   creation_time_ = Time::Now();
   fire_time_ = creation_time_ + TimeDelta::FromMilliseconds(delay_);
@@ -144,7 +157,7 @@ bool TimerManager::RunSomePendingTimers() {
     // now (i.e., current task needs to be reentrant).
     // TODO(jar): We may block tasks that we can queue from being popped.
     if (!message_loop_->NestableTasksAllowed() &&
-        !pending->task()->is_owned_by_message_loop())
+        !pending->task()->owned_by_message_loop_)
       break;
 
     timers_.pop();
diff --git a/base/timer.h b/base/timer.h
index add7844..d78423a 100644
--- a/base/timer.h
+++ b/base/timer.h
@@ -74,6 +74,9 @@ class Timer {
  public:
   Timer(int delay, Task* task, bool repeating);
 
+  // For one-shot timers, you can also specify the exact fire time.
+  Timer(Time fire_time, Task* task);
+
   // The task to be run when the timer fires.
   Task* task() const { return task_; }
   void set_task(Task* task) { task_ = task; }
diff --git a/webkit/port/platform/SharedTimerWin.cpp b/webkit/port/platform/SharedTimerWin.cpp
index 385bae8..b7f3b08 100644
--- a/webkit/port/platform/SharedTimerWin.cpp
+++ b/webkit/port/platform/SharedTimerWin.cpp
@@ -30,9 +30,7 @@
 
 #undef LOG
 
-#include "base/timer.h"
 #include "base/message_loop.h"
-#include <map>
 
 namespace WebCore {
 
@@ -40,7 +38,6 @@ class WebkitTimerTask;
 
 // We maintain a single active timer and a single active task for
 // setting timers directly on the platform.
-static Timer* msgLoopTimer;
 static WebkitTimerTask* msgLoopTask;
 static void (*sharedTimerFiredFunction)();
 
@@ -48,22 +45,25 @@ static void (*sharedTimerFiredFunction)();
 class WebkitTimerTask : public Task {
 public:
     WebkitTimerTask(void (*callback)()) : m_callback(callback) {}
+
     virtual void Run() {
-        MessageLoop* msgloop = MessageLoop::current();
-        delete msgLoopTimer;
-        msgLoopTimer = NULL;
+        if (!m_callback)
+            return;
         // Since we only have one task running at a time, verify that it
         // is 'this'.
         ASSERT(msgLoopTask == this);
         msgLoopTask = NULL;
         m_callback();
-        delete this;
+    }
+
+    void Cancel() {
+        m_callback = NULL;
     }
 
 private:
     void (*m_callback)();
 
-    DISALLOW_EVIL_CONSTRUCTORS(WebkitTimerTask);
+    DISALLOW_COPY_AND_ASSIGN(WebkitTimerTask);
 };
 
 void setSharedTimerFiredFunction(void (*f)())
@@ -80,43 +80,20 @@ void setSharedTimerFireTime(double fireTime)
 
     stopSharedTimer();
 
-    MessageLoop* msgloop = MessageLoop::current();
-    TimerManager* mgr = msgloop->timer_manager();
-
     // Verify that we didn't leak the task or timer objects.
-    ASSERT(msgLoopTimer == NULL);
     ASSERT(msgLoopTask == NULL);
 
-    // Create a new task and timer.  We are responsible for cleanup 
-    // of each.
     msgLoopTask = new WebkitTimerTask(sharedTimerFiredFunction);
-    msgLoopTimer = mgr->StartTimer(interval, msgLoopTask, false);
+    MessageLoop::current()->PostDelayedTask(FROM_HERE, msgLoopTask, interval);
 }
 
 void stopSharedTimer()
 {
-    if (msgLoopTimer != NULL) {
-        // When we have a timer running, there must be an associated task.
-        ASSERT(msgLoopTask != NULL);
-
-        MessageLoop* msgloop = MessageLoop::current();
-        // msgloop can be NULL in one particular instance:
-        // KJS uses static GCController object, which has a Timer member,
-        // which attempts to stop() when it's destroyed, which calls this.
-        // But since the object is static, and the current MessageLoop can be
-        // scoped to main(), the static object can be destroyed (and this
-        // code can run) after the current MessageLoop is gone.
-        // TODO(evanm): look into whether there's a better solution for this.
-        if (msgloop) {
-            TimerManager* mgr = msgloop->timer_manager();
-            mgr->StopTimer(msgLoopTimer);
-        }
-
-        delete msgLoopTimer;
-        msgLoopTimer = NULL;
-        delete msgLoopTask;
-        msgLoopTask = NULL;
-    }
-}
+    if (!msgLoopTask)
+        return;
 
+    msgLoopTask->Cancel();
+    msgLoopTask = NULL;
 }
+
+} // namespace WebCore
diff --git a/webkit/port/platform/SystemTimeWin.cpp b/webkit/port/platform/SystemTimeWin.cpp
index 8a4f140..fe3f6d6 100644
--- a/webkit/port/platform/SystemTimeWin.cpp
+++ b/webkit/port/platform/SystemTimeWin.cpp
@@ -27,7 +27,6 @@
 #include "SystemTime.h"
 
 #include "NotImplemented.h"
-#include <windows.h>
 #include "base/time.h"
 
 namespace WebCore {
@@ -35,7 +34,7 @@ namespace WebCore {
 // Get the current time in seconds since epoch.
 double currentTime()
 {
-  return Time::Now().ToDoubleT();
+    return Time::Now().ToDoubleT();
 }
 
 float userIdleTime()