cc: Move more functionality from host to LayerTreeImpl

cc: Move more functionality from host to LayerTreeImpl

This cleans up some longstanding TODOs that were creating when splitting
out logic into LayerTreeImpl.  It's possible that some of the helpers to
get at active tree layers (root, scrolling, etc) could be moved, but
given that scrolling only happens on the active tree (with the pending
tree being updated with the result), these accessors could arguably stay
permanently.

This also moves background color / has transparent background to the tree
itself, so that these changes will synchronize with any content on that
tree.  Things like viewport, device scale, and debug state are considered
host-specific and take effect immediately.

R=nduca@chromium.org
BUG=155209
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=175252

[danakj, 2012-12-31 15:18:38]

This is me just keeping up with what's happening.

https://codereview.chromium.org/11704003/diff/1/cc/delegated_renderer_layer_i...
File cc/delegated_renderer_layer_impl_unittest.cc (right):

https://codereview.chromium.org/11704003/diff/1/cc/delegated_renderer_layer_i...
cc/delegated_renderer_layer_impl_unittest.cc:110:
m_hostImpl->activeTree()->SetRootLayer(rootLayer.Pass());
IIUC, in general the LTH would never set the root layer directly on the active
tree, right? It set it on the pending tree, then cause a tree swap when ready?


It seems like these tests that were written against the LTHImpl are getting
further from the truth. I should probably make these tests create main thread
objects instead of playing around with impl stuff directly. WDYT?

https://codereview.chromium.org/11704003/diff/1/cc/layer_tree_host_impl.h
File cc/layer_tree_host_impl.h (left):

https://codereview.chromium.org/11704003/diff/1/cc/layer_tree_host_impl.h#old...
cc/layer_tree_host_impl.h:227: // TOOD(nduca): This goes away when scrolling
moves to LayerTreeImpl.
Just double checking that you meant to remove this TOOD without removing the
method it speaks about.

[enne (OOO), 2012-12-31 23:52:36]

https://codereview.chromium.org/11704003/diff/1/cc/delegated_renderer_layer_i...
File cc/delegated_renderer_layer_impl_unittest.cc (right):

https://codereview.chromium.org/11704003/diff/1/cc/delegated_renderer_layer_i...
cc/delegated_renderer_layer_impl_unittest.cc:110:
m_hostImpl->activeTree()->SetRootLayer(rootLayer.Pass());
On 2012/12/31 15:18:38, danakj wrote:
> IIUC, in general the LTH would never set the root layer directly on the active
> tree, right? It set it on the pending tree, then cause a tree swap when ready?
> 
> 
> It seems like these tests that were written against the LTHImpl are getting
> further from the truth. I should probably make these tests create main thread
> objects instead of playing around with impl stuff directly. WDYT?

Yeah, with impl-side painting, the LTH would only set the pending tree.  See the
way tree sync works with the syncTree variable in layer_tree_host.cc.

However, in tests, I don't think it's *that* problematic to just "skip right to
the active tree" where you're really just trying to construct a tree to test
with, rather than trying to test the commit flow.

Honestly, I find it more problematic that we need so much infrastructure to test
what passes a layer tree generates.  Having to add a pending tree in that mix
doesn't make that any better.  In other words, I understand where you're coming
from and I don't disagree, but I think I'd rather see us spend time on better
abstractions rather than making things more real by pulling in more boilerplate
to do simple things.

https://codereview.chromium.org/11704003/diff/1/cc/layer_tree_host_impl.h
File cc/layer_tree_host_impl.h (left):

https://codereview.chromium.org/11704003/diff/1/cc/layer_tree_host_impl.h#old...
cc/layer_tree_host_impl.h:227: // TOOD(nduca): This goes away when scrolling
moves to LayerTreeImpl.
On 2012/12/31 15:18:38, danakj wrote:
> Just double checking that you meant to remove this TOOD without removing the
> method it speaks about.

Definitely intended.  This is what I obliquely was waving my hands at in the
first paragraph of the CL description about methods that could stay on the host.

[danakj, 2013-01-01 00:39:22]

https://codereview.chromium.org/11704003/diff/1/cc/delegated_renderer_layer_i...
File cc/delegated_renderer_layer_impl_unittest.cc (right):

