cc: Block commit on activate by setting a flag on LayerTreeHost.

Currently the ThreadProxy recursively asks all layers in the tree if
they should block the commit. This is problematic as when you remove
a layer from a the tree, it may want to block the commit to get back
resources from its active-tree impl-layer.

Instead, have layers call SetNextCommitWaitsForActivation() when they
want the next commit to block on activate. This way we only block
commits that matter, not every commit when there's a texture layer
present. And we can allow a layer to block the commit when it is
leaving the tree.

Tests:
TextureLayerNoMailboxIsActivatedDuringCommit
TextureLayerMailboxIsActivatedDuringCommit
DelegatedFrameIsActivatedDuringCommit

R=enne, piman
BUG=277953

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

Review URL: https://chromiumcodereview.appspot.com/23530003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@220515 0039d316-1c4b-4281-b951-d872f2087c98

18 files changed, 442 insertions, 65 deletions

diff --git a/cc/layers/delegated_renderer_layer.cc b/cc/layers/delegated_renderer_layer.cc
index 0ffa54a..1e1a8a0 100644
--- a/cc/layers/delegated_renderer_layer.cc
+++ b/cc/layers/delegated_renderer_layer.cc
@@ -29,6 +29,23 @@ scoped_ptr<LayerImpl> DelegatedRendererLayer::CreateLayerImpl(
       tree_impl, layer_id_).PassAs<LayerImpl>();
 }
 
+void DelegatedRendererLayer::SetLayerTreeHost(LayerTreeHost* host) {
+  if (layer_tree_host() == host) {
+    Layer::SetLayerTreeHost(host);
+    return;
+  }
+
+  if (!host) {
+    // The active frame needs to be removed from the active tree and resources
+    // returned before the commit is called complete.
+    // TODO(danakj): Don't need to do this if the last frame commited was empty
+    // or we never commited a frame with resources.
+    SetNextCommitWaitsForActivation();
+  }
+
+  Layer::SetLayerTreeHost(host);
+}
+
 bool DelegatedRendererLayer::DrawsContent() const {
   return Layer::DrawsContent() && !frame_size_.IsEmpty();
 }
@@ -83,6 +100,9 @@ void DelegatedRendererLayer::SetFrameData(
     frame_size_ = gfx::Size();
   }
   SetNeedsCommit();
+  // The active frame needs to be replaced and resources returned before the
+  // commit is called complete.
+  SetNextCommitWaitsForActivation();
 }
 
 void DelegatedRendererLayer::TakeUnusedResourcesForChildCompositor(
@@ -93,12 +113,4 @@ void DelegatedRendererLayer::TakeUnusedResourcesForChildCompositor(
   array->swap(unused_resources_for_child_compositor_);
 }
 
-bool DelegatedRendererLayer::BlocksPendingCommit() const {
-  // The active frame needs to be replaced and resources returned before the
-  // commit is called complete. This is true even whenever there may be
-  // resources to return, regardless of if the layer will draw in its new
-  // state.
-  return true;
-}
-
 }  // namespace cc
