cc: Make scheduling be driven by vsync for android webview.

android_webview/browser/browser_view_renderer.cc

Index: android_webview/browser/browser_view_renderer.cc
diff --git a/android_webview/browser/browser_view_renderer.cc b/android_webview/browser/browser_view_renderer.cc
index 486702bc6b10a046000c999c86bf13cece094aa2..2029edf53de52192dbc66a8eb9d22b38b17fec96 100644
--- a/android_webview/browser/browser_view_renderer.cc
+++ b/android_webview/browser/browser_view_renderer.cc
@@ -68,7 +68,6 @@ BrowserViewRenderer::BrowserViewRenderer(
       page_scale_factor_(1.0),
       on_new_picture_enable_(false),
       clear_view_(false),
-      compositor_needs_continuous_invalidate_(false),
       invalidate_after_composite_(false),
       block_invalidates_(false),
       fallback_tick_pending_(false) {
@@ -241,6 +240,15 @@ scoped_ptr<cc::CompositorFrame> BrowserViewRenderer::CompositeHw() {
   return frame.Pass();
 }
 
+void BrowserViewRenderer::ForceCompositeHw() {
+  ReturnResourceFromParent();
+  ReturnUnusedResource(shared_renderer_state_.PassUncommittedFrameOnUI());
+  scoped_ptr<cc::CompositorFrame> frame = CompositeHw();
+  if (frame.get()) {
+    shared_renderer_state_.SetCompositorFrameOnUI(frame.Pass(), true);
+  }
+}
+
 void BrowserViewRenderer::UpdateParentDrawConstraints() {
   // Post an invalidate if the parent draw constraints are stale and there is
   // no pending invalidate.
@@ -250,7 +258,7 @@ void BrowserViewRenderer::UpdateParentDrawConstraints() {
       !parent_draw_constraints_.Equals(
           shared_renderer_state_.GetParentDrawConstraintsOnUI())) {
     shared_renderer_state_.SetForceInvalidateOnNextDrawGLOnUI(false);
-    EnsureContinuousInvalidation(true, needs_force_invalidate);
+    PostInvalidateWithFallback(needs_force_invalidate);
   }
 }
 
@@ -323,7 +331,7 @@ void BrowserViewRenderer::ClearView() {
 
   clear_view_ = true;
   // Always invalidate ignoring the compositor to actually clear the webview.
-  EnsureContinuousInvalidation(true, false);
+  PostInvalidateWithFallback(false);
 }
 
 void BrowserViewRenderer::SetIsPaused(bool paused) {
@@ -333,7 +341,7 @@ void BrowserViewRenderer::SetIsPaused(bool paused) {
                        "paused",
                        paused);
   is_paused_ = paused;
-  EnsureContinuousInvalidation(false, false);
+  UpdateCompositorIsActive();

[boliu, 2015/02/11 02:28:15]

Hmm, for now when fallback tick is still not removed, it's still important to
check the state of fallback ticks, since fallback tick might go from throttled
to non-throttled. In that case, if compositor is waiting for an invalidate, then
we need to schedule a fallback tick.

 }
 
 void BrowserViewRenderer::SetViewVisibility(bool view_visible) {
@@ -352,7 +360,7 @@ void BrowserViewRenderer::SetWindowVisibility(bool window_visible) {
                        "window_visible",
                        window_visible);
   window_visible_ = window_visible;
-  EnsureContinuousInvalidation(false, false);
+  UpdateCompositorIsActive();
 }
 
 void BrowserViewRenderer::OnSizeChanged(int width, int height) {
@@ -375,6 +383,7 @@ void BrowserViewRenderer::OnAttachedToWindow(int width, int height) {
                height);
   attached_to_window_ = true;
   size_.SetSize(width, height);
+  UpdateCompositorIsActive();
 }
 
 void BrowserViewRenderer::OnDetachedFromWindow() {
@@ -382,6 +391,7 @@ void BrowserViewRenderer::OnDetachedFromWindow() {
   shared_renderer_state_.ReleaseHardwareDrawIfNeededOnUI();
   attached_to_window_ = false;
   DCHECK(!hardware_enabled_);
+  UpdateCompositorIsActive();
 }
 
 void BrowserViewRenderer::ReleaseHardware() {
@@ -413,6 +423,7 @@ void BrowserViewRenderer::DidInitializeCompositor(
   DCHECK(compositor);
   DCHECK(!compositor_);
   compositor_ = compositor;
+  UpdateCompositorIsActive();
 }
 
 void BrowserViewRenderer::DidDestroyCompositor(
@@ -422,20 +433,6 @@ void BrowserViewRenderer::DidDestroyCompositor(
   compositor_ = NULL;
 }
 
-void BrowserViewRenderer::SetContinuousInvalidate(bool invalidate) {
-  if (compositor_needs_continuous_invalidate_ == invalidate)
-    return;
-
-  TRACE_EVENT_INSTANT1("android_webview",
-                       "BrowserViewRenderer::SetContinuousInvalidate",
-                       TRACE_EVENT_SCOPE_THREAD,
-                       "invalidate",
-                       invalidate);
-  compositor_needs_continuous_invalidate_ = invalidate;
-
-  EnsureContinuousInvalidation(false, false);
-}
-
 void BrowserViewRenderer::SetDipScale(float dip_scale) {
   dip_scale_ = dip_scale;
   CHECK_GT(dip_scale_, 0.f);
@@ -603,25 +600,26 @@ void BrowserViewRenderer::DidOverscroll(gfx::Vector2dF accumulated_overscroll,
   client_->DidOverscroll(rounded_overscroll_delta);
 }
 
-void BrowserViewRenderer::EnsureContinuousInvalidation(
-    bool force_invalidate,
+void BrowserViewRenderer::PostInvalidate() {
+  TRACE_EVENT_INSTANT0("android_webview", "BrowserViewRenderer::PostInvalidate",
+                       TRACE_EVENT_SCOPE_THREAD);
+  PostInvalidateWithFallback(false);
+}
+
+void BrowserViewRenderer::PostInvalidateWithFallback(
     bool skip_reschedule_tick) {
-  if (force_invalidate)
+  if (block_invalidates_) {
     invalidate_after_composite_ = true;

[boliu, 2015/02/11 02:28:14]

I think BVR needs a general clean up before having massive changes like this :(

invalidate_after_composite_ and skip_reschedule_tick can be removed period, we
don't need or want that behavior anymore

block_invalidate_ and fallback_tick_pending_ can probably be merged into one
because DidSkipCompositeInDraw should be removed too

I don't know what's easier, me trying to clean this up first and you try a
rebase, just remove these things here and I'll clean the pieces up later..

-
-  // This method should be called again when any of these conditions change.
-  bool need_invalidate =
-      compositor_needs_continuous_invalidate_ || invalidate_after_composite_;
-  if (!need_invalidate || block_invalidates_)
     return;
-
-  if (!compositor_needs_continuous_invalidate_ && invalidate_after_composite_)
-    invalidate_after_composite_ = false;
+  }
 
   // Always call view invalidate. We rely the Android framework to ignore the
   // invalidate when it's not needed such as when view is not visible.
   client_->PostInvalidate();
 
+  // Always clear any queued invalidates when posting a new one.
+  invalidate_after_composite_ = false;
+
   // Stop fallback ticks when one of these is true.
   // 1) Webview is paused. Also need to check we are not in clear view since
   // paused, offscreen still expect clear view to recover.
@@ -630,69 +628,55 @@ void BrowserViewRenderer::EnsureContinuousInvalidation(
   // "on-screen" but that updates are not needed when in the background.
   bool throttle_fallback_tick =
       (is_paused_ && !clear_view_) || (attached_to_window_ && !window_visible_);

[boliu, 2015/02/11 02:28:14]

clear_view_ *was* here because we still want activations to happen, because
activations unsets clear_view_. So in theory, we don't care about clear_view_
here (ie draw), but we do care about it in UpdateCompositorIsActive (ie
activation).

But this is all too complicated to reason about. How about use the exact same
(or I guess opposite) condition in both places for now?

+
   if (throttle_fallback_tick)
     return;
 
-  block_invalidates_ = compositor_needs_continuous_invalidate_;
+  // Start blocking any invalidates posted. Eventually DidComposite or
+  // DidSkipCompositeInDraw will be called either via onDraw or the fallback
+  // tick and invalidates will be unblocked.
+  block_invalidates_ = true;
+
   if (skip_reschedule_tick && fallback_tick_pending_)
     return;
 
-  // Unretained here is safe because the callbacks are cancelled when
-  // they are destroyed.
   post_fallback_tick_.Reset(base::Bind(&BrowserViewRenderer::PostFallbackTick,
                                        base::Unretained(this)));
   fallback_tick_fired_.Cancel();
-  fallback_tick_pending_ = false;
 
-  // No need to reschedule fallback tick if compositor does not need to be
-  // ticked. This can happen if this is reached because force_invalidate is
-  // true.
-  if (compositor_needs_continuous_invalidate_) {
-    fallback_tick_pending_ = true;
-    ui_task_runner_->PostTask(FROM_HERE, post_fallback_tick_.callback());
-  }
+  fallback_tick_pending_ = true;
+  ui_task_runner_->PostTask(FROM_HERE, post_fallback_tick_.callback());
 }
 
 void BrowserViewRenderer::PostFallbackTick() {
   DCHECK(fallback_tick_fired_.IsCancelled());
   fallback_tick_fired_.Reset(base::Bind(&BrowserViewRenderer::FallbackTickFired,
                                         base::Unretained(this)));
-  if (compositor_needs_continuous_invalidate_) {
+  if (compositor_) {

[boliu, 2015/02/11 02:28:14]

don't really need this check either

     ui_task_runner_->PostDelayedTask(
         FROM_HERE,
         fallback_tick_fired_.callback(),
         base::TimeDelta::FromMilliseconds(kFallbackTickTimeoutInMilliseconds));
-  } else {
-    // Pretend we just composited to unblock further invalidates.
-    DidComposite();
   }
+  // Pretend we just composited to unblock further invalidates.
+  DidComposite();

[boliu, 2015/02/11 02:28:14]

just remove this, once compositor is gone, it's gone for good

 }
 
 void BrowserViewRenderer::FallbackTickFired() {
-  TRACE_EVENT1("android_webview",
-               "BrowserViewRenderer::FallbackTickFired",
-               "compositor_needs_continuous_invalidate_",
-               compositor_needs_continuous_invalidate_);
-
+  TRACE_EVENT0("android_webview", "BrowserViewRenderer::FallbackTickFired");
   // This should only be called if OnDraw or DrawGL did not come in time, which
   // means block_invalidates_ must still be true.
   DCHECK(block_invalidates_);
   fallback_tick_pending_ = false;
-  if (compositor_needs_continuous_invalidate_ && compositor_) {
+  if (compositor_) {
     if (hardware_enabled_) {
-      ReturnResourceFromParent();
-      ReturnUnusedResource(shared_renderer_state_.PassUncommittedFrameOnUI());
-      scoped_ptr<cc::CompositorFrame> frame = CompositeHw();
-      if (frame.get()) {
-        shared_renderer_state_.SetCompositorFrameOnUI(frame.Pass(), true);
-      }
+      ForceCompositeHw();
     } else {
       ForceFakeCompositeSW();
     }
-  } else {
-    // Pretend we just composited to unblock further invalidates.
-    DidComposite();
   }
+  // Pretend we just composited to unblock further invalidates.
+  DidComposite();

[boliu, 2015/02/11 02:28:15]

ditto

 }
 
 void BrowserViewRenderer::ForceFakeCompositeSW() {
@@ -714,15 +698,25 @@ bool BrowserViewRenderer::CompositeSW(SkCanvas* canvas) {
 
 void BrowserViewRenderer::DidComposite() {
   block_invalidates_ = false;
+
   post_fallback_tick_.Cancel();
   fallback_tick_fired_.Cancel();
   fallback_tick_pending_ = false;
-  EnsureContinuousInvalidation(false, false);
+
+  if (invalidate_after_composite_)
+    PostInvalidateWithFallback(false);
 }
 
 void BrowserViewRenderer::DidSkipCompositeInDraw() {
+  DCHECK(block_invalidates_);
   block_invalidates_ = false;
-  EnsureContinuousInvalidation(true, true);
+  PostInvalidateWithFallback(true);
+}
+
+void BrowserViewRenderer::UpdateCompositorIsActive() {
+  if (compositor_)
+    compositor_->SetIsActive(!is_paused_ &&
+                             (!attached_to_window_ || window_visible_));
 }
 
 std::string BrowserViewRenderer::ToString() const {
@@ -732,10 +726,11 @@ std::string BrowserViewRenderer::ToString() const {
   base::StringAppendF(&str, "window_visible: %d ", window_visible_);
   base::StringAppendF(&str, "dip_scale: %f ", dip_scale_);
   base::StringAppendF(&str, "page_scale_factor: %f ", page_scale_factor_);
-  base::StringAppendF(&str,
-                      "compositor_needs_continuous_invalidate: %d ",
-                      compositor_needs_continuous_invalidate_);
+  base::StringAppendF(&str, "invalidate_after_composite: %d ",
+                      invalidate_after_composite_);
   base::StringAppendF(&str, "block_invalidates: %d ", block_invalidates_);
+  base::StringAppendF(&str, "fallback_tick_pending: %d ",
+                      fallback_tick_pending_);
   base::StringAppendF(&str, "view size: %s ", size_.ToString().c_str());
   base::StringAppendF(&str, "attached_to_window: %d ", attached_to_window_);
   base::StringAppendF(&str,