https://codereview.chromium.org/11704003/diff/1/cc/delegated_renderer_layer_i...
cc/delegated_renderer_layer_impl_unittest.cc:110:
m_hostImpl->activeTree()->SetRootLayer(rootLayer.Pass());
On 2012/12/31 23:52:36, enne wrote:
> On 2012/12/31 15:18:38, danakj wrote:
> > IIUC, in general the LTH would never set the root layer directly on the
active
> > tree, right? It set it on the pending tree, then cause a tree swap when
ready?
> > 
> > 
> > It seems like these tests that were written against the LTHImpl are getting
> > further from the truth. I should probably make these tests create main
thread
> > objects instead of playing around with impl stuff directly. WDYT?
> 
> Yeah, with impl-side painting, the LTH would only set the pending tree.  See
the
> way tree sync works with the syncTree variable in layer_tree_host.cc.
> 
> However, in tests, I don't think it's *that* problematic to just "skip right
to
> the active tree" where you're really just trying to construct a tree to test
> with, rather than trying to test the commit flow.
> 
> Honestly, I find it more problematic that we need so much infrastructure to
test
> what passes a layer tree generates.  Having to add a pending tree in that mix
> doesn't make that any better.  In other words, I understand where you're
coming
> from and I don't disagree, but I think I'd rather see us spend time on better
> abstractions rather than making things more real by pulling in more
boilerplate
> to do simple things.

Yah okay, fair enough. Hmmm abstractions..

[danakj, 2013-01-04 22:14:28]

In the spirit of Nat: Three cheers for sweet LGTMMMmmm

https://codereview.chromium.org/11704003/diff/1/cc/layer_tree_impl.h
File cc/layer_tree_impl.h (right):

https://codereview.chromium.org/11704003/diff/1/cc/layer_tree_impl.h#newcode100
cc/layer_tree_impl.h:100: bool has_transparent_background() const { return
has_transparent_background_; }
80 cols

[commit-bot: I haz the power, 2013-01-04 22:23:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/11704003/9001

[commit-bot: I haz the power, 2013-01-04 22:23:53]

Failed to apply patch for cc/layer_tree_host_impl.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/layer_tree_host_impl.cc
  Hunk #1 FAILED at 219.
  Hunk #2 succeeded at 614 (offset 10 lines).
  Hunk #3 succeeded at 1000 (offset 12 lines).
  Hunk #4 succeeded at 1040 (offset 12 lines).
  1 out of 4 hunks FAILED -- saving rejects to file
cc/layer_tree_host_impl.cc.rej

Patch:       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
ac160844b11f26821d788b6d66f719419f66fcd4..dd1a4bf2449e73c0de051de173a2059786038617
100644
--- a/cc/layer_tree_host_impl.cc
+++ b/cc/layer_tree_host_impl.cc
@@ -219,8 +219,6 @@ LayerTreeHostImpl::LayerTreeHostImpl(const
LayerTreeSettings& settings, LayerTre
                             PriorityCalculator::allowEverythingCutoff(),
                             0,
                             PriorityCalculator::allowNothingCutoff())
-    , m_backgroundColor(0)
-    , m_hasTransparentBackground(false)
     , m_needsUpdateDrawProperties(false)
     , m_pinchGestureActive(false)
     , m_fpsCounter(FrameRateCounter::create(m_proxy->hasImplThread()))
@@ -604,9 +602,9 @@ bool LayerTreeHostImpl::calculateRenderPasses(FrameData&
frame)
     }
 #endif
 
-    if (!m_hasTransparentBackground) {
+    if (!activeTree()->has_transparent_background()) {
         frame.renderPasses.last()->has_transparent_background = false;
-        appendQuadsToFillScreen(frame.renderPasses.last(), rootLayer(),
m_backgroundColor, occlusionTracker);
+        appendQuadsToFillScreen(frame.renderPasses.last(), rootLayer(),
activeTree()->background_color(), occlusionTracker);
     }
 
     if (drawFrame)
@@ -988,12 +986,6 @@ static LayerImpl* findScrollLayerForContentLayer(LayerImpl*
layerImpl)
     return 0;
 }
 
-void LayerTreeHostImpl::setRootLayer(scoped_ptr<LayerImpl> layer)
-{
-    m_activeTree->SetRootLayer(layer.Pass());
-    setNeedsUpdateDrawProperties();
-}
-
 void LayerTreeHostImpl::createPendingTree()
 {
     CHECK(!m_pendingTree);
@@ -1034,12 +1026,6 @@ void LayerTreeHostImpl::activatePendingTree()
     m_client->onHasPendingTreeStateChanged(pendingTree());
 }
 
-scoped_ptr<LayerImpl> LayerTreeHostImpl::detachLayerTree()
-{
-    scoped_ptr<LayerImpl> layer = m_activeTree->DetachLayerTree();
-    return layer.Pass();
-}
-
 void LayerTreeHostImpl::setVisible(bool visible)
 {
     DCHECK(m_proxy->isImplThread());

[commit-bot: I haz the power, 2013-01-04 22:51:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/11704003/12001

[commit-bot: I haz the power, 2013-01-05 01:27:02]

Change committed as 175252