From 2e02f73e17f4fc3f611a08582b7944bb32362a2f Mon Sep 17 00:00:00 2001 From: tobiasjs Date: Wed, 2 Sep 2015 05:08:03 -0700 Subject: [PATCH] Support swap promises that are pinned to a particular layer tree. A pinned swap promise queued on a pending tree will only see DidActivate() or DidNotSwap(ACTIVATION_FAILS). It is not passed to the active tree, so it will not swap. A pinned swap promise on the active tree will either see only DidSwap() or DidNotSwap(SWAP_FAILS). No DidActivate() will be seen because that has already happened prior to queueing of the swap promise. Conversely, an unpinned swap promise (queued by QueueSwapPromise()) travels with the layer information currently associated with the tree. For example, when a pending tree is activated, the swap promise is passed to the active tree along with the layer information. Similarly, when a new activation overwrites layer information on the active tree, queued swap promises are broken. This allows us to break unpinned swap promises on the active tree at the point at which we activate the pending tree without changing latency measurement. BUG=498824 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review URL: https://codereview.chromium.org/1292773003 Cr-Commit-Position: refs/heads/master@{#346894} --- cc/layers/surface_layer_unittest.cc | 78 +++++++++++++++++++++------ cc/trees/latency_info_swap_promise_monitor.cc | 7 ++- cc/trees/layer_tree_host_impl_unittest.cc | 2 +- cc/trees/layer_tree_host_unittest.cc | 48 ++++++++++++++++- cc/trees/layer_tree_impl.cc | 25 +++++++-- cc/trees/layer_tree_impl.h | 19 +++++++ 6 files changed, 158 insertions(+), 21 deletions(-) diff --git a/cc/layers/surface_layer_unittest.cc b/cc/layers/surface_layer_unittest.cc index eac14a0c245f..e35b88683d81 100644 --- a/cc/layers/surface_layer_unittest.cc +++ b/cc/layers/surface_layer_unittest.cc @@ -143,23 +143,43 @@ class SurfaceLayerSwapPromise : public LayerTreeTest { gfx::Size bounds(100, 100); layer_tree_host()->SetViewportSize(bounds); + + blank_layer_ = SolidColorLayer::Create(layer_settings()); + blank_layer_->SetIsDrawable(true); + blank_layer_->SetBounds(gfx::Size(10, 10)); + PostSetNeedsCommitToMainThread(); } + virtual void ChangeTree() = 0; + void DidCommitAndDrawFrame() override { base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::Bind(&SurfaceLayerSwapPromise::ChangeTree, base::Unretained(this))); } - void ChangeTree() { + protected: + int commit_count_; + bool sequence_was_satisfied_; + scoped_refptr layer_; + scoped_refptr blank_layer_; + SurfaceSequence satisfied_sequence_; + + SurfaceId required_id_; + std::set required_set_; +}; + +// Check that SurfaceSequence is sent through swap promise. +class SurfaceLayerSwapPromiseWithDraw : public SurfaceLayerSwapPromise { + public: + SurfaceLayerSwapPromiseWithDraw() : SurfaceLayerSwapPromise() {} + + void ChangeTree() override { ++commit_count_; switch (commit_count_) { case 1: // Remove SurfaceLayer from tree to cause SwapPromise to be created. - blank_layer_ = SolidColorLayer::Create(layer_settings()); - blank_layer_->SetIsDrawable(true); - blank_layer_->SetBounds(gfx::Size(10, 10)); layer_tree_host()->SetRootLayer(blank_layer_); break; case 2: @@ -193,21 +213,49 @@ class SurfaceLayerSwapPromise : public LayerTreeTest { // callback. EXPECT_TRUE(satisfied_sequence_.is_null()); } - - private: - int commit_count_; - bool sequence_was_satisfied_; - scoped_refptr layer_; - scoped_refptr blank_layer_; - SurfaceSequence satisfied_sequence_; - - SurfaceId required_id_; - std::set required_set_; }; // TODO(jbauman): Reenable on single thread once http://crbug.com/421923 is // fixed. -MULTI_THREAD_TEST_F(SurfaceLayerSwapPromise); +MULTI_THREAD_TEST_F(SurfaceLayerSwapPromiseWithDraw); + +// Check that SurfaceSequence is sent through swap promise and resolved when +// swap fails. +class SurfaceLayerSwapPromiseWithoutDraw : public SurfaceLayerSwapPromise { + public: + SurfaceLayerSwapPromiseWithoutDraw() : SurfaceLayerSwapPromise() {} + + DrawResult PrepareToDrawOnThread(LayerTreeHostImpl* host_impl, + LayerTreeHostImpl::FrameData* frame, + DrawResult draw_result) override { + return DRAW_ABORTED_MISSING_HIGH_RES_CONTENT; + } + + void ChangeTree() override { + ++commit_count_; + switch (commit_count_) { + case 1: + // Remove SurfaceLayer from tree to cause SwapPromise to be created. + layer_tree_host()->SetRootLayer(blank_layer_); + break; + case 2: + layer_tree_host()->SetNeedsCommit(); + break; + default: + EndTest(); + break; + } + } + + void AfterTest() override { + EXPECT_TRUE(required_id_ == SurfaceId(1)); + EXPECT_EQ(1u, required_set_.size()); + // Sequence should have been satisfied with the callback. + EXPECT_TRUE(satisfied_sequence_ == SurfaceSequence(1u, 1u)); + } +}; + +MULTI_THREAD_TEST_F(SurfaceLayerSwapPromiseWithoutDraw); } // namespace } // namespace cc diff --git a/cc/trees/latency_info_swap_promise_monitor.cc b/cc/trees/latency_info_swap_promise_monitor.cc index 6e9c57e04231..a57ae4c8c41c 100644 --- a/cc/trees/latency_info_swap_promise_monitor.cc +++ b/cc/trees/latency_info_swap_promise_monitor.cc @@ -59,7 +59,12 @@ void LatencyInfoSwapPromiseMonitor::OnSetNeedsCommitOnMain() { void LatencyInfoSwapPromiseMonitor::OnSetNeedsRedrawOnImpl() { if (AddRenderingScheduledComponent(latency_, false /* on_main */)) { scoped_ptr swap_promise(new LatencyInfoSwapPromise(*latency_)); - layer_tree_host_impl_->active_tree()->QueueSwapPromise(swap_promise.Pass()); + // Queue a pinned swap promise on the active tree. This will allow + // measurement of the time to the next SwapBuffers(). The swap + // promise is pinned so that it is not interrupted by new incoming + // activations (which would otherwise break the swap promise). + layer_tree_host_impl_->active_tree()->QueuePinnedSwapPromise( + swap_promise.Pass()); } } diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc index 3b5765f2b817..90ad5cf17658 100644 --- a/cc/trees/layer_tree_host_impl_unittest.cc +++ b/cc/trees/layer_tree_host_impl_unittest.cc @@ -6978,7 +6978,7 @@ TEST_F(LayerTreeHostImplTest, LatencyInfoPassedToCompositorFrameMetadata) { ui::INPUT_EVENT_LATENCY_BEGIN_RWH_COMPONENT, 0, 0); scoped_ptr swap_promise( new LatencyInfoSwapPromise(latency_info)); - host_impl_->active_tree()->QueueSwapPromise(swap_promise.Pass()); + host_impl_->active_tree()->QueuePinnedSwapPromise(swap_promise.Pass()); host_impl_->SetNeedsRedraw(); gfx::Rect full_frame_damage(host_impl_->DrawViewportSize()); diff --git a/cc/trees/layer_tree_host_unittest.cc b/cc/trees/layer_tree_host_unittest.cc index 9636f38e366e..10d8e3765b01 100644 --- a/cc/trees/layer_tree_host_unittest.cc +++ b/cc/trees/layer_tree_host_unittest.cc @@ -3879,7 +3879,6 @@ class TestSwapPromise : public SwapPromise { 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; @@ -3902,6 +3901,53 @@ class TestSwapPromise : public SwapPromise { TestSwapPromiseResult* result_; }; +class PinnedLayerTreeSwapPromise : public LayerTreeHostTest { + protected: + void BeginTest() override { + PostSetNextCommitForcesRedrawToMainThread(); + PostSetNeedsCommitToMainThread(); + } + + void CommitCompleteOnThread(LayerTreeHostImpl* host_impl) override { + int frame = host_impl->active_tree()->source_frame_number(); + if (frame == -1) { + host_impl->active_tree()->QueuePinnedSwapPromise(make_scoped_ptr( + new TestSwapPromise(&pinned_active_swap_promise_result_))); + host_impl->pending_tree()->QueueSwapPromise( + make_scoped_ptr(new TestSwapPromise(&pending_swap_promise_result_))); + host_impl->active_tree()->QueueSwapPromise( + make_scoped_ptr(new TestSwapPromise(&active_swap_promise_result_))); + } + } + + void SwapBuffersOnThread(LayerTreeHostImpl* host_impl, bool result) override { + EndTest(); + } + + void AfterTest() override { + // The pending swap promise should activate and swap. + EXPECT_TRUE(pending_swap_promise_result_.did_activate_called); + EXPECT_TRUE(pending_swap_promise_result_.did_swap_called); + + // The active swap promise should fail to swap (it is cancelled by + // the activation of a new frame). + EXPECT_FALSE(active_swap_promise_result_.did_activate_called); + EXPECT_FALSE(active_swap_promise_result_.did_swap_called); + EXPECT_TRUE(active_swap_promise_result_.did_not_swap_called); + EXPECT_EQ(active_swap_promise_result_.reason, SwapPromise::SWAP_FAILS); + + // The pinned active swap promise should not activate, but should swap. + EXPECT_FALSE(pinned_active_swap_promise_result_.did_activate_called); + EXPECT_TRUE(pinned_active_swap_promise_result_.did_swap_called); + } + + TestSwapPromiseResult pending_swap_promise_result_; + TestSwapPromiseResult active_swap_promise_result_; + TestSwapPromiseResult pinned_active_swap_promise_result_; +}; + +MULTI_THREAD_TEST_F(PinnedLayerTreeSwapPromise); + class LayerTreeHostTestBreakSwapPromise : public LayerTreeHostTest { protected: LayerTreeHostTestBreakSwapPromise() diff --git a/cc/trees/layer_tree_impl.cc b/cc/trees/layer_tree_impl.cc index 19d85bb2741c..8fe74d0f3e09 100644 --- a/cc/trees/layer_tree_impl.cc +++ b/cc/trees/layer_tree_impl.cc @@ -1026,6 +1026,11 @@ void LayerTreeImpl::AsValueInto(base::trace_event::TracedValue* state) const { for (auto* swap_promise : swap_promise_list_) state->AppendDouble(swap_promise->TraceId()); state->EndArray(); + + state->BeginArray("pinned_swap_promise_trace_ids"); + for (auto* swap_promise : pinned_swap_promise_list_) + state->AppendDouble(swap_promise->TraceId()); + state->EndArray(); } void LayerTreeImpl::SetRootLayerScrollOffsetDelegate( @@ -1103,23 +1108,37 @@ void LayerTreeImpl::QueueSwapPromise(scoped_ptr swap_promise) { swap_promise_list_.push_back(swap_promise.Pass()); } +void LayerTreeImpl::QueuePinnedSwapPromise( + scoped_ptr swap_promise) { + DCHECK(IsActiveTree()); + DCHECK(swap_promise); + pinned_swap_promise_list_.push_back(swap_promise.Pass()); +} + void LayerTreeImpl::PassSwapPromises( ScopedPtrVector* new_swap_promise) { - swap_promise_list_.insert_and_take(swap_promise_list_.end(), - new_swap_promise); - new_swap_promise->clear(); + for (auto* swap_promise : swap_promise_list_) + swap_promise->DidNotSwap(SwapPromise::SWAP_FAILS); + swap_promise_list_.clear(); + swap_promise_list_.swap(*new_swap_promise); } void LayerTreeImpl::FinishSwapPromises(CompositorFrameMetadata* metadata) { for (auto* swap_promise : swap_promise_list_) swap_promise->DidSwap(metadata); swap_promise_list_.clear(); + for (auto* swap_promise : pinned_swap_promise_list_) + swap_promise->DidSwap(metadata); + pinned_swap_promise_list_.clear(); } void LayerTreeImpl::BreakSwapPromises(SwapPromise::DidNotSwapReason reason) { for (auto* swap_promise : swap_promise_list_) swap_promise->DidNotSwap(reason); swap_promise_list_.clear(); + for (auto* swap_promise : pinned_swap_promise_list_) + swap_promise->DidNotSwap(reason); + pinned_swap_promise_list_.clear(); } void LayerTreeImpl::DidModifyTilePriorities() { diff --git a/cc/trees/layer_tree_impl.h b/cc/trees/layer_tree_impl.h index 88f543550201..fb35fd393a6e 100644 --- a/cc/trees/layer_tree_impl.h +++ b/cc/trees/layer_tree_impl.h @@ -285,8 +285,26 @@ class CC_EXPORT LayerTreeImpl { // Call this function when you expect there to be a swap buffer. // See swap_promise.h for how to use SwapPromise. + // + // A swap promise queued by QueueSwapPromise travels with the layer + // information currently associated with the tree. For example, when + // a pending tree is activated, the swap promise is passed to the + // active tree along with the layer information. Similarly, when a + // new activation overwrites layer information on the active tree, + // queued swap promises are broken. void QueueSwapPromise(scoped_ptr swap_promise); + // Queue a swap promise, pinned to this tree. Pinned swap promises + // may only be queued on the active tree. + // + // An active tree pinned swap promise will see only DidSwap() or + // DidNotSwap(SWAP_FAILS). No DidActivate() will be seen because + // that has already happened prior to queueing of the swap promise. + // + // Pinned active tree swap promises will not be broken prematurely + // on the active tree if a new tree is activated. + void QueuePinnedSwapPromise(scoped_ptr swap_promise); + // Take the |new_swap_promise| and append it to |swap_promise_list_|. void PassSwapPromises(ScopedPtrVector* new_swap_promise); void FinishSwapPromises(CompositorFrameMetadata* metadata); @@ -445,6 +463,7 @@ class CC_EXPORT LayerTreeImpl { bool has_ever_been_drawn_; ScopedPtrVector swap_promise_list_; + ScopedPtrVector pinned_swap_promise_list_; UIResourceRequestQueue ui_resource_request_queue_; -- 2.11.4.GIT