Move VISUAL_STATE promise to activation

cc/trees/layer_tree_host_unittest.cc

 // Copyright 2011 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/trees/layer_tree_host.h"
 
 #include <algorithm>
 
 #include "base/auto_reset.h"
 #include "base/location.h"
@@ skipping 4909 matching lines @@
   size_t second_output_surface_memory_limit_;
   FakeContentLayerClient client_;
   scoped_refptr<Layer> root_;
 };
 
 SINGLE_AND_MULTI_THREAD_TEST_F(
     LayerTreeHostTestSetMemoryPolicyOnLostOutputSurface);
 
 struct TestSwapPromiseResult {
   TestSwapPromiseResult()
-      : did_swap_called(false),
+      : did_activate_called(false),
+        did_swap_called(false),
         did_not_swap_called(false),
         dtor_called(false),
-        reason(SwapPromise::DID_NOT_SWAP_UNKNOWN) {}
+        reason(SwapPromise::COMMIT_FAILS) {}
 
+  bool did_activate_called;
   bool did_swap_called;
   bool did_not_swap_called;
   bool dtor_called;
   SwapPromise::DidNotSwapReason reason;
   base::Lock lock;
 };
 
 class TestSwapPromise : public SwapPromise {
  public:
   explicit TestSwapPromise(TestSwapPromiseResult* result) : result_(result) {}
 
   ~TestSwapPromise() override {
     base::AutoLock lock(result_->lock);
     result_->dtor_called = true;
   }
 
+  void DidActivate() override {
+    base::AutoLock lock(result_->lock);
+    EXPECT_FALSE(result_->did_activate_called);
+    EXPECT_FALSE(result_->did_swap_called);
+    EXPECT_FALSE(result_->did_not_swap_called);
+    result_->did_activate_called = true;
+  }
+
   void DidSwap(CompositorFrameMetadata* metadata) override {
     base::AutoLock lock(result_->lock);
+    EXPECT_TRUE(result_->did_activate_called);
     EXPECT_FALSE(result_->did_swap_called);
     EXPECT_FALSE(result_->did_not_swap_called);
     result_->did_swap_called = true;
   }
 
   void DidNotSwap(DidNotSwapReason reason) override {
     base::AutoLock lock(result_->lock);
     EXPECT_FALSE(result_->did_swap_called);
     EXPECT_FALSE(result_->did_not_swap_called);
+    auto implies = [](bool a, bool b) -> bool { return !a || b; };
+    EXPECT_TRUE(implies(reason == DidNotSwapReason::ACTIVATION_FAILS,

[boliu, 2015/05/14 14:40:21]

That's overkill. Can you just write it as EXPECT_TRUE(!foo || bar) then

[Tobias Sargeant, 2015/05/14 21:40:48]

Yes, but then we're almost back to where we started.

IMO, the following is the most legible:

EXPECT_FALSE(result_->did_activate_called && reason ==
DidNotSwapReason::ACTIVATION_FAILS);

+                        !result_->did_activate_called));
     result_->did_not_swap_called = true;
     result_->reason = reason;
   }
 
   int64 TraceId() const override { return 0; }
 
  private:
   // Not owned.
   TestSwapPromiseResult* result_;
 };
@@ skipping 13 matching lines @@
   void BeginTest() override { PostSetNeedsCommitToMainThread(); }
 
   void DidCommit() override {
     commit_count_++;
     if (commit_count_ == 2) {
       // This commit will finish.
       layer_tree_host()->SetNeedsCommit();
     }
   }
 
