CC: Fix missing swap-used-incomplete-tile updates

When we early-out of a frame that has no damage, we didn't
correctly detect if the frame had incomplete tiles. This
patch only updates the flag if a swap actually occurs.

To prevent this kind of bug in the future, this patch
always sets the did-swap-use-incomplete-tile flag when it is
known, rather than resetting it to false in one place and
hoping it will always be set again correctly in all cases.

The test times-out without this patch.

BUG=284810

Review URL: https://chromiumcodereview.appspot.com/23686011

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

cc/scheduler/scheduler.cc

@@ -65,8 +65,8 @@ void Scheduler::SetNeedsRedraw() {
   ProcessScheduledActions();
 }
 
-void Scheduler::DidSwapUseIncompleteTile() {
-  state_machine_.DidSwapUseIncompleteTile();
+void Scheduler::SetSwapUsedIncompleteTile(bool used_incomplete_tile) {
+  state_machine_.SetSwapUsedIncompleteTile(used_incomplete_tile);
   ProcessScheduledActions();
 }
 

cc/scheduler/scheduler.h

@@ -77,7 +77,7 @@ class CC_EXPORT Scheduler {
 
   void SetMainThreadNeedsLayerTextures();
 
-  void DidSwapUseIncompleteTile();
+  void SetSwapUsedIncompleteTile(bool used_incomplete_tile);
 
   void FinishCommit();
   void BeginFrameAbortedByMainThread(bool did_handle);

cc/scheduler/scheduler_state_machine.cc

@@ -673,10 +673,8 @@ void SchedulerStateMachine::UpdateStateOnDraw(bool did_swap) {
   draw_if_possible_failed_ = false;
   active_tree_needs_first_draw_ = false;
 
-  if (did_swap) {
-    swap_used_incomplete_tile_ = false;
+  if (did_swap)
     last_frame_number_swap_performed_ = current_frame_number_;
-  }
 }
 
 void SchedulerStateMachine::SetMainThreadNeedsLayerTextures() {
@@ -759,8 +757,9 @@ void SchedulerStateMachine::SetVisible(bool visible) { visible_ = visible; }
 
 void SchedulerStateMachine::SetNeedsRedraw() { needs_redraw_ = true; }
 
-void SchedulerStateMachine::DidSwapUseIncompleteTile() {
-  swap_used_incomplete_tile_ = true;
+void SchedulerStateMachine::SetSwapUsedIncompleteTile(
+    bool used_incomplete_tile) {
+  swap_used_incomplete_tile_ = used_incomplete_tile;
 }
 
 void SchedulerStateMachine::DidDrawIfPossibleCompleted(bool success) {

cc/scheduler/scheduler_state_machine.h

@@ -128,9 +128,9 @@ class CC_EXPORT SchedulerStateMachine {
   // or the screen being damaged and simply needing redisplay.
   void SetNeedsRedraw();
 
-  // Indicates that a redraw is required because we are currently rendering
+  // Indicates whether a redraw is required because we are currently rendering
   // with a low resolution or checkerboarded tile.
-  void DidSwapUseIncompleteTile();
+  void SetSwapUsedIncompleteTile(bool used_incomplete_tile);
 
   // Indicates whether ACTION_DRAW_AND_SWAP_IF_POSSIBLE drew to the screen or
   // not.

cc/test/layer_tree_test.cc

@@ -117,6 +117,11 @@ class LayerTreeHostImplForTesting : public LayerTreeHostImpl {
     test_hooks_->SwapBuffersCompleteOnThread(this);
   }
 
+  virtual void UpdateVisibleTiles() OVERRIDE {
+    LayerTreeHostImpl::UpdateVisibleTiles();
+    test_hooks_->UpdateVisibleTilesOnThread(this);
+  }
+
   virtual void NotifyReadyToActivate() OVERRIDE {
     if (block_notify_ready_to_activate_for_testing_)
       notify_ready_to_activate_was_blocked_ = true;

cc/test/layer_tree_test.h

@@ -45,6 +45,7 @@ class TestHooks : public AnimationDelegate {
   virtual void DrawLayersOnThread(LayerTreeHostImpl* host_impl) {}
   virtual void SwapBuffersOnThread(LayerTreeHostImpl* host_impl, bool result) {}
   virtual void SwapBuffersCompleteOnThread(LayerTreeHostImpl* host_impl) {}
+  virtual void UpdateVisibleTilesOnThread(LayerTreeHostImpl* host_impl) {}
   virtual void AnimateLayers(LayerTreeHostImpl* host_impl,
                              base::TimeTicks monotonic_time) {}
   virtual void UpdateAnimationState(LayerTreeHostImpl* host_impl,

cc/trees/layer_tree_host_impl.h

@@ -271,7 +271,7 @@ class CC_EXPORT LayerTreeHostImpl
   const LayerTreeImpl* pending_tree() const { return pending_tree_.get(); }
   const LayerTreeImpl* recycle_tree() const { return recycle_tree_.get(); }
   virtual void CreatePendingTree();
-  void UpdateVisibleTiles();
+  virtual void UpdateVisibleTiles();
   virtual void ActivatePendingTree();
 
   // Shortcuts to layers on the active tree.
@@ -413,6 +413,8 @@ class CC_EXPORT LayerTreeHostImpl
 
   ResourceProvider::ResourceId ResourceIdForUIResource(UIResourceId uid) const;
 
+  void DidInitializeVisibleTileForTesting() { DidInitializeVisibleTile(); }
+
  protected:
   LayerTreeHostImpl(
       const LayerTreeSettings& settings,

cc/trees/layer_tree_host_unittest_damage.cc

@@ -12,6 +12,7 @@
 #include "cc/test/fake_content_layer.h"
 #include "cc/test/fake_content_layer_client.h"
 #include "cc/test/fake_painted_scrollbar_layer.h"
+#include "cc/test/fake_picture_layer.h"
 #include "cc/test/layer_tree_test.h"
 #include "cc/trees/damage_tracker.h"
 #include "cc/trees/layer_tree_impl.h"
@@ -688,5 +689,102 @@ class LayerTreeHostDamageTestScrollbarCommitDoesNoDamage
 
 MULTI_THREAD_TEST_F(LayerTreeHostDamageTestScrollbarCommitDoesNoDamage);
 
+class LayerTreeHostDamageTestVisibleTilesStillTriggerDraws
+    : public LayerTreeHostDamageTest {
+
+  virtual void InitializeSettings(LayerTreeSettings* settings) OVERRIDE {
+    settings->impl_side_painting = true;
+  }
+
+  virtual void BeginTest() OVERRIDE {
+    PostSetNeedsCommitToMainThread();
+  }
+
+  virtual void SetupTree() OVERRIDE {
+    scoped_refptr<FakePictureLayer> root = FakePictureLayer::Create(&client_);
+    root->SetBounds(gfx::Size(500, 500));
+    layer_tree_host()->SetRootLayer(root);
+    LayerTreeHostDamageTest::SetupTree();
+
+    swap_count_ = 0;
+    prepare_to_draw_count_ = 0;
+    update_visible_tile_count_ = 0;
+  }
+
+  virtual bool PrepareToDrawOnThread(LayerTreeHostImpl* host_impl,
+                                     LayerTreeHostImpl::FrameData* frame_data,
+                                     bool result) OVERRIDE {
+    EXPECT_TRUE(result);
+    prepare_to_draw_count_++;
+    switch (prepare_to_draw_count_) {
+      case 1:
+        // Detect that we have an incomplete tile, during the first frame.
+        // The first frame should have damage.
+        frame_data->contains_incomplete_tile = true;
+        DCHECK(!frame_data->has_no_damage);
+        break;
+      case 2:
+        // Make a no-damage frame. We early out and can't detect
+        // incomplete tiles, even if they still exist.
+        frame_data->contains_incomplete_tile = false;
+        frame_data->has_no_damage = true;
+        break;
+      case 3:
+        // Trigger the last swap for the completed tile.
+        frame_data->contains_incomplete_tile = false;
+        frame_data->has_no_damage = false;
+        EndTest();
+        break;
+      default:
+        NOTREACHED();
+        break;
+    }
+
+    return result;
+  }
+
+  virtual void UpdateVisibleTilesOnThread(
+      LayerTreeHostImpl* host_impl) OVERRIDE {
+    // Simulate creating some visible tiles (that trigger prepare-to-draws).
+    // The first we make into a no-damage-frame during prepare-to-draw (see
+    // above). This is to ensure we still get UpdateVisibleTiles calls after
+    // a no-damage or aborted frame.
+    update_visible_tile_count_++;
+    switch (update_visible_tile_count_) {
+      case 3:
+      case 6:
+        host_impl->DidInitializeVisibleTileForTesting();
+        break;
+      case 7:
+        NOTREACHED();
+        break;
+    }
+  }
+
+  virtual void SwapBuffersOnThread(LayerTreeHostImpl* host_impl,
+                                   bool didSwap) OVERRIDE {
+    if (!didSwap)
+      return;
+    ++swap_count_;
+  }
+
+  virtual void AfterTest() OVERRIDE {
+    // We should keep getting update-visible-tiles calls
+    // until we report there are no more incomplete-tiles.
+    EXPECT_EQ(update_visible_tile_count_, 6);
+    // First frame, plus two triggered by DidInitializeVisibleTile()
+    EXPECT_EQ(prepare_to_draw_count_, 3);
+    // First swap, plus final swap (contained damage).
+    EXPECT_EQ(swap_count_, 2);
+  }
+
+  FakeContentLayerClient client_;
+  int swap_count_;
+  int prepare_to_draw_count_;
+  int update_visible_tile_count_;
+};
+
+MULTI_THREAD_TEST_F(LayerTreeHostDamageTestVisibleTilesStillTriggerDraws);
+
 }  // namespace
 }  // namespace cc

cc/trees/thread_proxy.cc

@@ -544,10 +544,17 @@ void ThreadProxy::SetNeedsRedrawRectOnImplThread(gfx::Rect damage_rect) {
   SetNeedsRedrawOnImplThread();
 }
 
-void ThreadProxy::DidSwapUseIncompleteTileOnImplThread() {
+void ThreadProxy::SetSwapUsedIncompleteTileOnImplThread(
+    bool used_incomplete_tile) {
   DCHECK(IsImplThread());
-  TRACE_EVENT0("cc", "ThreadProxy::DidSwapUseIncompleteTileOnImplThread");
-  scheduler_on_impl_thread_->DidSwapUseIncompleteTile();
+  if (used_incomplete_tile) {
+    TRACE_EVENT_INSTANT0(
+        "cc",
+        "ThreadProxy::SetSwapUsedIncompleteTileOnImplThread",
+        TRACE_EVENT_SCOPE_THREAD);
+  }
+  scheduler_on_impl_thread_->SetSwapUsedIncompleteTile(
+    used_incomplete_tile);
 }
 
 void ThreadProxy::DidInitializeVisibleTileOnImplThread() {
@@ -1087,8 +1094,11 @@ DrawSwapReadbackResult ThreadProxy::DrawSwapReadbackInternal(
     DCHECK(swap_requested);
     result.did_swap = layer_tree_host_impl_->SwapBuffers(frame);
 
-    if (frame.contains_incomplete_tile)
-      DidSwapUseIncompleteTileOnImplThread();
+    // We don't know if we have incomplete tiles if we didn't actually swap.
+    if (result.did_swap) {
+      DCHECK(!frame.has_no_damage);
+      SetSwapUsedIncompleteTileOnImplThread(frame.contains_incomplete_tile);
+    }
   }
 
   // Tell the main thread that the the newly-commited frame was drawn.

cc/trees/thread_proxy.h

@@ -180,7 +180,7 @@ class ThreadProxy : public Proxy,
   void AsValueOnImplThread(CompletionEvent* completion,
                            base::DictionaryValue* state) const;
   void RenewTreePriorityOnImplThread();
-  void DidSwapUseIncompleteTileOnImplThread();
+  void SetSwapUsedIncompleteTileOnImplThread(bool used_incomplete_tile);
   void StartScrollbarAnimationOnImplThread();
   void MainThreadHasStoppedFlingingOnImplThread();
   void SetInputThrottledUntilCommitOnImplThread(bool is_throttled);