Calculate animation_floor_time_ only when actually updating animations

chrome/renderer/render_widget.cc

Index: chrome/renderer/render_widget.cc
diff --git a/chrome/renderer/render_widget.cc b/chrome/renderer/render_widget.cc
index 5f66e49987ccf8a3709caac736a76c0726d45cad..dda2058c33231655f8a3a110c7a8d66ff2a9192e 100644
--- a/chrome/renderer/render_widget.cc
+++ b/chrome/renderer/render_widget.cc
@@ -77,7 +77,8 @@ RenderWidget::RenderWidget(RenderThreadBase* render_thread,
       pending_window_rect_count_(0),
       suppress_next_char_events_(false),
       is_accelerated_compositing_active_(false),
-      animation_update_pending_(false) {
+      animation_update_pending_(false),
+      animation_waiting_for_paint_(false) {
   RenderProcess::current()->AddRefProcess();
   DCHECK(render_thread_);
 }
@@ -490,9 +491,14 @@ void RenderWidget::CallDoDeferredUpdate() {
 void RenderWidget::UpdateAnimationsIfNeeded() {
   if (!is_hidden() && animation_update_pending_) {
     base::Time now = base::Time::Now();
-    if (now >= animation_floor_time_) {
+    if (now >= animation_floor_time_ && !animation_waiting_for_paint_) {
        animation_update_pending_ = false;
+       animation_floor_time_ = base::Time::Now() +
+          base::TimeDelta::FromMilliseconds(16); // Target 60FPS.

[darin (slow to review), 2011/01/31 22:25:57]

nit: 4 space indent instead of 3
nit: 2 spaces before a comment at the end of a statement

        webwidget_->animate();
+       webwidget_->layout();
+       if (paint_aggregator_.HasPendingUpdate())
+           animation_waiting_for_paint_ = true;

[darin (slow to review), 2011/01/31 22:25:57]

nit: 2 space indent

     } else {
       // This code uses base::Time::Now() to calculate the floor and next fire
       // time because javascript's Date object uses base::Time::Now().  The
@@ -501,8 +507,10 @@ void RenderWidget::UpdateAnimationsIfNeeded() {
       // The upshot of all this is that this function might be called before
       // base::Time::Now() has advanced past the animation_floor_time_.  To
       // avoid exposing this delay to javascript, we keep posting delayed
-      // tasks until we observe base::Time::Now() advancing far enough.
-      int64 delay = (animation_floor_time_ - now).InMillisecondsRoundedUp();
+      // tasks until base::Time::Now() has advanced far enough.
+      int64 delay =
+          std::max(static_cast<int64>(0),
+                   (animation_floor_time_ - now).InMillisecondsRoundedUp());

[darin (slow to review), 2011/01/31 22:25:57]

if we are waiting for paint, then why bother scheduling another call to
UpdateAnimationsIfNeeded?

       MessageLoop::current()->PostDelayedTask(FROM_HERE, NewRunnableMethod(
           this, &RenderWidget::UpdateAnimationsIfNeeded), delay);
     }
@@ -521,7 +529,9 @@ void RenderWidget::DoDeferredUpdate() {
   }
 
   if (base::Time::Now() > animation_floor_time_)
-    UpdateAnimationsIfNeeded();
+      UpdateAnimationsIfNeeded();

[darin (slow to review), 2011/01/31 22:25:57]

nit: 2 space indent

[darin (slow to review), 2011/01/31 22:25:57]

maybe this should just call animate() directly?  you don't really need to call
layout() here since it will be called below.  you also don't need to kick the
animation timer since that'll happen when scheduleAnimation is called again,
right?

+
+  animation_waiting_for_paint_ = false;
 
   // Layout may generate more invalidation.  It may also enable the
   // GPU acceleration, so make sure to run layout before we send the
@@ -717,10 +727,8 @@ void RenderWidget::scheduleComposite() {
 void RenderWidget::scheduleAnimation() {
     if (!animation_update_pending_) {
       animation_update_pending_ = true;
-      animation_floor_time_ =
-          base::Time::Now() + base::TimeDelta::FromMilliseconds(10);
-      MessageLoop::current()->PostDelayedTask(FROM_HERE, NewRunnableMethod(
-          this, &RenderWidget::UpdateAnimationsIfNeeded), 10);
+      MessageLoop::current()->PostTask(FROM_HERE, NewRunnableMethod(
+          this, &RenderWidget::UpdateAnimationsIfNeeded));
     }
 }