SetNeedsRedraw directly when updating a visible tile.

cc/trees/layer_tree_host_impl.cc

Index: cc/trees/layer_tree_host_impl.cc
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index c5ee10e59350d02e998744334cd02e8047bc0792..625dff0a61637b7cfa571d8430b16558c792c295 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -238,8 +238,8 @@ class LayerTreeHostImplTimeSourceAdapter : public TimeSourceClient {
   DISALLOW_COPY_AND_ASSIGN(LayerTreeHostImplTimeSourceAdapter);
 };
 
-LayerTreeHostImpl::FrameData::FrameData()
-    : contains_incomplete_tile(false), has_no_damage(false) {}
+LayerTreeHostImpl::FrameData::FrameData() : has_no_damage(false) {
+}
 
 LayerTreeHostImpl::FrameData::~FrameData() {}
 
@@ -304,7 +304,6 @@ LayerTreeHostImpl::LayerTreeHostImpl(
       animation_registrar_(AnimationRegistrar::Create()),
       rendering_stats_instrumentation_(rendering_stats_instrumentation),
       micro_benchmark_controller_(this),
-      need_to_update_visible_tiles_before_draw_(false),
       shared_bitmap_manager_(shared_bitmap_manager),
       gpu_memory_buffer_manager_(gpu_memory_buffer_manager),
       id_(id),
@@ -546,7 +545,6 @@ void LayerTreeHostImpl::TrackDamageForAllSurfaces(
 
 void LayerTreeHostImpl::FrameData::AsValueInto(
     base::debug::TracedValue* value) const {
-  value->SetBoolean("contains_incomplete_tile", contains_incomplete_tile);
   value->SetBoolean("has_no_damage", has_no_damage);
 
   // Quad data can be quite large, so only dump render passes if we select
@@ -896,7 +894,6 @@ DrawResult LayerTreeHostImpl::CalculateRenderPasses(
 
     if (append_quads_data.num_incomplete_tiles ||
         append_quads_data.num_missing_tiles) {
-      frame->contains_incomplete_tile = true;
       if (RequiresHighResToDraw())
         draw_result = DRAW_ABORTED_MISSING_HIGH_RES_CONTENT;
     }
@@ -1117,11 +1114,8 @@ DrawResult LayerTreeHostImpl::PrepareToDraw(FrameData* frame) {
                "SourceFrameNumber",
                active_tree_->source_frame_number());
 
-  if (need_to_update_visible_tiles_before_draw_ &&
-      tile_manager_ && tile_manager_->UpdateVisibleTiles()) {
-    DidInitializeVisibleTile();
-  }
-  need_to_update_visible_tiles_before_draw_ = true;
+  if (settings().impl_side_painting)
+    tile_manager_->UpdateVisibleTiles();
 
   UMA_HISTOGRAM_CUSTOM_COUNTS(
       "Compositing.NumActiveLayers", active_tree_->NumLayers(), 1, 400, 20);
@@ -1133,7 +1127,6 @@ DrawResult LayerTreeHostImpl::PrepareToDraw(FrameData* frame) {
   frame->render_passes.clear();
   frame->render_passes_by_id.clear();
   frame->will_draw_layers.clear();
-  frame->contains_incomplete_tile = false;
   frame->has_no_damage = false;
 
   if (active_tree_->root_layer()) {
@@ -1164,13 +1157,6 @@ void LayerTreeHostImpl::BlockNotifyReadyToActivateForTesting(bool block) {
   NOTREACHED();
 }
 
-void LayerTreeHostImpl::DidInitializeVisibleTileForTesting() {
-  // Add arbitrary damage, to trigger prepare-to-draws.
-  // Here, setting damage as viewport size, used only for testing.
-  SetFullRootLayerDamage();
-  DidInitializeVisibleTile();
-}
-
 void LayerTreeHostImpl::ResetTreesForTesting() {
   if (active_tree_)
     active_tree_->DetachLayerTree();
@@ -1262,11 +1248,6 @@ void LayerTreeHostImpl::DidModifyTilePriorities() {
   client_->SetNeedsManageTilesOnImplThread();
 }
 
-void LayerTreeHostImpl::DidInitializeVisibleTile() {
-  if (client_ && !client_->IsInsideDraw())
-    client_->DidInitializeVisibleTileOnImplThread();
-}
-
 void LayerTreeHostImpl::GetPictureLayerImplPairs(
     std::vector<PictureLayerImpl::Pair>* layer_pairs) const {
   DCHECK(layer_pairs->empty());
@@ -1340,6 +1321,14 @@ void LayerTreeHostImpl::NotifyTileStateChanged(const Tile* tile) {
     if (layer_impl)
       layer_impl->NotifyTileStateChanged(tile);
   }
+
+  // Check for a non-null active tree to avoid doing this during shutdown.
+  if (active_tree_ && !client_->IsInsideDraw() &&
+      tile->priority(ACTIVE_TREE).distance_to_visible == 0.f) {
+    // The LayerImpl::NotifyTileStateChanged() should damage the layer, so this
+    // redraw will make those tiles be displayed.
+    SetNeedsRedraw();
+  }
 }
 
 void LayerTreeHostImpl::SetMemoryPolicy(const ManagedMemoryPolicy& policy) {
@@ -1688,6 +1677,14 @@ void LayerTreeHostImpl::WillBeginImplFrame(const BeginFrameArgs& args) {
   UpdateCurrentBeginFrameArgs(args);
   // Cache the begin impl frame interval
   begin_impl_frame_interval_ = args.interval;
+
+  if (settings().impl_side_painting &&
+      tile_manager_->has_active_visible_tile_scheduled()) {
+    // Pre-emptively schedule a draw now if any visible tile is being created,
+    // so that we will expect it with our decisions for choosing a deadline for
+    // the impl frame we are beginning now.
+    SetNeedsRedraw();

[reveman, 2014/10/30 19:37:42]

So this is just speculative. Would be nice if we had a better way to inform the
scheduler that we might need to redraw than calling SetNeedsRedraw and
pretending that we really need to redraw.

[danakj, 2014/10/30 19:49:07]

I could add something, but it wouldn't be that different in practice. When we
need to draw we go to PrepareToDraw, do UpdateVisibleTiles, see if there is any
damage, and early out. I think that's better than adding a CheckVisibleTiles
step, then check some state, then start a draw and have some state to prevent
check visible tiles again (which is more like what we had before). It grew to be
complicated and we can just try draw and early out it's much simpler.

[reveman, 2014/10/31 00:44:01]

Ok, SetNeedsRedraw really means something like SetNeedsRedrawIfDamageExists. You
can keep this as SetNeedsRedraw for now but we might want to follow up with some
naming changes if the redraw is conditional on damage to make the intent here a
bit more clear.

[danakj, 2014/10/31 15:39:46]

We could, I lean slightly toward leaving it how it is. It's an old name and we
always draw "what is damaged". And it does try to redraw in the future
regardless of damage existing now or not, IfDamageExists kinda sounds like the
Set part early outs rather than the Draw part.

[reveman, 2014/10/31 16:08:50]

Yes, SetMayNeedRedraw? :) Let's not worry about this now though. SetNeedsRedraw
is fine for now. It just looked wrong when reading the code but I understand now
that it's not as bad as it looks.

+  }
 }
 
 void LayerTreeHostImpl::UpdateViewportContainerSizes() {
@@ -1809,15 +1806,7 @@ void LayerTreeHostImpl::CreatePendingTree() {
   TRACE_EVENT_ASYNC_BEGIN0("cc", "PendingTree:waiting", pending_tree_.get());
 }
 
-void LayerTreeHostImpl::UpdateVisibleTiles() {
-  if (tile_manager_ && tile_manager_->UpdateVisibleTiles())
-    DidInitializeVisibleTile();
-  need_to_update_visible_tiles_before_draw_ = false;
-}
-
 void LayerTreeHostImpl::ActivateSyncTree() {
-  need_to_update_visible_tiles_before_draw_ = true;
-
   if (pending_tree_) {
     TRACE_EVENT_ASYNC_END0("cc", "PendingTree:waiting", pending_tree_.get());
 
@@ -2077,7 +2066,6 @@ void LayerTreeHostImpl::CreateAndSetTileManager() {
                                       settings().scheduled_raster_task_limit);
 
   UpdateTileManagerMemoryPolicy(ActualManagedMemoryPolicy());
-  need_to_update_visible_tiles_before_draw_ = false;
 }
 
 void LayerTreeHostImpl::DestroyTileManager() {
@@ -2988,9 +2976,6 @@ void LayerTreeHostImpl::PinchGestureEnd() {
   // scales that we want when we're not inside a pinch.
   active_tree_->set_needs_update_draw_properties();
   SetNeedsRedraw();
-  // TODO(danakj): Don't set root damage. Just updating draw properties and
-  // getting new tiles rastered should be enough! crbug.com/427423
-  SetFullRootLayerDamage();
 }
 
 static void CollectScrollDeltas(ScrollAndScaleSet* scroll_info,