cc: Control activation from the Scheduler

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 9f664fcc0ee40385784ff1cf74c61cc32fcf5371..6d053e66ada912520870a08fb46c1a99036749e2 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -110,12 +110,8 @@ class LayerTreeHostImplTimeSourceAdapter : public TimeSourceClient {
 
     // TODO(enne): This should probably happen post-animate.
     if (layer_tree_host_impl_->pending_tree()) {
-      layer_tree_host_impl_->ActivatePendingTreeIfNeeded();
-
-      if (layer_tree_host_impl_->pending_tree()) {
-        layer_tree_host_impl_->pending_tree()->UpdateDrawProperties();
-        layer_tree_host_impl_->ManageTiles();
-      }
+      layer_tree_host_impl_->pending_tree()->UpdateDrawProperties();
+      layer_tree_host_impl_->ManageTiles();
     }
 
     layer_tree_host_impl_->Animate(
@@ -254,7 +250,10 @@ void LayerTreeHostImpl::CommitComplete() {
     pending_tree_->set_needs_update_draw_properties();
     pending_tree_->UpdateDrawProperties();
     // Start working on newly created tiles immediately if needed.
-    ManageTiles();
+    if (!manage_tiles_needed_)
+      client_->NotifyReadyToActivate();
+    else
+      ManageTiles();
   } else {
     active_tree_->set_needs_update_draw_properties();
   }
@@ -1085,10 +1084,7 @@ void LayerTreeHostImpl::DidInitializeVisibleTile() {
 }
 
 void LayerTreeHostImpl::NotifyReadyToActivate() {
-  if (pending_tree_) {
-    need_to_update_visible_tiles_before_draw_ = true;
-    ActivatePendingTree();
-  }
+  client_->NotifyReadyToActivate();
 }
 
 bool LayerTreeHostImpl::ShouldClearRootRenderPass() const {
@@ -1433,7 +1429,6 @@ void LayerTreeHostImpl::CreatePendingTree() {
   else
     pending_tree_ = LayerTreeImpl::create(this);
   client_->OnCanDrawStateChanged(CanDraw());
-  client_->OnHasPendingTreeStateChanged(pending_tree_);
   TRACE_EVENT_ASYNC_BEGIN0("cc", "PendingTree", pending_tree_.get());
   TRACE_EVENT_ASYNC_STEP0("cc",
                           "PendingTree", pending_tree_.get(), "waiting");
@@ -1449,33 +1444,12 @@ void LayerTreeHostImpl::UpdateVisibleTiles() {
   need_to_update_visible_tiles_before_draw_ = false;
 }
 
-void LayerTreeHostImpl::ActivatePendingTreeIfNeeded() {
-  DCHECK(pending_tree_);
-  CHECK(settings_.impl_side_painting);
-
-  if (!pending_tree_)
-    return;
-
-  // The tile manager is usually responsible for notifying activation.
-  // If there is no tile manager, then we need to manually activate.
-  if (!tile_manager_ || tile_manager_->AreTilesRequiredForActivationReady()) {

[enne (OOO), 2013/08/26 21:55:13]

Where does this removed !tile_manager logic end up in your patch?  Maybe this
should go with the !manage_tiles_needed conditional to immediately activate
during commit?

Could you add a basic LayerTreeHostTest with impl-side painting to make sure
that a TileManager-less LTHI will still continue to activate and not get stuck?

[brianderson, 2013/08/26 22:47:25]

Good idea. I will add handle the !time_manager case and add a test that fails if
that case is not handled.

[brianderson, 2013/08/26 23:33:20]

In looking at how to implement this test, it seems like we're only missinga
tile_manager when we don't have a renderer / output surface.  In which case, the
SchedulerStateMachine::PendingActicationsShouldBeForced() will force activations
and there are many context lost tests that exercise this code path.

Is there a case where we can have an output surface but no tile_manager_ that
i'm missing?

-    ActivatePendingTree();
-    return;
-  }
-
-  // Manage tiles in case state affecting tile priority has changed.
-  ManageTiles();
-
-  TRACE_EVENT_ASYNC_STEP1(
-    "cc",
-    "PendingTree", pending_tree_.get(), "activate",
-    "state", TracedValue::FromValue(ActivationStateAsValue().release()));
-}
-
 void LayerTreeHostImpl::ActivatePendingTree() {
   CHECK(pending_tree_);
   TRACE_EVENT_ASYNC_END0("cc", "PendingTree", pending_tree_.get());
 
+  need_to_update_visible_tiles_before_draw_ = true;
+
   active_tree_->SetRootLayerScrollOffsetDelegate(NULL);
   active_tree_->PushPersistedState(pending_tree_.get());
   if (pending_tree_->needs_full_tree_sync()) {
@@ -1504,12 +1478,15 @@ void LayerTreeHostImpl::ActivatePendingTree() {
       root_layer_scroll_offset_delegate_);
   active_tree_->DidBecomeActive();
 
+  client_->DidActivatePendingTree();

[enne (OOO), 2013/08/26 21:55:13]

Why did this move from the end of the function? (Just trying to understand.)

[brianderson, 2013/08/26 22:47:25]

I did this a long time ago and copied it over, so I forget my original
motivation... but I think it was to unblock other threads as soon as possible. I
wonder if I could move this up to the top of the function even and rename it to
WillActivatePendingTree().

In any case, it's only tangentially related to this patch, so I should probably
make it a separate patch for easier bisecting if anything goes wrong.

+  if (!tree_activation_callback_.is_null())
+    tree_activation_callback_.Run();
+
   // Reduce wasted memory now that unlinked resources are guaranteed not
   // to be used.
   client_->ReduceWastedContentsTextureMemoryOnImplThread();
 
   client_->OnCanDrawStateChanged(CanDraw());
-  client_->OnHasPendingTreeStateChanged(pending_tree_);
   client_->SetNeedsRedrawOnImplThread();
   client_->RenewTreePriority();
 
@@ -1520,10 +1497,6 @@ void LayerTreeHostImpl::ActivatePendingTree() {
         stats.main_stats.paint_time + stats.main_stats.record_time +
             stats.impl_stats.rasterize_time_for_now_bins_on_pending_tree);
   }
-
-  client_->DidActivatePendingTree();
-  if (!tree_activation_callback_.is_null())
-    tree_activation_callback_.Run();
 }
 
 void LayerTreeHostImpl::SetVisible(bool visible) {