cc: Release main thread earlier

Previously, the main thread would wait for the compositor thread to do a
good bit of work during its CommitComplete.  This work doesn't involve
the main thread and so there is no reason to not release it earlier.

However, releasing earlier creates raciness between
LayerTreeHost::CommitComplete and LayerTreeHostImpl::CommitComplete that
did not exist before.  Several failing tests were updated to try to take
this into account.

BUG=333013

Review URL: https://codereview.chromium.org/139053002

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

8 files changed, 97 insertions, 185 deletions

diff --git a/cc/layers/texture_layer_unittest.cc b/cc/layers/texture_layer_unittest.cc
index 5261704..feaa585 100644
--- a/cc/layers/texture_layer_unittest.cc
+++ b/cc/layers/texture_layer_unittest.cc
@@ -886,15 +886,9 @@ class TextureLayerNoMailboxIsActivatedDuringCommit : public LayerTreeTest,
                                                      public TextureLayerClient {
  protected:
   TextureLayerNoMailboxIsActivatedDuringCommit()
-      : wait_thread_("WAIT"),
-        wait_event_(false, false),
-        texture_(0u) {
-    wait_thread_.Start();
-  }
+      : texture_(0u), activate_count_(0) {}
 
   virtual void BeginTest() OVERRIDE {
-    activate_count_ = 0;
-
     gfx::Size bounds(100, 100);
     root_ = Layer::Create();
     root_->SetAnchorPoint(gfx::PointF());
@@ -931,45 +925,21 @@ class TextureLayerNoMailboxIsActivatedDuringCommit : public LayerTreeTest,
   }
 
   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_) {
+  virtual void DidCommit() OVERRIDE {
+    switch (layer_tree_host()->source_frame_number()) {
       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();
@@ -977,29 +947,23 @@ class TextureLayerNoMailboxIsActivatedDuringCommit : public LayerTreeTest,
     }
   }
 
-  virtual void DidCommit() OVERRIDE {
-    switch (layer_tree_host()->source_frame_number()) {
+  virtual void CommitCompleteOnThread(LayerTreeHostImpl* host_impl) OVERRIDE {
+    switch (host_impl->active_tree()->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_;
   unsigned texture_;
   int activate_count_;
   scoped_refptr<Layer> root_;
@@ -1011,11 +975,7 @@ SINGLE_AND_MULTI_THREAD_DIRECT_RENDERER_TEST_F(
 
 class TextureLayerMailboxIsActivatedDuringCommit : public LayerTreeTest {
  protected:
-  TextureLayerMailboxIsActivatedDuringCommit()
-      : wait_thread_("WAIT"),
-        wait_event_(false, false) {
-    wait_thread_.Start();
-  }
+  TextureLayerMailboxIsActivatedDuringCommit() : activate_count_(0) {}
 
   static void ReleaseCallback(unsigned sync_point, bool lost_resource) {}
 
@@ -1028,8 +988,6 @@ class TextureLayerMailboxIsActivatedDuringCommit : public LayerTreeTest {
   }
 
   virtual void BeginTest() OVERRIDE {
-    activate_count_ = 0;
-
     gfx::Size bounds(100, 100);
     root_ = Layer::Create();
     root_->SetAnchorPoint(gfx::PointF());
@@ -1049,45 +1007,21 @@ class TextureLayerMailboxIsActivatedDuringCommit : public LayerTreeTest {
   }
 
   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_) {
+  virtual void DidCommit() OVERRIDE {
+    switch (layer_tree_host()->source_frame_number()) {
       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();
@@ -1095,17 +1029,15 @@ class TextureLayerMailboxIsActivatedDuringCommit : public LayerTreeTest {
     }
   }
 
-  virtual void DidCommit() OVERRIDE {
-    switch (layer_tree_host()->source_frame_number()) {
+  virtual void CommitCompleteOnThread(LayerTreeHostImpl* host_impl) OVERRIDE {
+    switch (host_impl->active_tree()->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;
       }
@@ -1115,9 +1047,6 @@ class TextureLayerMailboxIsActivatedDuringCommit : public LayerTreeTest {
 
   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_;
diff --git a/cc/test/layer_tree_test.cc b/cc/test/layer_tree_test.cc
index 36621d0..373f92b 100644
--- a/cc/test/layer_tree_test.cc
+++ b/cc/test/layer_tree_test.cc
@@ -23,6 +23,7 @@
 #include "cc/trees/layer_tree_host_client.h"
 #include "cc/trees/layer_tree_host_impl.h"
 #include "cc/trees/layer_tree_host_single_thread_client.h"
+#include "cc/trees/layer_tree_impl.h"
 #include "cc/trees/single_thread_proxy.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "ui/gfx/frame_time.h"
@@ -700,4 +701,14 @@ TestWebGraphicsContext3D* LayerTreeTest::TestContext() {
       output_surface_->context_provider().get())->TestContext3d();
 }
 
+int LayerTreeTest::LastCommittedSourceFrameNumber(LayerTreeHostImpl* impl)
+    const {
+  if (impl->pending_tree())
+    return impl->pending_tree()->source_frame_number();
+  if (impl->active_tree())
+    return impl->active_tree()->source_frame_number();
+  // Source frames start at 0, so this is invalid.
+  return -1;
+}
+
 }  // namespace cc
diff --git a/cc/test/layer_tree_test.h b/cc/test/layer_tree_test.h
index 6d282e9..5646d7a 100644
--- a/cc/test/layer_tree_test.h
+++ b/cc/test/layer_tree_test.h
@@ -181,6 +181,7 @@ class LayerTreeTest : public testing::Test, public TestHooks {
   LayerTreeHost* layer_tree_host() { return layer_tree_host_.get(); }
   bool delegating_renderer() const { return delegating_renderer_; }
   FakeOutputSurface* output_surface() { return output_surface_; }
+  int LastCommittedSourceFrameNumber(LayerTreeHostImpl* impl) const;
 
   virtual scoped_ptr<OutputSurface> CreateOutputSurface(bool fallback) OVERRIDE;
   virtual scoped_refptr<ContextProvider> OffscreenContextProvider() OVERRIDE;
diff --git a/cc/trees/layer_tree_host_unittest.cc b/cc/trees/layer_tree_host_unittest.cc
index b587667..5c8d624 100644
--- a/cc/trees/layer_tree_host_unittest.cc
+++ b/cc/trees/layer_tree_host_unittest.cc
@@ -815,7 +815,7 @@ SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestUndrawnLayersDamageLater);
 // If the layerTreeHost says it can't draw, Then we should not try to draw.
 class LayerTreeHostTestCanDrawBlocksDrawing : public LayerTreeHostTest {
  public:
-  LayerTreeHostTestCanDrawBlocksDrawing() : num_commits_(0), done_(false) {}
+  LayerTreeHostTestCanDrawBlocksDrawing() : done_(false) {}
 
   virtual void BeginTest() OVERRIDE { PostSetNeedsCommitToMainThread(); }
 
@@ -830,32 +830,35 @@ class LayerTreeHostTestCanDrawBlocksDrawing : public LayerTreeHostTest {
   virtual void CommitCompleteOnThread(LayerTreeHostImpl* impl) OVERRIDE {
     if (done_)
       return;
-    if (num_commits_ >= 1) {
+    if (LastCommittedSourceFrameNumber(impl) >= 1) {
       // After the first commit, we should not be able to draw.
       EXPECT_FALSE(impl->CanDraw());
     }
   }
 
   virtual void DidCommit() OVERRIDE {
-    num_commits_++;
-    if (num_commits_ == 1) {
-      // Make the viewport empty so the host says it can't draw.
-      layer_tree_host()->SetViewportSize(gfx::Size(0, 0));
-    } else if (num_commits_ == 2) {
-      char pixels[4];
-      layer_tree_host()->CompositeAndReadback(&pixels, gfx::Rect(0, 0, 1, 1));
-    } else if (num_commits_ == 3) {
-      // Let it draw so we go idle and end the test.
-      layer_tree_host()->SetViewportSize(gfx::Size(1, 1));
-      done_ = true;
-      EndTest();
+    switch (layer_tree_host()->source_frame_number()) {
+      case 1:
+        // Make the viewport empty so the host says it can't draw.
+        layer_tree_host()->SetViewportSize(gfx::Size(0, 0));
+        break;
+      case 2: {
+        char pixels[4];
+        layer_tree_host()->CompositeAndReadback(&pixels, gfx::Rect(0, 0, 1, 1));
+        break;
+      }
+      case 3:
+        // Let it draw so we go idle and end the test.
+        layer_tree_host()->SetViewportSize(gfx::Size(1, 1));
+        done_ = true;
+        EndTest();
+        break;
     }
   }
 
   virtual void AfterTest() OVERRIDE {}
 
  private:
-  int num_commits_;
   bool done_;
 };
 
@@ -3087,7 +3090,7 @@ MULTI_THREAD_TEST_F(LayerTreeHostTestDeferredInitialize);
 // Test for UI Resource management.
 class LayerTreeHostTestUIResource : public LayerTreeHostTest {
  public:
-  LayerTreeHostTestUIResource() : num_ui_resources_(0), num_commits_(0) {}
+  LayerTreeHostTestUIResource() : num_ui_resources_(0) {}
 
   virtual void InitializeSettings(LayerTreeSettings* settings) OVERRIDE {
     settings->texture_id_allocation_chunk_size = 1;
@@ -3096,7 +3099,7 @@ class LayerTreeHostTestUIResource : public LayerTreeHostTest {
   virtual void BeginTest() OVERRIDE { PostSetNeedsCommitToMainThread(); }
 
   virtual void DidCommit() OVERRIDE {
-    int frame = num_commits_;
+    int frame = layer_tree_host()->source_frame_number();
     switch (frame) {
       case 1:
         CreateResource();
@@ -3130,25 +3133,25 @@ class LayerTreeHostTestUIResource : public LayerTreeHostTest {
   void PerformTest(LayerTreeHostImpl* impl) {
     TestWebGraphicsContext3D* context = TestContext();
 
-    int frame = num_commits_;
+    int frame = impl->active_tree()->source_frame_number();
     switch (frame) {
-      case 1:
+      case 0:
         ASSERT_EQ(0u, context->NumTextures());
         break;
-      case 2:
+      case 1:
         // Created two textures.
         ASSERT_EQ(2u, context->NumTextures());
         break;
-      case 3:
+      case 2:
         // One texture left after one deletion.
         ASSERT_EQ(1u, context->NumTextures());
         break;
-      case 4:
+      case 3:
         // Resource manager state should not change when delete is called on an
         // invalid id.
         ASSERT_EQ(1u, context->NumTextures());
         break;
-      case 5:
+      case 4:
         // Creation after deletion: two more creates should total up to
         // three textures.
         ASSERT_EQ(3u, context->NumTextures());
@@ -3157,7 +3160,6 @@ class LayerTreeHostTestUIResource : public LayerTreeHostTest {
   }
 
   virtual void CommitCompleteOnThread(LayerTreeHostImpl* impl) OVERRIDE {
-    ++num_commits_;
     if (!layer_tree_host()->settings().impl_side_painting)
       PerformTest(impl);
   }
@@ -3183,7 +3185,6 @@ class LayerTreeHostTestUIResource : public LayerTreeHostTest {
 
   scoped_ptr<FakeScopedUIResource> ui_resources_[5];
   int num_ui_resources_;
-  int num_commits_;
 };
 
 MULTI_THREAD_TEST_F(LayerTreeHostTestUIResource);
@@ -4498,10 +4499,14 @@ class LayerTreeHostTestMemoryLimits : public LayerTreeHostTest {
 
   virtual void BeginTest() OVERRIDE { PostSetNeedsCommitToMainThread(); }
 
+  virtual void WillCommit() OVERRIDE {
+    // Some commits are aborted, so increment number of attempted commits here.
+    num_commits_++;
+  }
+
   virtual void DidCommit() OVERRIDE {
-    int frame = num_commits_;
-    switch (frame) {
-      case 0:
+    switch (num_commits_) {
+      case 1:
         // Verify default values.
         EXPECT_EQ(PrioritizedResourceManager::DefaultMemoryAllocationLimit(),
                   layer_tree_host()
@@ -4513,7 +4518,7 @@ class LayerTreeHostTestMemoryLimits : public LayerTreeHostTest {
                       ->ExternalPriorityCutoff());
         PostSetNeedsCommitToMainThread();
         break;
-      case 1:
+      case 2:
         // The values should remain the same until the commit after the policy
         // is changed.
         EXPECT_EQ(PrioritizedResourceManager::DefaultMemoryAllocationLimit(),
@@ -4525,7 +4530,7 @@ class LayerTreeHostTestMemoryLimits : public LayerTreeHostTest {
                       ->contents_texture_manager()
                       ->ExternalPriorityCutoff());
         break;
-      case 2:
+      case 3:
         // Verify values were correctly passed.
         EXPECT_EQ(16u * 1024u * 1024u,
                   layer_tree_host()
@@ -4537,28 +4542,25 @@ class LayerTreeHostTestMemoryLimits : public LayerTreeHostTest {
                       ->ExternalPriorityCutoff());
         EndTest();
         break;
-      case 3:
+      case 4:
         // Make sure no extra commits happen.
         NOTREACHED();
         break;
     }
-
-    ++num_commits_;
   }
 
   virtual void CommitCompleteOnThread(LayerTreeHostImpl* impl) OVERRIDE {
-    int frame = num_commits_;
-    switch (frame) {
-      case 0:
-        break;
+    switch (num_commits_) {
       case 1:
+        break;
+      case 2:
         // This will trigger a commit because the priority cutoff has changed.
         impl->SetMemoryPolicy(ManagedMemoryPolicy(
             16u * 1024u * 1024u,
             gpu::MemoryAllocation::CUTOFF_ALLOW_NICE_TO_HAVE,
             1000));
         break;
-      case 2:
+      case 3:
         // This will not trigger a commit because the priority cutoff has not
         // changed, and there is already enough memory for all allocations.
         impl->SetMemoryPolicy(ManagedMemoryPolicy(
@@ -4566,7 +4568,7 @@ class LayerTreeHostTestMemoryLimits : public LayerTreeHostTest {
             gpu::MemoryAllocation::CUTOFF_ALLOW_NICE_TO_HAVE,
             1000));
         break;
-      case 3:
+      case 4:
         NOTREACHED();
         break;
     }
diff --git a/cc/trees/layer_tree_host_unittest_context.cc b/cc/trees/layer_tree_host_unittest_context.cc
index 4d40bda..0fef2e3 100644
--- a/cc/trees/layer_tree_host_unittest_context.cc
+++ b/cc/trees/layer_tree_host_unittest_context.cc
@@ -1925,8 +1925,7 @@ class LayerTreeHostContextTestSurfaceCreateCallback
  public:
   LayerTreeHostContextTestSurfaceCreateCallback()
       : LayerTreeHostContextTest(),
-        layer_(FakeContentLayer::Create(&client_)),
-        num_commits_(0) {}
+        layer_(FakeContentLayer::Create(&client_)) {}
 
   virtual void SetupTree() OVERRIDE {
     layer_->SetBounds(gfx::Size(10, 20));
@@ -1937,29 +1936,28 @@ class LayerTreeHostContextTestSurfaceCreateCallback
   virtual void BeginTest() OVERRIDE { PostSetNeedsCommitToMainThread(); }
 
   virtual void DidCommit() OVERRIDE {
-    switch (num_commits_) {
-      case 0:
-        EXPECT_EQ(1u, layer_->output_surface_created_count());
-        layer_tree_host()->SetNeedsCommit();
-        break;
+    switch (layer_tree_host()->source_frame_number()) {
       case 1:
         EXPECT_EQ(1u, layer_->output_surface_created_count());
         layer_tree_host()->SetNeedsCommit();
         break;
       case 2:
         EXPECT_EQ(1u, layer_->output_surface_created_count());
+        layer_tree_host()->SetNeedsCommit();
         break;
       case 3:
+        EXPECT_EQ(1u, layer_->output_surface_created_count());
+        break;
+      case 4:
         EXPECT_EQ(2u, layer_->output_surface_created_count());
         layer_tree_host()->SetNeedsCommit();
         break;
     }
-    ++num_commits_;
   }
 
   virtual void CommitCompleteOnThread(LayerTreeHostImpl* impl) OVERRIDE {
     LayerTreeHostContextTest::CommitCompleteOnThread(impl);
-    switch (num_commits_) {
+    switch (LastCommittedSourceFrameNumber(impl)) {
       case 0:
         break;
       case 1:
@@ -1982,7 +1980,6 @@ class LayerTreeHostContextTestSurfaceCreateCallback
  protected:
   FakeContentLayerClient client_;
   scoped_refptr<FakeContentLayer> layer_;
-  int num_commits_;
 };
 
 SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostContextTestSurfaceCreateCallback);
diff --git a/cc/trees/layer_tree_host_unittest_delegated.cc b/cc/trees/layer_tree_host_unittest_delegated.cc
index 683c34d..7b0926f 100644
--- a/cc/trees/layer_tree_host_unittest_delegated.cc
+++ b/cc/trees/layer_tree_host_unittest_delegated.cc
@@ -1863,12 +1863,7 @@ SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostDelegatedTestCommitWithoutTake);
 class DelegatedFrameIsActivatedDuringCommit
     : public LayerTreeHostDelegatedTestCaseSingleDelegatedLayer {
  protected:
-  DelegatedFrameIsActivatedDuringCommit()
-      : wait_thread_("WAIT"),
-        wait_event_(false, false),
-        returned_resource_count_(0) {
-    wait_thread_.Start();
-  }
+  DelegatedFrameIsActivatedDuringCommit() : returned_resource_count_(0) {}
 
   virtual void BeginTest() OVERRIDE {
     activate_count_ = 0;
@@ -1883,31 +1878,11 @@ class DelegatedFrameIsActivatedDuringCommit
   }
 
   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_) {
+  virtual void DidCommit() OVERRIDE {
+    switch (layer_tree_host()->source_frame_number()) {
       case 1: {
         // The first frame has been activated. Set a new frame, and
         // expect the next commit to finish *after* it is activated.
@@ -1916,8 +1891,6 @@ class DelegatedFrameIsActivatedDuringCommit
         AddTextureQuad(frame.get(), 555);
         AddTransferableResource(frame.get(), 555);
         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:
@@ -1925,28 +1898,26 @@ class DelegatedFrameIsActivatedDuringCommit
         // 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:
+        // Finish the test by releasing resources on the next frame.
+        scoped_ptr<DelegatedFrameData> frame =
+            CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
+        SetFrameData(frame.Pass());
         break;
     }
   }
 
-  virtual void DidCommit() OVERRIDE {
-    switch (layer_tree_host()->source_frame_number()) {
+  virtual void CommitCompleteOnThread(LayerTreeHostImpl* host_impl) OVERRIDE {
+    switch (host_impl->active_tree()->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_);
-
-        scoped_ptr<DelegatedFrameData> frame =
-            CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
-        SetFrameData(frame.Pass());
         break;
       }
     }
@@ -1968,9 +1939,6 @@ class DelegatedFrameIsActivatedDuringCommit
       EndTest();
   }
 
-  base::Thread wait_thread_;
-  base::WaitableEvent wait_event_;
-  base::Lock activate_lock_;
   int activate_count_;
   size_t returned_resource_count_;
 };
diff --git a/cc/trees/thread_proxy.cc b/cc/trees/thread_proxy.cc
index e60a29d..0b7d7ef 100644
--- a/cc/trees/thread_proxy.cc
+++ b/cc/trees/thread_proxy.cc
@@ -92,7 +92,7 @@ ThreadProxy::ThreadProxy(
       in_composite_and_readback_(false),
       manage_tiles_pending_(false),
       commit_waits_for_activation_(false),
-      inside_commit_(false),
+      main_thread_inside_commit_(false),
       begin_main_frame_sent_completion_event_on_impl_thread_(NULL),
       readback_request_on_impl_thread_(NULL),
       commit_completion_event_on_impl_thread_(NULL),
@@ -520,7 +520,7 @@ void ThreadProxy::SetNeedsRedraw(const gfx::Rect& damage_rect) {
 
 void ThreadProxy::SetNextCommitWaitsForActivation() {
   DCHECK(IsMainThread());
-  DCHECK(!inside_commit_);
+  DCHECK(!main_thread_inside_commit_);
   commit_waits_for_activation_ = true;
 }
 
@@ -1019,18 +1019,11 @@ void ThreadProxy::ScheduledActionCommit() {
   current_resource_update_controller_on_impl_thread_->Finalize();
   current_resource_update_controller_on_impl_thread_.reset();
 
-  inside_commit_ = true;
+  main_thread_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);
-
-  UpdateBackgroundAnimateTicking();
-
-  next_frame_is_newly_committed_frame_on_impl_thread_ = true;
+  main_thread_inside_commit_ = false;
 
   bool hold_commit = layer_tree_host()->settings().impl_side_painting &&
                      commit_waits_for_activation_;
@@ -1049,6 +1042,17 @@ void ThreadProxy::ScheduledActionCommit() {
     commit_completion_event_on_impl_thread_ = NULL;
   }
 
+  // Delay this step until afer the main thread has been released as it's
+  // often a good bit of work to update the tree and prepare the new frame.
+  layer_tree_host_impl_->CommitComplete();
+
+  SetInputThrottledUntilCommitOnImplThread(false);
+
+  UpdateBackgroundAnimateTicking();
+
+  next_frame_is_newly_committed_frame_on_impl_thread_ = true;
+  commit_waits_for_activation_ = false;
+
   commit_complete_time_ = base::TimeTicks::HighResNow();
   begin_main_frame_to_commit_duration_history_.InsertSample(
       commit_complete_time_ - begin_main_frame_sent_time_);
diff --git a/cc/trees/thread_proxy.h b/cc/trees/thread_proxy.h
index 7da65a1..dacd9cb 100644
--- a/cc/trees/thread_proxy.h
+++ b/cc/trees/thread_proxy.h
@@ -229,7 +229,7 @@ class ThreadProxy : public Proxy,
 
   // Accessed on the main thread, or when main thread is blocked.
   bool commit_waits_for_activation_;
-  bool inside_commit_;
+  bool main_thread_inside_commit_;
 
   scoped_ptr<LayerTreeHostImpl> layer_tree_host_impl_;