Activate LayerImpl tree with sync+push instead of pointer swap

Activate LayerImpl tree with sync+push instead of pointer swap

This changes the activation model to do a push + sync from the pending -> active
tree instead of just doing a pointer swap.  This fixes behavior for video and
scrollbar animations and allows for recycling of the LayerImpl tree from commit
to commit.  There are several changes:

*) The tree synchronization code is templatized to support Layer->LayerImpl and
LayerImpl->LayerImpl tree syncs.  All LayerImpl subclasses now support a
createLayerImpl() call to construct a LayerImpl of matching dynamic type and a
LayerImpl::pushPropertiesTo(LayerImpl*) to sync properties.  The structural
syncing stuff is identical to the existing Layer->LayerImpl sync

*) Video layer's provider flow is tweaked.  Instead of the VideoLayerImpl being
the VideoFrameProvider::Client, a new object VideoFrameProviderClientImpl is. 
This is constructed initially on the pending tree and associated with the
VideoFrameProvider during initial commit (since that happens with the main
thread blocked).  A reference to this object is passed to the active tree, which
then registers itself for setNeedsRedraw notifications when it becomes active. 
On shutdown, the pending layer's destructor (which always happens first) unhooks
the VFPCI from the VideoFrameProvider.  This step has to happen during the
pending tree commit, not activation, since it has to happen on the impl thread
with the main thread blocked.  The active VideoLayerImpl can then proceed with a
VFPCI that isn't wired up to a VFP until the pending tree is activated, at which
point it tears down the VFPCI as well.

*) Scrollbar layer IDs are hooked up on the impl thread as well so we can set up
the pointers during LayerImpl->LayerImpl tree syncs

*) Picture layer syncing is largely unchanged, except that there's new code to
do a PictureLayerImpl -> PictureLayerImpl sync.  This step simply replaces the
active layer's pile and tilings with the pending tree's.

BUG=169143
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=178267

[jamesr, 2013-01-17 22:42:18]

enne@ - please review
piman@ - please check the texture mailbox stuff (and anything else you see)

This changes the activation model to do a push + sync from the pending -> active
tree instead of just doing a pointer swap.  This fixes behavior for video and
scrollbar animations and allows for recycling of the LayerImpl tree from commit
to commit.  There are several changes:

*) The tree synchronization code is templatized to support Layer->LayerImpl and
LayerImpl->LayerImpl tree syncs.  All LayerImpl subclasses now support a
createLayerImpl() call to construct a LayerImpl of matching dynamic type and a
LayerImpl::pushPropertiesTo(LayerImpl*) to sync properties.  The structural
syncing stuff is identical to the existing Layer->LayerImpl sync

*) Video layer's provider flow is tweaked.  Instead of the VideoLayerImpl being
the VideoFrameProvider::Client, a new object VideoFrameProviderClientImpl is. 
This is constructed initially on the pending tree and associated with the
VideoFrameProvider during initial commit (since that happens with the main
thread blocked).  A reference to this object is passed to the active tree, which
then registers itself for setNeedsRedraw notifications when it becomes active. 
On shutdown, the pending layer's destructor (which always happens first) unhooks
the VFPCI from the VideoFrameProvider.  This step has to happen during the
pending tree commit, not activation, since it has to happen on the impl thread
with the main thread blocked.  The active VideoLayerImpl can then proceed with a
VFPCI that isn't wired up to a VFP until the pending tree is activated, at which
point it tears down the VFPCI as well.

*) Scrollbar layer IDs are hooked up on the impl thread as well so we can set up
the pointers during LayerImpl->LayerImpl tree syncs

*) Picture layer syncing is largely unchanged, except that there's new code to
do a PictureLayerImpl -> PictureLayerImpl sync.  This step simply replaces the
active layer's pile and tilings with the pending tree's.

[piman, 2013-01-17 23:27:12]

On 2013/01/17 22:42:18, jamesr wrote:
> enne@ - please review
> piman@ - please check the texture mailbox stuff (and anything else you see)

