Activate LayerImpl tree with sync+push instead of pointer swap

cc/layer_tree_host_impl.cc

Index: cc/layer_tree_host_impl.cc
diff --git a/cc/layer_tree_host_impl.cc b/cc/layer_tree_host_impl.cc
index 797b46d6ce624c5c55a732df28e9ea864e8ae6f2..54433b0263375e7306f798f547193061a9bfcce7 100644
--- a/cc/layer_tree_host_impl.cc
+++ b/cc/layer_tree_host_impl.cc
@@ -39,6 +39,7 @@
 #include "cc/solid_color_draw_quad.h"
 #include "cc/texture_uploader.h"
 #include "cc/top_controls_manager.h"
+#include "cc/tree_synchronizer.h"
 #include "cc/util.h"
 #include "ui/gfx/size_conversions.h"
 #include "ui/gfx/vector2d_conversions.h"
@@ -176,6 +177,7 @@ LayerTreeHostImpl::~LayerTreeHostImpl()
         // made a contract with our animation controllers that the registrar
         // will outlive them, and we must make good.
         m_activeTree.reset();
+        m_recycleTree.reset();

[enne (OOO), 2013/01/17 23:34:55]

There are a couple of other cleanup functions in LTHI, like initializeRenderer
calling sendDidLoseOutputSurfaceRecursive and clearRenderSurfaces.  Should those
get called on the recycle tree as well?

[jamesr, 2013/01/18 01:16:42]

Added code to clear the render surfaces on the recycle tree after activation.

Lost context is less clear, because logically the recycle tree shouldn't have
ownership of any resources. However it will have resource IDs and I think it'll
be really confusing to have bogus resource IDs on the recycle tree the next time
we try to sync to it, so added sendDidLose...() calls for the recycle tree as
well

         m_pendingTree.reset();
     }
 }
@@ -919,7 +921,10 @@ LayerTreeImpl* LayerTreeHostImpl::activeTree()
 void LayerTreeHostImpl::createPendingTree()
 {
     CHECK(!m_pendingTree);
-    m_pendingTree = LayerTreeImpl::create(this);
+    if (m_recycleTree)
+        m_recycleTree.swap(m_pendingTree);
+    else
+        m_pendingTree = LayerTreeImpl::create(this);
     m_client->onCanDrawStateChanged(canDraw());
     m_client->onHasPendingTreeStateChanged(pendingTree());
 }
@@ -951,9 +956,13 @@ void LayerTreeHostImpl::activatePendingTree()
     CHECK(m_pendingTree);
 
     m_activeTree->PushPersistedState(m_pendingTree.get());
-    m_activeTree.swap(m_pendingTree);
-    // TODO(enne): consider recycling this tree to prevent layer churn
-    m_pendingTree.reset();
+    m_activeTree->SetRootLayer(TreeSynchronizer::synchronizeTrees(m_pendingTree->RootLayer(), m_activeTree->DetachLayerTree(), m_activeTree.get()));
+    TreeSynchronizer::pushProperties(m_pendingTree->RootLayer(), m_activeTree->RootLayer());
+    DCHECK(!m_recycleTree);
+    m_activeTree->set_source_frame_number(m_pendingTree->source_frame_number());

[enne (OOO), 2013/01/17 23:34:55]

I wish there were some better way to establish the correspondence between this
block of code pushing tree values and the one in LayerTreeHost.  Maybe some
comment saying "new values pushed to the tree here should also be added to this
other function"?

[jamesr, 2013/01/18 01:16:42]

Added a comment.  While adding this I also thought it would be nicer to move
this code in LayerTreeHost over to this class so they can live side-by-side (or
even possibly share code), but that seemed like a bit too much to pile into this
patch.

+    m_activeTree->set_background_color(m_pendingTree->background_color());
+    m_activeTree->set_has_transparent_background(m_pendingTree->has_transparent_background());
+    m_pendingTree.swap(m_recycleTree);

[jamesr, 2013/01/18 01:16:42]

added another comment here, for kicks

     m_activeTree->DidBecomeActive();
 
     m_client->onCanDrawStateChanged(canDraw());