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 a9b27aad3ff45f405e99860fc179d58a1e485678..fe282fa22266cc5b809aaa36b5a267c2893fb46a 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,11 +304,11 @@ 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),
-      requires_high_res_to_draw_(false) {
+      requires_high_res_to_draw_(false),
+      required_for_draw_tile_is_top_of_raster_queue_(false) {
   DCHECK(proxy_->IsImplThread());
   DidVisibilityChange(this, visible_);
   animation_registrar_->set_supports_scroll_animations(
@@ -546,7 +546,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 +895,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;
     }
@@ -1113,12 +1111,11 @@ DrawResult LayerTreeHostImpl::PrepareToDraw(FrameData* frame) {
                "LayerTreeHostImpl::PrepareToDraw",
                "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;
+  // This will cause NotifyTileStateChanged() to be called for any visible tiles
+  // that completed, which will add damage to the frame for them so they appear
+  // as part of the current frame being drawn.g

[brianderson, 2014/10/31 22:42:53]

typo: ".g"

[danakj, 2014/11/03 16:26:04]

I blame emacs. Thanks :)

+  if (settings().impl_side_painting)
+    tile_manager_->UpdateVisibleTiles();
 
   UMA_HISTOGRAM_CUSTOM_COUNTS(
       "Compositing.NumActiveLayers", active_tree_->NumLayers(), 1, 400, 20);
@@ -1130,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()) {
@@ -1161,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();
@@ -1259,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());
@@ -1302,6 +1286,13 @@ void LayerTreeHostImpl::BuildRasterQueue(RasterTilePriorityQueue* queue,
   picture_layer_pairs_.clear();
   GetPictureLayerImplPairs(&picture_layer_pairs_);
   queue->Build(picture_layer_pairs_, tree_priority);
+
+  if (!queue->IsEmpty()) {
+    required_for_draw_tile_is_top_of_raster_queue_ =
+        queue->Top()->required_for_draw();
+  } else {
+    required_for_draw_tile_is_top_of_raster_queue_ = false;
+  }

[reveman, 2014/10/31 20:28:39]

How about using conditional assignment here instead of if-else?

[danakj, 2014/10/31 20:43:01]

It's a similar number of lines of code with the line wrapping, and I thot this
was easier to read.

[reveman, 2014/10/31 21:38:13]

Acknowledged.

 }
 
 void LayerTreeHostImpl::BuildEvictionQueue(EvictionTilePriorityQueue* queue,
@@ -1322,7 +1313,10 @@ void LayerTreeHostImpl::NotifyReadyToActivate() {
 }
 
 void LayerTreeHostImpl::NotifyReadyToDraw() {
-  client_->NotifyReadyToDraw();

[brianderson, 2014/10/31 22:42:54]

Was this accidentally removed? The scheduler doesn't use NotifyReadyToDraw yet,
but it will soon.

[danakj, 2014/11/03 16:26:04]

I assumed it could go away since we already SetNeedsRedraw() for each tile that
gets completed. From your comment elsewhere it sounds like not.

+  // Tiles that are ready will cause NotifyTileStateChanged() to be called so we
+  // don't need to schedule a draw here. Just stop WillBeginImplFrame() from
+  // causing optimistic requests to draw a frame.
+  required_for_draw_tile_is_top_of_raster_queue_ = false;
 }
 
 void LayerTreeHostImpl::NotifyTileStateChanged(const Tile* tile) {
@@ -1341,6 +1335,13 @@ 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->required_for_draw()) {
+    // The LayerImpl::NotifyTileStateChanged() should damage the layer, so this
+    // redraw will make those tiles be displayed.
+    SetNeedsRedraw();
+  }
 }
 
 void LayerTreeHostImpl::SetMemoryPolicy(const ManagedMemoryPolicy& policy) {
@@ -1689,6 +1690,13 @@ void LayerTreeHostImpl::WillBeginImplFrame(const BeginFrameArgs& args) {
   UpdateCurrentBeginFrameArgs(args);
   // Cache the begin impl frame interval
   begin_impl_frame_interval_ = args.interval;
+
+  if (required_for_draw_tile_is_top_of_raster_queue_) {
+    // Optimistically schedule a draw, as a tile required for draw is at the top
+    // of the current raster queue. This will let us expect the tile to complete
+    // and draw it within the impl frame we are beginning now.
+    SetNeedsRedraw();
+  }
 }
 
 void LayerTreeHostImpl::UpdateViewportContainerSizes() {
@@ -1816,15 +1824,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());
 
@@ -2028,7 +2028,6 @@ void LayerTreeHostImpl::CreateAndSetTileManager() {
                                       scheduled_raster_task_limit);
 
   UpdateTileManagerMemoryPolicy(ActualManagedMemoryPolicy());
-  need_to_update_visible_tiles_before_draw_ = false;
 }
 
 void LayerTreeHostImpl::CreateResourceAndRasterWorkerPool(
@@ -3030,9 +3029,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,