diff --git a/cc/layers/delegated_renderer_layer.h b/cc/layers/delegated_renderer_layer.h
index f16217b..afee475f 100644
--- a/cc/layers/delegated_renderer_layer.h
+++ b/cc/layers/delegated_renderer_layer.h
@@ -21,6 +21,7 @@ class CC_EXPORT DelegatedRendererLayer : public Layer {
 
   virtual scoped_ptr<LayerImpl> CreateLayerImpl(LayerTreeImpl* tree_impl)
       OVERRIDE;
+  virtual void SetLayerTreeHost(LayerTreeHost* host) OVERRIDE;
   virtual void PushPropertiesTo(LayerImpl* impl) OVERRIDE;
   virtual bool DrawsContent() const OVERRIDE;
 
@@ -36,8 +37,6 @@ class CC_EXPORT DelegatedRendererLayer : public Layer {
   // compositor to the given array, so they can be given back to the child.
   void TakeUnusedResourcesForChildCompositor(ReturnedResourceArray* array);
 
-  virtual bool BlocksPendingCommit() const OVERRIDE;
-
  protected:
   explicit DelegatedRendererLayer(DelegatedRendererLayerClient* client);
   virtual ~DelegatedRendererLayer();
diff --git a/cc/layers/layer.cc b/cc/layers/layer.cc
index 8cb61d9..e7cf755 100644
--- a/cc/layers/layer.cc
+++ b/cc/layers/layer.cc
@@ -152,14 +152,11 @@ void Layer::SetNeedsFullTreeSync() {
   layer_tree_host_->SetNeedsFullTreeSync();
 }
 
-bool Layer::IsPropertyChangeAllowed() const {
+void Layer::SetNextCommitWaitsForActivation() {
   if (!layer_tree_host_)
-    return true;
-
-  if (!layer_tree_host_->settings().strict_layer_property_change_checking)
-    return true;
+    return;
 
-  return !layer_tree_host_->in_paint_layer_contents();
+  layer_tree_host_->SetNextCommitWaitsForActivation();
 }
 
 void Layer::SetNeedsPushProperties() {
@@ -187,6 +184,16 @@ void Layer::RemoveDependentNeedsPushProperties() {
       parent_->RemoveDependentNeedsPushProperties();
 }
 
+bool Layer::IsPropertyChangeAllowed() const {
+  if (!layer_tree_host_)
+    return true;
+
+  if (!layer_tree_host_->settings().strict_layer_property_change_checking)
+    return true;
+
+  return !layer_tree_host_->in_paint_layer_contents();
+}
+
 gfx::Rect Layer::LayerRectToContentRect(const gfx::RectF& layer_rect) const {
   gfx::RectF content_rect =
       gfx::ScaleRect(layer_rect, contents_scale_x(), contents_scale_y());
@@ -196,10 +203,6 @@ gfx::Rect Layer::LayerRectToContentRect(const gfx::RectF& layer_rect) const {
   return gfx::ToEnclosingRect(content_rect);
 }
 
-bool Layer::BlocksPendingCommit() const {
-  return false;
-}
-
 skia::RefPtr<SkPicture> Layer::GetPicture() const {
   return skia::RefPtr<SkPicture>();
 }
@@ -208,20 +211,6 @@ bool Layer::CanClipSelf() const {
   return false;
 }
 
-bool Layer::BlocksPendingCommitRecursive() const {
-  if (BlocksPendingCommit())
-    return true;
-  if (mask_layer() && mask_layer()->BlocksPendingCommitRecursive())
-    return true;
-  if (replica_layer() && replica_layer()->BlocksPendingCommitRecursive())
-    return true;
-  for (size_t i = 0; i < children_.size(); ++i) {
-    if (children_[i]->BlocksPendingCommitRecursive())
-      return true;
-  }
-  return false;
-}
-
 void Layer::SetParent(Layer* layer) {
   DCHECK(!layer || !layer->HasAncestor(this));
 
diff --git a/cc/layers/layer.h b/cc/layers/layer.h
index 2f4d581..7a8f9d1 100644
--- a/cc/layers/layer.h
+++ b/cc/layers/layer.h
@@ -397,13 +397,6 @@ class CC_EXPORT Layer : public base::RefCounted<Layer>,
 
   gfx::Rect LayerRectToContentRect(const gfx::RectF& layer_rect) const;
 
-  // In impl-side painting, this returns true if this layer type is not
-  // compatible with the main thread running freely, such as a double-buffered
-  // canvas that doesn't want to be triple-buffered across all three trees.
-  virtual bool BlocksPendingCommit() const;
-  // Returns true if anything in this tree blocksPendingCommit.
-  bool BlocksPendingCommitRecursive() const;
-
   virtual skia::RefPtr<SkPicture> GetPicture() const;
 
   virtual bool CanClipSelf() const;
@@ -455,7 +448,11 @@ class CC_EXPORT Layer : public base::RefCounted<Layer>,
   // Called when there's been a change in layer structure.  Implies both
   // SetNeedsUpdate and SetNeedsCommit, but not SetNeedsPushProperties.
   void SetNeedsFullTreeSync();
-  bool IsPropertyChangeAllowed() const;
+
+  // Called when the next commit should wait until the pending tree is activated
+  // before finishing the commit and unblocking the main thread. Used to ensure
+  // unused resources on the impl thread are returned before commit completes.
+  void SetNextCommitWaitsForActivation();
 
   void SetNeedsPushProperties();
   void AddDependentNeedsPushProperties();
@@ -464,6 +461,8 @@ class CC_EXPORT Layer : public base::RefCounted<Layer>,
     return needs_push_properties() || descendant_needs_push_properties();
   }
 
+  bool IsPropertyChangeAllowed() const;
+
   // If this layer has a scroll parent, it removes |this| from its list of
   // scroll children.
   void RemoveFromScrollTree();
diff --git a/cc/layers/texture_layer.cc b/cc/layers/texture_layer.cc
index 4d6df14..d0ab7d9 100644
--- a/cc/layers/texture_layer.cc
+++ b/cc/layers/texture_layer.cc
@@ -125,6 +125,9 @@ void TextureLayer::SetTextureId(unsigned id) {
     layer_tree_host()->AcquireLayerTextures();
   texture_id_ = id;
   SetNeedsCommit();
+  // The texture id needs to be removed from the active tree before the
+  // commit is called complete.
+  SetNextCommitWaitsForActivation();
 }
 
 void TextureLayer::SetTextureMailbox(const TextureMailbox& mailbox) {
@@ -138,6 +141,9 @@ void TextureLayer::SetTextureMailbox(const TextureMailbox& mailbox) {
     holder_ref_.reset();
   needs_set_mailbox_ = true;
   SetNeedsCommit();
+  // The active frame needs to be replaced and the mailbox returned before the
+  // commit is called complete.
+  SetNextCommitWaitsForActivation();
 }
 
 void TextureLayer::WillModifyTexture() {
@@ -161,16 +167,24 @@ void TextureLayer::SetLayerTreeHost(LayerTreeHost* host) {
   }
 
   if (layer_tree_host()) {
-    if (texture_id_)
+    if (texture_id_) {
       layer_tree_host()->AcquireLayerTextures();
+      // The texture id needs to be removed from the active tree before the
+      // commit is called complete.
+      SetNextCommitWaitsForActivation();
+    }
     if (rate_limit_context_ && client_)
       layer_tree_host()->StopRateLimiter(client_->Context3d());
   }
   // If we're removed from the tree, the TextureLayerImpl will be destroyed, and
   // we will need to set the mailbox again on a new TextureLayerImpl the next
   // time we push.
-  if (!host && uses_mailbox_ && holder_ref_)
+  if (!host && uses_mailbox_ && holder_ref_) {
     needs_set_mailbox_ = true;
+    // The active frame needs to be replaced and the mailbox returned before the
+    // commit is called complete.
+    SetNextCommitWaitsForActivation();
+  }
   Layer::SetLayerTreeHost(host);
 }
 
@@ -196,6 +210,9 @@ bool TextureLayer::Update(ResourceUpdateQueue* queue,
           client_->Context3d()->getGraphicsResetStatusARB() != GL_NO_ERROR)
         texture_id_ = 0;
       updated = true;
+      // The texture id needs to be removed from the active tree before the
+      // commit is called complete.
+      SetNextCommitWaitsForActivation();
     }
   }
 
@@ -241,13 +258,6 @@ Region TextureLayer::VisibleContentOpaqueRegion() const {
   return Region();
 }
 
-bool TextureLayer::BlocksPendingCommit() const {
-  // Double-buffered texture layers need to be blocked until they can be made
-  // triple-buffered.  Single-buffered layers already prevent draws, so
-  // can block too for simplicity.
-  return DrawsContent();
-}
-
 bool TextureLayer::CanClipSelf() const {
   return true;
 }
diff --git a/cc/layers/texture_layer.h b/cc/layers/texture_layer.h
index f8c1c6a..cb34cf5 100644
--- a/cc/layers/texture_layer.h
+++ b/cc/layers/texture_layer.h
@@ -120,6 +120,7 @@ class CC_EXPORT TextureLayer : public Layer {
   void SetRateLimitContext(bool rate_limit);
 
   // Code path for plugins which supply their own texture ID.
+  // DEPRECATED. DO NOT USE.
   void SetTextureId(unsigned texture_id);
 
   // Code path for plugins which supply their own mailbox.
@@ -136,7 +137,6 @@ class CC_EXPORT TextureLayer : public Layer {
                       const OcclusionTracker* occlusion) OVERRIDE;
   virtual void PushPropertiesTo(LayerImpl* layer) OVERRIDE;
   virtual Region VisibleContentOpaqueRegion() const OVERRIDE;
-  virtual bool BlocksPendingCommit() const OVERRIDE;
 
   virtual bool CanClipSelf() const OVERRIDE;
 
diff --git a/cc/layers/texture_layer_unittest.cc b/cc/layers/texture_layer_unittest.cc
index 729f4a1..a3c568f 100644
--- a/cc/layers/texture_layer_unittest.cc
+++ b/cc/layers/texture_layer_unittest.cc
@@ -8,6 +8,8 @@
 
 #include "base/callback.h"
 #include "base/synchronization/waitable_event.h"
+#include "base/threading/thread.h"
+#include "base/time/time.h"
 #include "cc/debug/test_web_graphics_context_3d.h"
 #include "cc/layers/texture_layer_client.h"
 #include "cc/layers/texture_layer_impl.h"
@@ -861,6 +863,245 @@ class TextureLayerImplWithMailboxThreadedCallback : public LayerTreeTest {
 SINGLE_AND_MULTI_THREAD_DIRECT_RENDERER_TEST_F(
     TextureLayerImplWithMailboxThreadedCallback);
 
+
+class TextureLayerNoMailboxIsActivatedDuringCommit : public LayerTreeTest,
+                                                     public TextureLayerClient {
+ protected:
+  TextureLayerNoMailboxIsActivatedDuringCommit()
+      : wait_thread_("WAIT"),
+        wait_event_(false, false) {
+    wait_thread_.Start();
+  }
+
+  virtual void BeginTest() OVERRIDE {
+    activate_count_ = 0;
+
+    gfx::Size bounds(100, 100);
+    root_ = Layer::Create();
+    root_->SetAnchorPoint(gfx::PointF());
+    root_->SetBounds(bounds);
+
+    layer_ = TextureLayer::Create(this);
+    layer_->SetIsDrawable(true);
+    layer_->SetAnchorPoint(gfx::PointF());
+    layer_->SetBounds(bounds);
+
+    root_->AddChild(layer_);
+    layer_tree_host()->SetRootLayer(root_);
+    layer_tree_host()->SetViewportSize(bounds);
+
+    PostSetNeedsCommitToMainThread();
+  }
+
+  // TextureLayerClient implementation.
+  virtual unsigned PrepareTexture() OVERRIDE {
+    return OffscreenContextProviderForMainThread()
+        ->Context3d()->createTexture();
+  }
+  virtual WebKit::WebGraphicsContext3D* Context3d() OVERRIDE {
+    return OffscreenContextProviderForMainThread()->Context3d();
+  }
+  virtual bool PrepareTextureMailbox(TextureMailbox* mailbox,
+                                     bool use_shared_memory) OVERRIDE {
+    return false;
+  }
+
+  virtual void WillActivateTreeOnThread(LayerTreeHostImpl* impl) OVERRIDE {
+    // Slow down activation so the main thread DidCommit() will run if
+    // not blocked.
+    wait_thread_.message_loop()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&base::WaitableEvent::Signal,
+                   base::Unretained(&wait_event_)),
+        base::TimeDelta::FromMilliseconds(10));
+    wait_event_.Wait();
+
+    base::AutoLock lock(activate_lock_);
+    ++activate_count_;
+  }
+
+  virtual void DidActivateTreeOnThread(LayerTreeHostImpl* impl) OVERRIDE {
+    // The main thread is awake now, and will run DidCommit() immediately.
+    // Run DidActivate() afterwards by posting it now.
+    proxy()->MainThreadTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(&TextureLayerNoMailboxIsActivatedDuringCommit::DidActivate,
+                   base::Unretained(this)));
+  }
+
+  void DidActivate() {
+    base::AutoLock lock(activate_lock_);
+    switch (activate_count_) {
+      case 1:
+        // The first texture has been activated. Invalidate the layer so it
+        // grabs a new texture id from the client.
+        layer_->SetNeedsDisplay();
+        // So this commit number should complete after the second activate.
+        EXPECT_EQ(1, layer_tree_host()->source_frame_number());
+        break;
+      case 2:
+        // The second mailbox has been activated. Remove the layer from
+        // the tree to cause another commit/activation. The commit should
+        // finish *after* the layer is removed from the active tree.
+        layer_->RemoveFromParent();
+        // So this commit number should complete after the third activate.
+        EXPECT_EQ(2, layer_tree_host()->source_frame_number());
+        break;
+      case 3:
+        EndTest();
+        break;
+    }
+  }
+
+  virtual void DidCommit() OVERRIDE {
+    switch (layer_tree_host()->source_frame_number()) {
+      case 2: {
+        // The activate for the 2nd texture should have happened before now.
+        base::AutoLock lock(activate_lock_);
+        EXPECT_EQ(2, activate_count_);
+        break;
+      }
+      case 3: {
+        // The activate to remove the layer should have happened before now.
+        base::AutoLock lock(activate_lock_);
+        EXPECT_EQ(3, activate_count_);
+        break;
+      }
+    }
+  }
+
+
+  virtual void AfterTest() OVERRIDE {}
+
+  base::Thread wait_thread_;
+  base::WaitableEvent wait_event_;
+  base::Lock activate_lock_;
+  int activate_count_;
+  int activate_commit_;
+  scoped_refptr<Layer> root_;
+  scoped_refptr<TextureLayer> layer_;
+};
+
+SINGLE_AND_MULTI_THREAD_DIRECT_RENDERER_TEST_F(
+    TextureLayerNoMailboxIsActivatedDuringCommit);
+
+class TextureLayerMailboxIsActivatedDuringCommit : public LayerTreeTest {
+ protected:
+  TextureLayerMailboxIsActivatedDuringCommit()
+      : wait_thread_("WAIT"),
+        wait_event_(false, false) {
+    wait_thread_.Start();
+  }
+
+  static void ReleaseCallback(unsigned sync_point, bool lost_resource) {}
+
+  void SetMailbox(char mailbox_char) {
+    TextureMailbox mailbox(
+        std::string(64, mailbox_char),
+        base::Bind(
+            &TextureLayerMailboxIsActivatedDuringCommit::ReleaseCallback));
+    layer_->SetTextureMailbox(mailbox);
+  }
+
+  virtual void BeginTest() OVERRIDE {
+    activate_count_ = 0;
+
+    gfx::Size bounds(100, 100);
+    root_ = Layer::Create();
+    root_->SetAnchorPoint(gfx::PointF());
+    root_->SetBounds(bounds);
+
+    layer_ = TextureLayer::CreateForMailbox(NULL);
+    layer_->SetIsDrawable(true);
+    layer_->SetAnchorPoint(gfx::PointF());
+    layer_->SetBounds(bounds);
+
+    root_->AddChild(layer_);
+    layer_tree_host()->SetRootLayer(root_);
+    layer_tree_host()->SetViewportSize(bounds);
+    SetMailbox('1');
+
+    PostSetNeedsCommitToMainThread();
+  }
+
+  virtual void WillActivateTreeOnThread(LayerTreeHostImpl* impl) OVERRIDE {
+    // Slow down activation so the main thread DidCommit() will run if
+    // not blocked.
+    wait_thread_.message_loop()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&base::WaitableEvent::Signal,
+                   base::Unretained(&wait_event_)),
+        base::TimeDelta::FromMilliseconds(10));
+    wait_event_.Wait();
+
+    base::AutoLock lock(activate_lock_);
+    ++activate_count_;
+  }
+
+  virtual void DidActivateTreeOnThread(LayerTreeHostImpl* impl) OVERRIDE {
+    // The main thread is awake now, and will run DidCommit() immediately.
+    // Run DidActivate() afterwards by posting it now.
+    proxy()->MainThreadTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(&TextureLayerMailboxIsActivatedDuringCommit::DidActivate,
+                   base::Unretained(this)));
+  }
+
+  void DidActivate() {
+    base::AutoLock lock(activate_lock_);
+    switch (activate_count_) {
+      case 1:
+        // The first mailbox has been activated. Set a new mailbox, and
+        // expect the next commit to finish *after* it is activated.
+        SetMailbox('2');
+        // So this commit number should complete after the second activate.
+        EXPECT_EQ(1, layer_tree_host()->source_frame_number());
+        break;
+      case 2:
+        // The second mailbox has been activated. Remove the layer from
+        // the tree to cause another commit/activation. The commit should
+        // finish *after* the layer is removed from the active tree.
+        layer_->RemoveFromParent();
+        // So this commit number should complete after the third activate.
+        EXPECT_EQ(2, layer_tree_host()->source_frame_number());
+        break;
+      case 3:
+        EndTest();
+        break;
+    }
+  }
+
+  virtual void DidCommit() OVERRIDE {
+    switch (layer_tree_host()->source_frame_number()) {
+      case 2: {
+        // The activate for the 2nd mailbox should have happened before now.
+        base::AutoLock lock(activate_lock_);
+        EXPECT_EQ(2, activate_count_);
+        break;
+      }
+      case 3: {
+        // The activate to remove the layer should have happened before now.
+        base::AutoLock lock(activate_lock_);
+        EXPECT_EQ(3, activate_count_);
+        break;
+      }
+    }
+  }
+
+
+  virtual void AfterTest() OVERRIDE {}
+
+  base::Thread wait_thread_;
+  base::WaitableEvent wait_event_;
+  base::Lock activate_lock_;
+  int activate_count_;
+  scoped_refptr<Layer> root_;
+  scoped_refptr<TextureLayer> layer_;
+};
+
+SINGLE_AND_MULTI_THREAD_DIRECT_RENDERER_TEST_F(
+    TextureLayerMailboxIsActivatedDuringCommit);
+
 class TextureLayerImplWithMailboxTest : public TextureLayerTest {
  protected:
   TextureLayerImplWithMailboxTest()
diff --git a/cc/layers/tiled_layer.cc b/cc/layers/tiled_layer.cc
index 556b8231..2d812e8 100644
--- a/cc/layers/tiled_layer.cc
+++ b/cc/layers/tiled_layer.cc
@@ -235,8 +235,6 @@ void TiledLayer::PushPropertiesTo(LayerImpl* layer) {
   needs_push_properties_ = true;
 }
 
-bool TiledLayer::BlocksPendingCommit() const { return true; }
-
 PrioritizedResourceManager* TiledLayer::ResourceManager() {
   if (!layer_tree_host())
     return NULL;
@@ -731,6 +729,10 @@ bool TiledLayer::Update(ResourceUpdateQueue* queue,
                         const OcclusionTracker* occlusion) {
   DCHECK(!skips_draw_ && !failed_update_);  // Did ResetUpdateState get skipped?
 
+  // Tiled layer always causes commits to wait for activation, as it does
+  // not support pending trees.
+  SetNextCommitWaitsForActivation();
+
   bool updated = false;
 
   {
diff --git a/cc/layers/tiled_layer.h b/cc/layers/tiled_layer.h
index 56e91f8..51afed2 100644
--- a/cc/layers/tiled_layer.h
+++ b/cc/layers/tiled_layer.h
@@ -26,7 +26,6 @@ class CC_EXPORT TiledLayer : public ContentsScalingLayer {
   // Layer implementation.
   virtual void SetIsMask(bool is_mask) OVERRIDE;
   virtual void PushPropertiesTo(LayerImpl* layer) OVERRIDE;
-  virtual bool BlocksPendingCommit() const OVERRIDE;
   virtual bool DrawsContent() const OVERRIDE;
   virtual void ReduceMemoryUsage() OVERRIDE;
   virtual void SetNeedsDisplayRect(const gfx::RectF& dirty_rect) OVERRIDE;
diff --git a/cc/test/fake_proxy.h b/cc/test/fake_proxy.h
index b3bca69..3127cb4 100644
--- a/cc/test/fake_proxy.h
+++ b/cc/test/fake_proxy.h
@@ -32,6 +32,7 @@ class FakeProxy : public Proxy {
   virtual void SetNeedsUpdateLayers() OVERRIDE {}
   virtual void SetNeedsCommit() OVERRIDE {}
   virtual void SetNeedsRedraw(gfx::Rect damage_rect) OVERRIDE {}
+  virtual void SetNextCommitWaitsForActivation() OVERRIDE {}
   virtual void NotifyInputThrottledUntilCommit() OVERRIDE {}
   virtual void SetDeferCommits(bool defer_commits) OVERRIDE {}
   virtual void MainThreadHasStoppedFlinging() OVERRIDE {}
diff --git a/cc/trees/layer_tree_host.cc b/cc/trees/layer_tree_host.cc
index f18e3b8..27659e0 100644
--- a/cc/trees/layer_tree_host.cc
+++ b/cc/trees/layer_tree_host.cc
@@ -518,6 +518,10 @@ bool LayerTreeHost::CommitRequested() const {
   return proxy_->CommitRequested();
 }
 
+void LayerTreeHost::SetNextCommitWaitsForActivation() {
+  proxy_->SetNextCommitWaitsForActivation();
+}
+
 void LayerTreeHost::SetAnimationEvents(scoped_ptr<AnimationEventsVector> events,
                                        base::Time wall_clock_time) {
   DCHECK(proxy_->IsMainThread());
@@ -1089,12 +1093,6 @@ void LayerTreeHost::UpdateTopControlsState(TopControlsState constraints,
                  animate));
 }
 
-bool LayerTreeHost::BlocksPendingCommit() const {
-  if (!root_layer_.get())
-    return false;
-  return root_layer_->BlocksPendingCommitRecursive();
-}
-
 scoped_ptr<base::Value> LayerTreeHost::AsValue() const {
   scoped_ptr<base::DictionaryValue> state(new base::DictionaryValue());
   state->Set("proxy", proxy_->AsValue().release());
diff --git a/cc/trees/layer_tree_host.h b/cc/trees/layer_tree_host.h
index 619c18e..be0ab06 100644
--- a/cc/trees/layer_tree_host.h
+++ b/cc/trees/layer_tree_host.h
@@ -191,6 +191,8 @@ class CC_EXPORT LayerTreeHost : NON_EXPORTED_BASE(public RateLimiterClient) {
   void SetNeedsRedrawRect(gfx::Rect damage_rect);
   bool CommitRequested() const;
 
+  void SetNextCommitWaitsForActivation();
+
   void SetAnimationEvents(scoped_ptr<AnimationEventsVector> events,
                           base::Time wall_clock_time);
 
@@ -266,8 +268,6 @@ class CC_EXPORT LayerTreeHost : NON_EXPORTED_BASE(public RateLimiterClient) {
     return animation_registrar_.get();
   }
 
-  bool BlocksPendingCommit() const;
-
   // Obtains a thorough dump of the LayerTreeHost as a value.
   scoped_ptr<base::Value> AsValue() const;
 
diff --git a/cc/trees/layer_tree_host_unittest_delegated.cc b/cc/trees/layer_tree_host_unittest_delegated.cc
index 721613a..3fd040bc 100644
--- a/cc/trees/layer_tree_host_unittest_delegated.cc
+++ b/cc/trees/layer_tree_host_unittest_delegated.cc
@@ -7,6 +7,9 @@
 #include <algorithm>
 
 #include "base/bind.h"
+#include "base/synchronization/waitable_event.h"
+#include "base/threading/thread.h"
+#include "base/time/time.h"
 #include "cc/layers/delegated_renderer_layer.h"
 #include "cc/layers/delegated_renderer_layer_client.h"
 #include "cc/layers/delegated_renderer_layer_impl.h"
@@ -1582,5 +1585,107 @@ class LayerTreeHostDelegatedTestCommitWithoutTake
 
 SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostDelegatedTestCommitWithoutTake);
 
+class DelegatedFrameIsActivatedDuringCommit
+    : public LayerTreeHostDelegatedTestCaseSingleDelegatedLayer {
+ protected:
+  DelegatedFrameIsActivatedDuringCommit()
+      : wait_thread_("WAIT"),
+        wait_event_(false, false) {
+    wait_thread_.Start();
+  }
+
+  virtual void BeginTest() OVERRIDE {
+    activate_count_ = 0;
+
+    scoped_ptr<DelegatedFrameData> frame =
+        CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
+    AddTextureQuad(frame.get(), 999);
+    AddTransferableResource(frame.get(), 999);
+    delegated_->SetFrameData(frame.Pass());
+
+    PostSetNeedsCommitToMainThread();
+  }
+
+  virtual void WillActivateTreeOnThread(LayerTreeHostImpl* impl) OVERRIDE {
+    // Slow down activation so the main thread DidCommit() will run if
+    // not blocked.
+    wait_thread_.message_loop()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&base::WaitableEvent::Signal,
+                   base::Unretained(&wait_event_)),
+        base::TimeDelta::FromMilliseconds(10));
+    wait_event_.Wait();
+
+    base::AutoLock lock(activate_lock_);
+    ++activate_count_;
+  }
+
+  virtual void DidActivateTreeOnThread(LayerTreeHostImpl* impl) OVERRIDE {
+    // The main thread is awake now, and will run DidCommit() immediately.
+    // Run DidActivate() afterwards by posting it now.
+    proxy()->MainThreadTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(&DelegatedFrameIsActivatedDuringCommit::DidActivate,
+                   base::Unretained(this)));
+  }
+
+  void DidActivate() {
+    base::AutoLock lock(activate_lock_);
+    switch (activate_count_) {
+      case 1: {
+        // The first frame has been activated. Set a new frame, and
+        // expect the next commit to finish *after* it is activated.
+        scoped_ptr<DelegatedFrameData> frame =
+            CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
+        AddTextureQuad(frame.get(), 555);
+        AddTransferableResource(frame.get(), 555);
+        delegated_->SetFrameData(frame.Pass());
+        // So this commit number should complete after the second activate.
+        EXPECT_EQ(1, layer_tree_host()->source_frame_number());
+        break;
+      }
+      case 2:
+        // The second frame has been activated. Remove the layer from
+        // the tree to cause another commit/activation. The commit should
+        // finish *after* the layer is removed from the active tree.
+        delegated_->RemoveFromParent();
+        // So this commit number should complete after the third activate.
+        EXPECT_EQ(2, layer_tree_host()->source_frame_number());
+        break;
+      case 3:
+        EndTest();
+        break;
+    }
+  }
+
+  virtual void DidCommit() OVERRIDE {
+    switch (layer_tree_host()->source_frame_number()) {
+      case 2: {
+        // The activate for the 2nd frame should have happened before now.
+        base::AutoLock lock(activate_lock_);
+        EXPECT_EQ(2, activate_count_);
+        break;
+      }
+      case 3: {
+        // The activate to remove the layer should have happened before now.
+        base::AutoLock lock(activate_lock_);
+        EXPECT_EQ(3, activate_count_);
+        break;
+      }
+    }
+  }
+
+
+  virtual void AfterTest() OVERRIDE {}
+
+  base::Thread wait_thread_;
+  base::WaitableEvent wait_event_;
+  base::Lock activate_lock_;
+  int activate_count_;
+};
+
+SINGLE_AND_MULTI_THREAD_TEST_F(
+    DelegatedFrameIsActivatedDuringCommit);
+
 }  // namespace
 }  // namespace cc
diff --git a/cc/trees/proxy.h b/cc/trees/proxy.h
index 9845b0f..84844c8 100644
--- a/cc/trees/proxy.h
+++ b/cc/trees/proxy.h
@@ -69,6 +69,7 @@ class CC_EXPORT Proxy {
   virtual void SetNeedsUpdateLayers() = 0;
   virtual void SetNeedsCommit() = 0;
   virtual void SetNeedsRedraw(gfx::Rect damage_rect) = 0;
+  virtual void SetNextCommitWaitsForActivation() = 0;
 
   virtual void NotifyInputThrottledUntilCommit() = 0;
 
diff --git a/cc/trees/single_thread_proxy.cc b/cc/trees/single_thread_proxy.cc
index 516fa69..7390a32 100644
--- a/cc/trees/single_thread_proxy.cc
+++ b/cc/trees/single_thread_proxy.cc
@@ -244,6 +244,10 @@ void SingleThreadProxy::SetNeedsRedraw(gfx::Rect damage_rect) {
   SetNeedsRedrawRectOnImplThread(damage_rect);
 }
 
+void SingleThreadProxy::SetNextCommitWaitsForActivation() {
+  // There is no activation here other than commit. So do nothing.
+}
+
 void SingleThreadProxy::SetDeferCommits(bool defer_commits) {
   // Thread-only feature.
   NOTREACHED();
diff --git a/cc/trees/single_thread_proxy.h b/cc/trees/single_thread_proxy.h
index 8ad0c2f..ee92173 100644
--- a/cc/trees/single_thread_proxy.h
+++ b/cc/trees/single_thread_proxy.h
@@ -35,6 +35,7 @@ class SingleThreadProxy : public Proxy, LayerTreeHostImplClient {
   virtual void SetNeedsUpdateLayers() OVERRIDE;
   virtual void SetNeedsCommit() OVERRIDE;
   virtual void SetNeedsRedraw(gfx::Rect damage_rect) OVERRIDE;
+  virtual void SetNextCommitWaitsForActivation() OVERRIDE;
   virtual void NotifyInputThrottledUntilCommit() OVERRIDE {}
   virtual void SetDeferCommits(bool defer_commits) OVERRIDE;
   virtual bool CommitRequested() const OVERRIDE;
diff --git a/cc/trees/thread_proxy.cc b/cc/trees/thread_proxy.cc
index dc73f3b..dda25ec 100644
--- a/cc/trees/thread_proxy.cc
+++ b/cc/trees/thread_proxy.cc
@@ -76,6 +76,8 @@ ThreadProxy::ThreadProxy(
       textures_acquired_(true),
       in_composite_and_readback_(false),
       manage_tiles_pending_(false),
+      commit_waits_for_activation_(false),
+      inside_commit_(false),
       weak_factory_on_impl_thread_(this),
       weak_factory_(this),
       begin_frame_sent_to_main_thread_completion_event_on_impl_thread_(NULL),
@@ -506,6 +508,12 @@ void ThreadProxy::SetNeedsRedraw(gfx::Rect damage_rect) {
                  damage_rect));
 }
 
+void ThreadProxy::SetNextCommitWaitsForActivation() {
+  DCHECK(IsMainThread());
+  DCHECK(!inside_commit_);
+  commit_waits_for_activation_ = true;
+}
+
 void ThreadProxy::SetDeferCommits(bool defer_commits) {
   DCHECK(IsMainThread());
   DCHECK_NE(defer_commits_, defer_commits);
@@ -933,10 +941,12 @@ void ThreadProxy::ScheduledActionCommit() {
   current_resource_update_controller_on_impl_thread_->Finalize();
   current_resource_update_controller_on_impl_thread_.reset();
 
+  inside_commit_ = true;
   layer_tree_host_impl_->BeginCommit();
   layer_tree_host_->BeginCommitOnImplThread(layer_tree_host_impl_.get());
   layer_tree_host_->FinishCommitOnImplThread(layer_tree_host_impl_.get());
   layer_tree_host_impl_->CommitComplete();
+  inside_commit_ = false;
 
   SetInputThrottledUntilCommitOnImplThread(false);
 
@@ -945,8 +955,7 @@ void ThreadProxy::ScheduledActionCommit() {
   next_frame_is_newly_committed_frame_on_impl_thread_ = true;
 
   if (layer_tree_host_->settings().impl_side_painting &&
-      layer_tree_host_->BlocksPendingCommit() &&
-      layer_tree_host_impl_->pending_tree()) {
+      commit_waits_for_activation_) {
     // For some layer types in impl-side painting, the commit is held until
     // the pending tree is activated.  It's also possible that the
     // pending tree has already activated if there was no work to be done.
@@ -959,6 +968,8 @@ void ThreadProxy::ScheduledActionCommit() {
     commit_completion_event_on_impl_thread_ = NULL;
   }
 
+  commit_waits_for_activation_ = false;
+
   commit_complete_time_ = base::TimeTicks::HighResNow();
   begin_frame_to_commit_duration_history_.InsertSample(
       commit_complete_time_ - begin_frame_sent_to_main_thread_time_);
diff --git a/cc/trees/thread_proxy.h b/cc/trees/thread_proxy.h
index d159eb5..8122bf4 100644
--- a/cc/trees/thread_proxy.h
+++ b/cc/trees/thread_proxy.h
@@ -52,6 +52,7 @@ class ThreadProxy : public Proxy,
   virtual void SetNeedsUpdateLayers() OVERRIDE;
   virtual void SetNeedsCommit() OVERRIDE;
   virtual void SetNeedsRedraw(gfx::Rect damage_rect) OVERRIDE;
+  virtual void SetNextCommitWaitsForActivation() OVERRIDE;
   virtual void NotifyInputThrottledUntilCommit() OVERRIDE;
   virtual void SetDeferCommits(bool defer_commits) OVERRIDE;
   virtual bool CommitRequested() const OVERRIDE;
@@ -204,6 +205,10 @@ class ThreadProxy : public Proxy,
   // anything else.
   scoped_ptr<OutputSurface> first_output_surface_;
 
+  // Accessed on the main thread, or when main thread is blocked.
+  bool commit_waits_for_activation_;
+  bool inside_commit_;
+
   base::WeakPtrFactory<ThreadProxy> weak_factory_on_impl_thread_;
 
   base::WeakPtr<ThreadProxy> main_thread_weak_ptr_;