LGTM. I looked in depth at the texture layer, and a took a cursory look at the
rest.

[enne (OOO), 2013-01-17 23:34:54]

DelegatedRendererLayerImpl and SolidColorLayerImpl don't seem to need
pushPropertiesTo, but surely they need createLayerImpl overrides?

https://codereview.chromium.org/11882037/diff/15003/cc/layer.h
File cc/layer.h (right):

https://codereview.chromium.org/11882037/diff/15003/cc/layer.h#newcode70
cc/layer.h:70: Layer* childAt(size_t index) const;
Can this be not const?

https://codereview.chromium.org/11882037/diff/15003/cc/layer_tree_host_impl.cc
File cc/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/11882037/diff/15003/cc/layer_tree_host_impl.c...
cc/layer_tree_host_impl.cc:180: m_recycleTree.reset();
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?

https://codereview.chromium.org/11882037/diff/15003/cc/layer_tree_host_impl.c...
cc/layer_tree_host_impl.cc:962:
m_activeTree->set_source_frame_number(m_pendingTree->source_frame_number());
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"?

https://codereview.chromium.org/11882037/diff/15003/cc/layer_tree_host_impl.h
File cc/layer_tree_host_impl.h (right):

https://codereview.chromium.org/11882037/diff/15003/cc/layer_tree_host_impl.h...
cc/layer_tree_host_impl.h:314: scoped_ptr<LayerTreeImpl> m_recycleTree;
Can you add some comment here about how all these trees work?

https://codereview.chromium.org/11882037/diff/15003/cc/picture_layer_tiling_s...
File cc/picture_layer_tiling_set.h (left):

https://codereview.chromium.org/11882037/diff/15003/cc/picture_layer_tiling_s...
cc/picture_layer_tiling_set.h:109: Region invalidation_;
Oops, thanks.

https://codereview.chromium.org/11882037/diff/15003/cc/picture_pile_impl.cc
File cc/picture_pile_impl.cc (right):

