cc: Use BeginFrameArgs

cc/layers/texture_layer_unittest.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/layers/texture_layer.h"
 
 #include <string>
 
 #include "base/callback.h"
 #include "cc/layers/texture_layer_client.h"
@@ skipping 1004 matching lines @@
   virtual void BeginTest() OVERRIDE {
     callback_count_ = 0;
 
     // Set the mailbox on the main thread.
     SetMailbox('1');
     EXPECT_EQ(0, callback_count_);
 
     PostSetNeedsCommitToMainThread();
   }
 
-  virtual void DidCommitAndDrawFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
     switch (layer_tree_host()->source_frame_number()) {
       case 1:
         // Delete the TextureLayer on the main thread while the mailbox is in
         // the impl tree.
         layer_->RemoveFromParent();
         layer_ = NULL;
         break;
     }
   }
 
@@ skipping 47 matching lines @@
   virtual void BeginTest() OVERRIDE {
     callback_count_ = 0;
 
     // Set the mailbox on the main thread.
     SetMailbox('1');
     EXPECT_EQ(0, callback_count_);
 
     PostSetNeedsCommitToMainThread();
   }
 
-  virtual void DidCommitAndDrawFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {

[enne (OOO), 2013/08/06 23:35:16]

It seems like this is not testing what it should be.  The test seems to be
waiting for the TextureLayerImpl to be deleted first as a result of the first
commit.  However, this won't be the case right after the second commit; it'll
only happen after the second activation?

[brianderson, 2013/08/07 01:55:51]

I changed this so that DidActivateTreeOnThread posts a task to the main thread
to delete the TextureLayer. Should that work?

[enne (OOO), 2013/08/07 15:44:19]

Yeah, that seems like the right approach.

[piman, 2013/08/07 19:39:52]

I would prefer that we wait for a frame to be drawn. We only create the resource
in WillDraw, and this is really meant to exercise this behavior (if we delete
the layer after the resource was created).

Why can't we keep DidCommitAndDrawFrame ?

[brianderson, 2013/08/07 20:12:29]

It's possible that DidCommitAndDrawFrame doesn't get called for the first time
until after two commits have occurred, so we end up not seeing all values of
source_frame_number() in DidCommitAndDrawFrame sometimes.

If we want to verify that a particular source_frame_number was activated or
drawn, we have to check that on the Impl thread.

Maybe it would help if DidCommitAndDrawFrame took the source_frame_number as an
argument that the Impl thread could communicate to the main thread?

For the purposes of this patch, however, my current plan is to post the task to
the main thread to delete the Texture layer from SwapBuffersCompleteOnThread
instead of DidActivateTreeOnThread as originally planned.

[piman, 2013/08/07 20:27:45]

You want to make sure you wait for the commit *and* the frame that corresponds
to that commit (i.e. not one before that).

I'm surprised we now see DidCommitAndDrawFrame being called after the next
commit (that increases the source frame number) happened. I suspect some amount
of synchronization depends on the current behavior.

Doesn't that mean that we have 2 commits in flight now? Isn't that a problem
(increasing memory load)?

[brianderson, 2013/08/07 20:33:01]

This patch doesn't allow 2 commits in flight, but it does allow a second commit
to start before the previous commit has drawn.  Previously, the commit was held
until after the draw.

     switch (layer_tree_host()->source_frame_number()) {
       case 1:
         // Remove the TextureLayer on the main thread while the mailbox is in
         // the impl tree, but don't delete the TextureLayer until after the impl
         // tree side is deleted.
         layer_->RemoveFromParent();
         break;
       case 2:
         layer_ = NULL;
         break;
     }
   }
 
   virtual void AfterTest() OVERRIDE {
     EXPECT_EQ(1, callback_count_);
   }
 
  private:
   int callback_count_;
   scoped_refptr<Layer> root_;
   scoped_refptr<TextureLayer> layer_;
 };
 
 SINGLE_AND_MULTI_THREAD_DIRECT_RENDERER_TEST_F(
     TextureLayerWithMailboxImplThreadDeleted);
 
 }  // namespace
 }  // namespace cc