+  void WillActivateTreeOnThread(LayerTreeHostImpl* host_impl) override {
+    if (host_impl->pending_tree()) {

[boliu, 2015/05/14 14:40:21]

Is this check the same as "host_impl->settings().impl_side_painting". Prefer to
use that, and assert the pending tree is there if impl side painting is on.

[Tobias Sargeant, 2015/05/14 21:40:48]

No, it's not the same.

If you assert that pending_tree is not null, the following tests fail:

8 tests failed:
   
LayerTreeHostTestBreakSwapPromise.RunMultiThread_DelegatingRenderer_MainThreadPaint
   
LayerTreeHostTestBreakSwapPromise.RunMultiThread_DirectRenderer_MainThreadPaint
   
LayerTreeHostTestKeepSwapPromise.RunMultiThread_DelegatingRenderer_MainThreadPaint
   
LayerTreeHostTestKeepSwapPromise.RunMultiThread_DirectRenderer_MainThreadPaint
   
LayerTreeHostTestKeepSwapPromise.RunSingleThread_DelegatingRenderer_ImplSidePaint
   
LayerTreeHostTestKeepSwapPromise.RunSingleThread_DelegatingRenderer_MainThreadPaint
   
LayerTreeHostTestKeepSwapPromise.RunSingleThread_DirectRenderer_ImplSidePaint
   
LayerTreeHostTestKeepSwapPromise.RunSingleThread_DirectRenderer_MainThreadPaint

[boliu, 2015/05/15 03:50:49]

You should DCHECK pending_tree() only if impl_side_painting is true. So only the
*_ImplSidePaint failures are relevant. I'm not sure why those ones are failing
though. Can you look into it?

I guess LayerTreeHostImpl::ActivateSyncTree only checks for pending_tree()
instead of impl_side_painting, so maybe it's good enough.

+      int frame = host_impl->pending_tree()->source_frame_number();
+      base::AutoLock lock(swap_promise_result_[frame].lock);
+      EXPECT_FALSE(swap_promise_result_[frame].did_activate_called);
+      EXPECT_FALSE(swap_promise_result_[frame].did_swap_called);
+    }
+  }
+
+  void DidActivateTreeOnThread(LayerTreeHostImpl* host_impl) override {
+    int frame = host_impl->active_tree()->source_frame_number();
+    base::AutoLock lock(swap_promise_result_[frame].lock);
+    EXPECT_TRUE(swap_promise_result_[frame].did_activate_called);
+    EXPECT_FALSE(swap_promise_result_[frame].did_swap_called);
+  }
+
   void CommitCompleteOnThread(LayerTreeHostImpl* host_impl) override {
     commit_complete_count_++;
     if (commit_complete_count_ == 1) {
       // This commit will be aborted because no actual update.
       PostSetNeedsUpdateLayersToMainThread();
     } else {
       EndTest();
     }
   }
 
   void AfterTest() override {
     // 3 commits are scheduled. 2 completes. 1 is aborted.
     EXPECT_EQ(commit_count_, 3);
     EXPECT_EQ(commit_complete_count_, 2);
 
     {
       // The first commit completes and causes swap buffer which finishes
       // the promise.
       base::AutoLock lock(swap_promise_result_[0].lock);
       EXPECT_TRUE(swap_promise_result_[0].did_swap_called);
       EXPECT_FALSE(swap_promise_result_[0].did_not_swap_called);
       EXPECT_TRUE(swap_promise_result_[0].dtor_called);
     }
 
     {
       // The second commit is aborted since it contains no updates.
       base::AutoLock lock(swap_promise_result_[1].lock);
+      EXPECT_FALSE(swap_promise_result_[1].did_activate_called);
       EXPECT_FALSE(swap_promise_result_[1].did_swap_called);
       EXPECT_TRUE(swap_promise_result_[1].did_not_swap_called);
       EXPECT_EQ(SwapPromise::COMMIT_NO_UPDATE, swap_promise_result_[1].reason);
       EXPECT_TRUE(swap_promise_result_[1].dtor_called);
     }
 
     {
       // The last commit completes but it does not cause swap buffer because
       // there is no damage in the frame data.
       base::AutoLock lock(swap_promise_result_[2].lock);
+      EXPECT_TRUE(swap_promise_result_[2].did_activate_called);
       EXPECT_FALSE(swap_promise_result_[2].did_swap_called);
       EXPECT_TRUE(swap_promise_result_[2].did_not_swap_called);
       EXPECT_EQ(SwapPromise::SWAP_FAILS, swap_promise_result_[2].reason);
       EXPECT_TRUE(swap_promise_result_[2].dtor_called);
     }
   }
 
   int commit_count_;
   int commit_complete_count_;
   TestSwapPromiseResult swap_promise_result_[3];
@@ skipping 29 matching lines @@
             make_scoped_ptr(new TestSwapPromise(&swap_promise_result_)));
         break;
       case 2:
         break;
       default:
         NOTREACHED();
         break;
     }
   }
 
