Calculate animation_floor_time_ only when actually updating animations

chrome/renderer/render_widget.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/renderer/render_widget.h"
 
 #include "app/surface/transport_dib.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
@@ skipping 59 matching lines @@
       needs_repainting_on_restore_(false),
       has_focus_(false),
       handling_input_event_(false),
       closing_(false),
       input_method_is_active_(false),
       text_input_type_(WebKit::WebTextInputTypeNone),
       popup_type_(popup_type),
       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_);
 }
 
 RenderWidget::~RenderWidget() {
   DCHECK(!webwidget_) << "Leaking our WebWidget!";
   if (current_paint_buf_) {
     RenderProcess::current()->ReleaseTransportDIB(current_paint_buf_);
     current_paint_buf_ = NULL;
   }
@@ skipping 392 matching lines @@
 void RenderWidget::CallDoDeferredUpdate() {
   DoDeferredUpdate();
 
   if (pending_input_event_ack_.get())
     Send(pending_input_event_ack_.release());
 }
 
 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
       // message loop uses base::TimeTicks, which on windows can have a
       // different granularity than base::Time.
       // 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);
     }
   }
 }
 
 void RenderWidget::DoDeferredUpdate() {
   if (!webwidget_ || update_reply_pending())
     return;
 
   // Suppress updating when we are hidden.
   if (is_hidden_ || size_.IsEmpty()) {
     paint_aggregator_.ClearPendingUpdate();
     needs_repainting_on_restore_ = true;
     return;
   }
 
   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
   // GpuRenderingActivated message.
   webwidget_->layout();
 
   // Suppress painting if nothing is dirty.  This has to be done after updating
   // animations running layout as these may generate further invalidations.
   if (!paint_aggregator_.HasPendingUpdate())
     return;
@@ skipping 175 matching lines @@
   // contains a lot of host-renderer synchronization logic that is still
   // important for the accelerated compositing case. The option of simply
   // duplicating all that code is less desirable than "faking out" the
   // invalidation path using a magical damage rect.
   didInvalidateRect(WebRect(0, 0, 1, 1));
 }
 
 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));
     }
 }
 
 void RenderWidget::didChangeCursor(const WebCursorInfo& cursor_info) {
   // TODO(darin): Eliminate this temporary.
   WebCursor cursor(cursor_info);
 
   // Only send a SetCursor message if we need to make a change.
   if (!current_cursor_.IsEqual(cursor)) {
     current_cursor_ = cursor;
@@ skipping 335 matching lines @@
 
 void RenderWidget::CleanupWindowInPluginMoves(gfx::PluginWindowHandle window) {
   for (WebPluginGeometryVector::iterator i = plugin_window_moves_.begin();
        i != plugin_window_moves_.end(); ++i) {
     if (i->window == window) {
       plugin_window_moves_.erase(i);
       break;
     }
   }
 }