https://codereview.chromium.org/11882037/diff/15003/cc/picture_pile_impl.cc#n...
cc/picture_pile_impl.cc:102: void
PicturePileImpl::PushPropertiesTo(PicturePileImpl* other) {
You should just call PicturePileBase::PushPropertiesTo and then just copy the
clones here.

https://codereview.chromium.org/11882037/diff/15003/cc/video_frame_provider_c...
File cc/video_frame_provider_client_impl.cc (right):

https://codereview.chromium.org/11882037/diff/15003/cc/video_frame_provider_c...
cc/video_frame_provider_client_impl.cc:25: // This only happens during a commit
on the compositor thread while the main
Can we assert that the main thread is blocked here too (and in Stop)?

https://codereview.chromium.org/11882037/diff/15003/cc/video_frame_provider_c...
cc/video_frame_provider_client_impl.cc:77: void
VideoFrameProviderClientImpl::DidUpdateMatrix(const float* matrix) {
What was the issue with the intermittently incorrect frame 1 stream texture
matrix?

[danakj, 2013-01-18 00:06:13]

I likey the video stuff.

https://codereview.chromium.org/11882037/diff/15003/cc/video_frame_provider_c...
File cc/video_frame_provider_client_impl.cc (right):

https://codereview.chromium.org/11882037/diff/15003/cc/video_frame_provider_c...
cc/video_frame_provider_client_impl.cc:43:
provider_->SetVideoFrameProviderClient(0);
nit: s/0/NULL/. also on next line.

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

New patch incoming with SolidColorLayerImpl / Delegating... - missed those since
they don't have pushPropertiesTo() and aren't reachable in web content.

https://codereview.chromium.org/11882037/diff/15003/cc/layer.h
File cc/layer.h (right):

https://codereview.chromium.org/11882037/diff/15003/cc/layer.h#newcode70
cc/layer.h:70: Layer* childAt(size_t index) const;
On 2013/01/17 23:34:55, enne wrote:
> Can this be not const?

No, TreeSynchronizer calls Layer::pushPropertiesTo() on the return value and
those functions aren't (generally) const

https://codereview.chromium.org/11882037/diff/15003/cc/layer_tree_host_impl.cc
File cc/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/11882037/diff/15003/cc/layer_tree_host_impl.c...
cc/layer_tree_host_impl.cc:180: m_recycleTree.reset();
On 2013/01/17 23:34:55, enne wrote:
> 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?

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

https://codereview.chromium.org/11882037/diff/15003/cc/layer_tree_host_impl.c...
cc/layer_tree_host_impl.cc:962:
m_activeTree->set_source_frame_number(m_pendingTree->source_frame_number());
On 2013/01/17 23:34:55, enne wrote:
> 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"?

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.

https://codereview.chromium.org/11882037/diff/15003/cc/layer_tree_host_impl.c...
cc/layer_tree_host_impl.cc:965: m_pendingTree.swap(m_recycleTree);
added another comment here, for kicks

https://codereview.chromium.org/11882037/diff/15003/cc/layer_tree_host_impl.h
File cc/layer_tree_host_impl.h (right):

https://codereview.chromium.org/11882037/diff/15003/cc/layer_tree_host_impl.h...
cc/layer_tree_host_impl.h:314: scoped_ptr<LayerTreeImpl> m_recycleTree;
On 2013/01/17 23:34:55, enne wrote:
> Can you add some comment here about how all these trees work?

Done.

https://codereview.chromium.org/11882037/diff/15003/cc/picture_pile_impl.cc
File cc/picture_pile_impl.cc (right):

https://codereview.chromium.org/11882037/diff/15003/cc/picture_pile_impl.cc#n...
cc/picture_pile_impl.cc:102: void
PicturePileImpl::PushPropertiesTo(PicturePileImpl* other) {
On 2013/01/17 23:34:55, enne wrote:
> You should just call PicturePileBase::PushPropertiesTo and then just copy the
> clones here.

Ah, missed that there was a base class.  Done

https://codereview.chromium.org/11882037/diff/15003/cc/video_frame_provider_c...
File cc/video_frame_provider_client_impl.cc (right):

https://codereview.chromium.org/11882037/diff/15003/cc/video_frame_provider_c...
cc/video_frame_provider_client_impl.cc:25: // This only happens during a commit
on the compositor thread while the main
On 2013/01/17 23:34:55, enne wrote:
> Can we assert that the main thread is blocked here too (and in Stop)?

Not without plumbing a pointer to the cc::Proxy down into this call (can't rely
on static state since that breaks in single process mode).  It would be quite a
journey - shortest path I can see is:

VideoFrameProviderClientImpl ->
VideoLayerImpl ->
LayerTreeImpl ->
LayerTreeHostImpl ->
Proxy

On the flip side, not having asserts probably means crazy threading bugs.

I'll try adding some up in VideoLayerImpl

https://codereview.chromium.org/11882037/diff/15003/cc/video_frame_provider_c...
cc/video_frame_provider_client_impl.cc:43:
provider_->SetVideoFrameProviderClient(0);
On 2013/01/18 00:06:13, danakj wrote:
> nit: s/0/NULL/. also on next line.

Done.

[enne (OOO), 2013-01-18 01:21:35]

https://codereview.chromium.org/11882037/diff/15003/cc/layer.h
File cc/layer.h (right):

https://codereview.chromium.org/11882037/diff/15003/cc/layer.h#newcode70
cc/layer.h:70: Layer* childAt(size_t index) const;
On 2013/01/18 01:16:42, jamesr wrote:
> On 2013/01/17 23:34:55, enne wrote:
> > Can this be not const?
> 
> No, TreeSynchronizer calls Layer::pushPropertiesTo() on the return value and
> those functions aren't (generally) const

Maybe you misunderstood? I was asking if this could instead be:

    Layer* childAt(size_t index);

...just for general "don't return a non-const pointer from a const member
function" reasons.

[jamesr, 2013-01-18 01:27:14]

https://codereview.chromium.org/11882037/diff/15003/cc/layer.h
File cc/layer.h (right):

https://codereview.chromium.org/11882037/diff/15003/cc/layer.h#newcode70
cc/layer.h:70: Layer* childAt(size_t index) const;
On 2013/01/18 01:21:35, enne wrote:
> On 2013/01/18 01:16:42, jamesr wrote:
> > On 2013/01/17 23:34:55, enne wrote:
> > > Can this be not const?
> > 
> > No, TreeSynchronizer calls Layer::pushPropertiesTo() on the return value and
> > those functions aren't (generally) const
> 
> Maybe you misunderstood? I was asking if this could instead be:
> 
>     Layer* childAt(size_t index);
> 
> ...just for general "don't return a non-const pointer from a const member
> function" reasons.

Oh right!  Definitely, anyone calling this should have a non-const pointer.

[klobag.chromium, 2013-01-18 09:23:33]

FYI, while applying this CL locally to Chrome for Android, I saw the screen
freeze when switching tabs. e.g. one tab techcrunch.com, one tab theverge.com,
scroll, then switch tabs. Repeat couple of times, screen stops updating. Trace
showing all the threads are still active.

[jamesr, 2013-01-18 21:43:36]

I think I've got everything working now, but will land a few cleanups first to
make this patch smaller and less risky.  First one here:

https://codereview.chromium.org/12025031/

[jamesr, 2013-01-18 21:46:48]

On 2013/01/18 09:23:33, klobag.chromium wrote:
> FYI, while applying this CL locally to Chrome for Android, I saw the screen
> freeze when switching tabs. e.g. one tab http://techcrunch.com, one tab
http://theverge.com,
> scroll, then switch tabs. Repeat couple of times, screen stops updating. Trace
> showing all the threads are still active.

Haven't seen this, although in the version you applied pinch and scrolling were
busted, which can seem like a frozen tab if you're trying to pinch/scroll.  Was
the tab unresponsive to all updates or just pinch/scroll?

[jamesr, 2013-01-21 20:49:47]

Looks like after switching tabs we get stuck in a state where we constantly
think contents textures are purged.

[jamesr, 2013-01-21 21:35:58]

On 2013/01/21 20:49:47, jamesr wrote:
> Looks like after switching tabs we get stuck in a state where we constantly
> think contents textures are purged.

This patch isn't syncing LayerTreeImpl::contents_textures_purged_, so that makes
sense.  Will verify and update the patch.

[jamesr, 2013-01-22 00:42:23]

Latest version fixes the tab switch hang, at least for me.  I can't test against
ToT until tomorrow.

I really doubt the texture mailbox handling is correct, but I don't think they
work in impl-side painting without this patch either.

[enne (OOO), 2013-01-22 21:03:38]

lgtm

[commit-bot: I haz the power, 2013-01-22 21:09:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/11882037/32001

[commit-bot: I haz the power, 2013-01-22 23:54:32]

Failed to apply patch for cc/picture_layer_impl.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/picture_layer_impl.cc
  Hunk #6 FAILED at 208.
  Hunk #9 succeeded at 285 (offset 4 lines).
  Hunk #10 succeeded at 324 (offset 4 lines).
  Hunk #11 succeeded at 349 (offset 4 lines).
  Hunk #12 succeeded at 410 (offset 4 lines).
  Hunk #13 succeeded at 420 (offset 4 lines).
  Hunk #14 succeeded at 471 (offset 4 lines).
  Hunk #15 succeeded at 482 (offset 4 lines).
  1 out of 15 hunks FAILED -- saving rejects to file
cc/picture_layer_impl.cc.rej

Patch:       cc/picture_layer_impl.cc
Index: cc/picture_layer_impl.cc
diff --git a/cc/picture_layer_impl.cc b/cc/picture_layer_impl.cc
index
ec355422c62bae4d4d6bcb51f69cc29598181c12..f901e710ccd94614578b01c721f02a8028291167
100644
--- a/cc/picture_layer_impl.cc
+++ b/cc/picture_layer_impl.cc
@@ -25,7 +25,6 @@ namespace cc {
 
 PictureLayerImpl::PictureLayerImpl(LayerTreeImpl* treeImpl, int id)
     : LayerImpl(treeImpl, id),
-      tilings_(this),
       pile_(PicturePileImpl::Create()),
       last_update_time_(0),
       last_content_scale_(0),
@@ -40,6 +39,36 @@ const char* PictureLayerImpl::layerTypeAsString() const {
   return "PictureLayer";
 }
 
+scoped_ptr<LayerImpl> PictureLayerImpl::createLayerImpl(
+    LayerTreeImpl* treeImpl) {
+  return PictureLayerImpl::create(treeImpl, id()).PassAs<LayerImpl>();
+}
+
+void PictureLayerImpl::CreateTilingSet() {
+  DCHECK(layerTreeImpl()->IsPendingTree());
+  DCHECK(!tilings_);
+  tilings_.reset(new PictureLayerTilingSet(this));
+  tilings_->SetLayerBounds(bounds());
+}
+
+void PictureLayerImpl::TransferTilingSet(scoped_ptr<PictureLayerTilingSet>
tilings) {
+  DCHECK(layerTreeImpl()->IsActiveTree());
+  tilings->SetClient(this);
+  tilings_ = tilings.Pass();
+}
+
+void PictureLayerImpl::pushPropertiesTo(LayerImpl* base_layer) {
+  LayerImpl::pushPropertiesTo(base_layer);
+
+  PictureLayerImpl* layer_impl = static_cast<PictureLayerImpl*>(base_layer);
+
+  layer_impl->SetIsMask(is_mask_);
+  layer_impl->TransferTilingSet(tilings_.Pass());
+  layer_impl->pile_ = pile_;
+  pile_ = PicturePileImpl::Create();
+}
+
+
 void PictureLayerImpl::appendQuads(QuadSink& quadSink,
                                    AppendQuadsData& appendQuadsData) {
   const gfx::Rect& rect = visibleContentRect();
@@ -56,7 +85,7 @@ void PictureLayerImpl::appendQuads(QuadSink& quadSink,
   bool useAA = !isAxisAlignedInTarget;
 
   if (showDebugBorders()) {
-    for (PictureLayerTilingSet::Iterator iter(&tilings_,
+    for (PictureLayerTilingSet::Iterator iter(tilings_.get(),
                                               contentsScaleX(),
                                               rect,
                                               ideal_contents_scale_);
@@ -84,7 +113,7 @@ void PictureLayerImpl::appendQuads(QuadSink& quadSink,
   // unused can be considered for removal.
   std::vector<PictureLayerTiling*> seen_tilings;
 
-  for (PictureLayerTilingSet::Iterator iter(&tilings_,
+  for (PictureLayerTilingSet::Iterator iter(tilings_.get(),
                                             contentsScaleX(),
                                             rect,
                                             ideal_contents_scale_);
@@ -162,7 +191,7 @@ void PictureLayerImpl::didUpdateTransforms() {
     time_delta = current_time - last_update_time_;
   }
   WhichTree tree = layerTreeImpl()->IsActiveTree() ? ACTIVE_TREE :
PENDING_TREE;
-  tilings_.UpdateTilePriorities(
+  tilings_->UpdateTilePriorities(
       tree,
       layerTreeImpl()->device_viewport_size(),
       last_content_scale_,
@@ -179,11 +208,13 @@ void PictureLayerImpl::didUpdateTransforms() {
 }
 
 void PictureLayerImpl::didBecomeActive() {
-  tilings_.MoveTilePriorities(PENDING_TREE, ACTIVE_TREE);
+  LayerImpl::didBecomeActive();
+  tilings_->MoveTilePriorities(PENDING_TREE, ACTIVE_TREE);
 }
 
 void PictureLayerImpl::didLoseOutputSurface() {
-  tilings_.RemoveAllTilings();
+  if (tilings_)
+    tilings_->RemoveAllTilings();
 }
 
 void PictureLayerImpl::calculateContentsScale(
@@ -192,7 +223,7 @@ void PictureLayerImpl::calculateContentsScale(
     float* contents_scale_y,
     gfx::Size* content_bounds) {
   if (!drawsContent()) {
-    DCHECK(!tilings_.num_tilings());
+    DCHECK(!tilings_->num_tilings());
     return;
   }
 
@@ -208,8 +239,8 @@ void PictureLayerImpl::calculateContentsScale(
   // tilings (and then map back to floating point texture coordinates), the
   // contents scale must be at least as large as the largest of the tilings.
   float max_contents_scale = min_contents_scale;
-  for (size_t i = 0; i < tilings_.num_tilings(); ++i) {
-    const PictureLayerTiling* tiling = tilings_.tiling_at(i);
+  for (size_t i = 0; i < tilings_->num_tilings(); ++i) {
+    const PictureLayerTiling* tiling = tilings_->tiling_at(i);
     max_contents_scale = std::max(max_contents_scale,
tiling->contents_scale());
   }
 
@@ -252,26 +283,26 @@ void PictureLayerImpl::SyncFromActiveLayer() {
 }
 
 void PictureLayerImpl::SyncFromActiveLayer(const PictureLayerImpl* other) {
-  tilings_.CloneAll(other->tilings_, invalidation_);
-  DCHECK(bounds() == tilings_.LayerBounds());
+  tilings_->CloneAll(*other->tilings_, invalidation_);
+  DCHECK(bounds() == tilings_->LayerBounds());
 }
 
 void PictureLayerImpl::SyncTiling(
     const PictureLayerTiling* tiling) {
-  tilings_.Clone(tiling, invalidation_);
+  tilings_->Clone(tiling, invalidation_);
 }
 
 void PictureLayerImpl::SetIsMask(bool is_mask) {
   if (is_mask_ == is_mask)
     return;
   is_mask_ = is_mask;
-  tilings_.RemoveAllTiles();
+  tilings_->RemoveAllTiles();
 }
 
 ResourceProvider::ResourceId PictureLayerImpl::contentsResourceId() const {
   gfx::Rect content_rect(gfx::Point(), contentBounds());
   float scale = contentsScaleX();
-  for (PictureLayerTilingSet::Iterator iter(&tilings_,
+  for (PictureLayerTilingSet::Iterator iter(tilings_.get(),
                                             scale,
                                             content_rect,
                                             ideal_contents_scale_);
@@ -291,8 +322,8 @@ ResourceProvider::ResourceId
PictureLayerImpl::contentsResourceId() const {
 bool PictureLayerImpl::areVisibleResourcesReady() const {
   const gfx::Rect& rect = visibleContentRect();
 
-  for (size_t i = 0; i < tilings_.num_tilings(); ++i) {
-    const PictureLayerTiling* tiling = tilings_.tiling_at(i);
+  for (size_t i = 0; i < tilings_->num_tilings(); ++i) {
+    const PictureLayerTiling* tiling = tilings_->tiling_at(i);
 
     // Ignore non-high resolution tilings.
     if (tiling->resolution() != HIGH_RESOLUTION)
@@ -316,7 +347,7 @@ PictureLayerTiling* PictureLayerImpl::AddTiling(float
contents_scale) {
   if (contents_scale < layerTreeImpl()->settings().minimumContentsScale)
     return NULL;
 
-  PictureLayerTiling* tiling = tilings_.AddTiling(
+  PictureLayerTiling* tiling = tilings_->AddTiling(
       contents_scale,
       TileSize());
 
@@ -377,8 +408,8 @@ void PictureLayerImpl::ManageTilings(float
ideal_contents_scale) {
   if (layerTreeImpl()->IsPendingTree() &&
       !layerTreeImpl()->PinchGestureActive()) {
     std::vector<PictureLayerTiling*> remove_list;
-    for (size_t i = 0; i < tilings_.num_tilings(); ++i) {
-      PictureLayerTiling* tiling = tilings_.tiling_at(i);
+    for (size_t i = 0; i < tilings_->num_tilings(); ++i) {
+      PictureLayerTiling* tiling = tilings_->tiling_at(i);
       if (tiling->contents_scale() == ideal_contents_scale)
         continue;
       if (tiling->contents_scale() == low_res_contents_scale)
@@ -387,14 +418,14 @@ void PictureLayerImpl::ManageTilings(float
ideal_contents_scale) {
     }
 
     for (size_t i = 0; i < remove_list.size(); ++i)
-      tilings_.Remove(remove_list[i]);
+      tilings_->Remove(remove_list[i]);
   }
 
   // Find existing tilings closest to ideal high / low res.
   PictureLayerTiling* high_res = NULL;
   PictureLayerTiling* low_res = NULL;
-  for (size_t i = 0; i < tilings_.num_tilings(); ++i) {
-    PictureLayerTiling* tiling = tilings_.tiling_at(i);
+  for (size_t i = 0; i < tilings_->num_tilings(); ++i) {
+    PictureLayerTiling* tiling = tilings_->tiling_at(i);
     if (!high_res || IsCloserToThan(tiling, high_res, ideal_contents_scale))
       high_res = tiling;
     if (!low_res || IsCloserToThan(tiling, low_res, low_res_contents_scale))
@@ -438,8 +469,8 @@ void PictureLayerImpl::CleanUpUnusedTilings(
     std::vector<PictureLayerTiling*> used_tilings) {
   std::vector<PictureLayerTiling*> to_remove;
 
-  for (size_t i = 0; i < tilings_.num_tilings(); ++i) {
-    PictureLayerTiling* tiling = tilings_.tiling_at(i);
+  for (size_t i = 0; i < tilings_->num_tilings(); ++i) {
+    PictureLayerTiling* tiling = tilings_->tiling_at(i);
     // Don't remove the current high or low res tilinig.
     if (tiling->resolution() != NON_IDEAL_RESOLUTION)
       continue;
@@ -449,7 +480,7 @@ void PictureLayerImpl::CleanUpUnusedTilings(
   }
 
   for (size_t i = 0; i < to_remove.size(); ++i)
-    tilings_.Remove(to_remove[i]);
+    tilings_->Remove(to_remove[i]);
 }
 
 }  // namespace cc

[commit-bot: I haz the power, 2013-01-23 00:11:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/11882037/39002

[commit-bot: I haz the power, 2013-01-23 02:52:17]

Retried try job too often on mac_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

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

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/11882037/39002

[commit-bot: I haz the power, 2013-01-23 04:14:32]

Failed to apply patch for cc/delegated_renderer_layer_impl.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/delegated_renderer_layer_impl.cc
  Hunk #1 FAILED at 60.
  1 out of 1 hunk FAILED -- saving rejects to file
cc/delegated_renderer_layer_impl.cc.rej

Patch:       cc/delegated_renderer_layer_impl.cc
Index: cc/delegated_renderer_layer_impl.cc
diff --git a/cc/delegated_renderer_layer_impl.cc
b/cc/delegated_renderer_layer_impl.cc
index
1207d3d3e03d9ba297506177f99088f43180e08e..8d85325aa913c2921304db98bb458a76da25cbc9
100644
--- a/cc/delegated_renderer_layer_impl.cc
+++ b/cc/delegated_renderer_layer_impl.cc
@@ -60,6 +60,11 @@ void DelegatedRendererLayerImpl::clearRenderPasses()
     m_renderPassesInDrawOrder.clear();
 }
 
+scoped_ptr<LayerImpl>
DelegatedRendererLayerImpl::createLayerImpl(LayerTreeImpl* treeImpl)
+{
+    return DelegatedRendererLayerImpl::create(treeImpl,
id()).PassAs<LayerImpl>();
+}
+
 void DelegatedRendererLayerImpl::didLoseOutputSurface()
 {
     clearRenderPasses();

[commit-bot: I haz the power, 2013-01-23 04:41:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/11882037/38004

[commit-bot: I haz the power, 2013-01-23 07:36:56]

Change committed as 178267