+  void WillActivateTreeOnThread(LayerTreeHostImpl* host_impl) override {
+    if (host_impl->pending_tree() &&
+        host_impl->pending_tree()->source_frame_number() == 1) {
+      base::AutoLock lock(swap_promise_result_.lock);
+      EXPECT_FALSE(swap_promise_result_.did_activate_called);
+      EXPECT_FALSE(swap_promise_result_.did_swap_called);
+    }
+  }
+
+  void DidActivateTreeOnThread(LayerTreeHostImpl* host_impl) override {
+    if (host_impl->active_tree()->source_frame_number() == 1) {
+      base::AutoLock lock(swap_promise_result_.lock);
+      EXPECT_TRUE(swap_promise_result_.did_activate_called);
+      EXPECT_FALSE(swap_promise_result_.did_swap_called);
+    }
+  }
+
   void SwapBuffersOnThread(LayerTreeHostImpl* host_impl, bool result) override {
     EXPECT_TRUE(result);
     if (host_impl->active_tree()->source_frame_number() >= 1) {
       // The commit changes layers so it should cause a swap.
       base::AutoLock lock(swap_promise_result_.lock);
       EXPECT_TRUE(swap_promise_result_.did_swap_called);
       EXPECT_FALSE(swap_promise_result_.did_not_swap_called);
       EXPECT_TRUE(swap_promise_result_.dtor_called);
       EndTest();
     }
@@ skipping 29 matching lines @@
   }
 
   void BeginMainFrameAbortedOnThread(LayerTreeHostImpl* host_impl,
                                      CommitEarlyOutReason reason) override {
     EndTest();
   }
 
   void AfterTest() override {
     {
       base::AutoLock lock(swap_promise_result_.lock);
+      EXPECT_FALSE(swap_promise_result_.did_activate_called);
       EXPECT_FALSE(swap_promise_result_.did_swap_called);
       EXPECT_TRUE(swap_promise_result_.did_not_swap_called);
       EXPECT_EQ(SwapPromise::COMMIT_FAILS, swap_promise_result_.reason);
       EXPECT_TRUE(swap_promise_result_.dtor_called);
     }
   }
 
   TestSwapPromiseResult swap_promise_result_;
 };
 
@@ skipping 29 matching lines @@
   void BeginMainFrameAbortedOnThread(LayerTreeHostImpl* host_impl,
                                      CommitEarlyOutReason reason) override {
     // This is needed so that the impl-thread state matches main-thread state.
     host_impl->DidLoseOutputSurface();
     EndTest();
   }
 
   void AfterTest() override {
     {
       base::AutoLock lock(swap_promise_result_.lock);
+      EXPECT_FALSE(swap_promise_result_.did_activate_called);
       EXPECT_FALSE(swap_promise_result_.did_swap_called);
       EXPECT_TRUE(swap_promise_result_.did_not_swap_called);
       EXPECT_EQ(SwapPromise::COMMIT_FAILS, swap_promise_result_.reason);
       EXPECT_TRUE(swap_promise_result_.dtor_called);
     }
   }
 
   bool output_surface_lost_triggered_;
   TestSwapPromiseResult swap_promise_result_;
 };
@@ skipping 633 matching lines @@
     {
       base::AutoLock lock(swap_promise_result_[0].lock);
       EXPECT_TRUE(swap_promise_result_[0].did_swap_called);
       EXPECT_FALSE(swap_promise_result_[0].did_not_swap_called);
       EXPECT_TRUE(swap_promise_result_[0].dtor_called);
     }
 
     // Second swap promise fails to swap.
     {
       base::AutoLock lock(swap_promise_result_[1].lock);
+      EXPECT_TRUE(swap_promise_result_[1].did_activate_called);
       EXPECT_FALSE(swap_promise_result_[1].did_swap_called);
       EXPECT_TRUE(swap_promise_result_[1].did_not_swap_called);
       EXPECT_EQ(SwapPromise::SWAP_FAILS, swap_promise_result_[1].reason);
       EXPECT_TRUE(swap_promise_result_[1].dtor_called);
     }
 
     // Third swap promises also fails to swap (and draw).
     {
       base::AutoLock lock(swap_promise_result_[2].lock);
+      EXPECT_TRUE(swap_promise_result_[2].did_activate_called);
       EXPECT_FALSE(swap_promise_result_[2].did_swap_called);
       EXPECT_TRUE(swap_promise_result_[2].did_not_swap_called);
       EXPECT_EQ(SwapPromise::SWAP_FAILS, swap_promise_result_[2].reason);
       EXPECT_TRUE(swap_promise_result_[2].dtor_called);
     }
   }
 
   int commit_count_;
   TestSwapPromiseResult swap_promise_result_[3];
 };
@@ skipping 1335 matching lines @@
   void AfterTest() override {}
 
   scoped_refptr<FakePictureLayer> content_child_layer_;
   FakeContentLayerClient client_;
 };
 
 SINGLE_AND_MULTI_THREAD_TEST_F(
     LayerTreeTestReflectionMaskLayerForSurfaceWithUnclippedChild);
 
 }  // namespace cc