Skip to content

Commit

Permalink
Revert 253910 "Make it possible to cancel commits following an a..."
Browse files Browse the repository at this point in the history
This caused rasterize_and_record_micro to fail on mac and windows perf
bots, e.g:
http://build.chromium.org/p/chromium.perf/builders/Mac%2010.8%20Perf%20%285%29/builds/3556

> Make it possible to cancel commits following an animation
> 
> If we didn't invalidate anything when running main thread animations, there is
> no need to continue with the commit.
> 
> BUG=345584,347255
> 
> Review URL: https://codereview.chromium.org/178123003

BUG=348433
TBR=skyostil@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@254420 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
nduca@chromium.org committed Mar 2, 2014
1 parent 0d0f782 commit 0cbb85c
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 97 deletions.
2 changes: 1 addition & 1 deletion cc/trees/layer_tree_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,7 @@ bool LayerTreeHost::UpdateLayers(ResourceUpdateQueue* queue) {

micro_benchmark_controller_.DidUpdateLayers();

return result || next_commit_forces_redraw_;
return result;
}

static Layer* FindFirstScrollableLayer(Layer* layer) {
Expand Down
4 changes: 1 addition & 3 deletions cc/trees/layer_tree_host_perftest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,8 @@ class LayerTreeHostPerfTest : public LayerTreeTest {
}

virtual void Animate(base::TimeTicks monotonic_time) OVERRIDE {
if (animation_driven_drawing_ && !TestEnded()) {
if (animation_driven_drawing_ && !TestEnded())
layer_tree_host()->SetNeedsAnimate();
layer_tree_host()->SetNextCommitForcesRedraw();
}
}

virtual void BeginCommitOnThread(LayerTreeHostImpl* host_impl) OVERRIDE {
Expand Down
81 changes: 0 additions & 81 deletions cc/trees/layer_tree_host_unittest_animation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -783,15 +783,6 @@ class LayerTreeHostAnimationTestContinuousAnimate
num_draw_layers_(0) {
}

virtual void SetupTree() OVERRIDE {
LayerTreeHostAnimationTest::SetupTree();
// Create a fake content layer so we actually produce new content for every
// animation frame.
content_ = FakeContentLayer::Create(&client_);
content_->set_always_update_resources(true);
layer_tree_host()->root_layer()->AddChild(content_);
}

virtual void BeginTest() OVERRIDE {
PostSetNeedsCommitToMainThread();
}
Expand Down Expand Up @@ -825,82 +816,10 @@ class LayerTreeHostAnimationTestContinuousAnimate
private:
int num_commit_complete_;
int num_draw_layers_;
FakeContentLayerClient client_;
scoped_refptr<FakeContentLayer> content_;
};

MULTI_THREAD_TEST_F(LayerTreeHostAnimationTestContinuousAnimate);

class LayerTreeHostAnimationTestCancelAnimateCommit
: public LayerTreeHostAnimationTest {
public:
LayerTreeHostAnimationTestCancelAnimateCommit() : num_animate_calls_(0) {}

virtual void BeginTest() OVERRIDE { PostSetNeedsCommitToMainThread(); }

virtual void Animate(base::TimeTicks) OVERRIDE {
// No-op animate will cancel the commit.
if (++num_animate_calls_ == 2) {
EndTest();
return;
}
layer_tree_host()->SetNeedsAnimate();
}

virtual void CommitCompleteOnThread(LayerTreeHostImpl* tree_impl) OVERRIDE {
FAIL() << "Commit should have been canceled.";
}

virtual void DrawLayersOnThread(LayerTreeHostImpl* impl) OVERRIDE {
FAIL() << "Draw should have been canceled.";
}

virtual void AfterTest() OVERRIDE { EXPECT_EQ(2, num_animate_calls_); }

private:
int num_animate_calls_;
FakeContentLayerClient client_;
scoped_refptr<FakeContentLayer> content_;
};

MULTI_THREAD_TEST_F(LayerTreeHostAnimationTestCancelAnimateCommit);

class LayerTreeHostAnimationTestForceRedraw
: public LayerTreeHostAnimationTest {
public:
LayerTreeHostAnimationTestForceRedraw()
: num_animate_(0), num_draw_layers_(0) {}

virtual void BeginTest() OVERRIDE { PostSetNeedsCommitToMainThread(); }

virtual void Animate(base::TimeTicks) OVERRIDE {
if (++num_animate_ < 2)
layer_tree_host()->SetNeedsAnimate();
}

virtual void Layout() OVERRIDE {
layer_tree_host()->SetNextCommitForcesRedraw();
}

virtual void DrawLayersOnThread(LayerTreeHostImpl* impl) OVERRIDE {
if (++num_draw_layers_ == 2)
EndTest();
}

virtual void AfterTest() OVERRIDE {
// The first commit will always draw; make sure the second draw triggered
// by the animation was not cancelled.
EXPECT_EQ(2, num_draw_layers_);
EXPECT_EQ(2, num_animate_);
}

private:
int num_animate_;
int num_draw_layers_;
};

MULTI_THREAD_TEST_F(LayerTreeHostAnimationTestForceRedraw);

// Make sure the main thread can still execute animations when CanDraw() is not
// true.
class LayerTreeHostAnimationTestRunAnimationWhenNotCanDraw
Expand Down
25 changes: 13 additions & 12 deletions cc/trees/thread_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ void ThreadProxy::SetNeedsAnimate() {

TRACE_EVENT0("cc", "ThreadProxy::SetNeedsAnimate");
main().animate_requested = true;
main().can_cancel_commit = true;
main().can_cancel_commit = false;
SendCommitRequestToImplThreadIfNeeded();
}

Expand Down Expand Up @@ -911,17 +911,6 @@ void ThreadProxy::BeginMainFrame(

layer_tree_host()->WillCommit();

// Before calling animate, we set main().animate_requested to false. If it is
// true now, it means SetNeedAnimate was called again, but during a state when
// main().commit_request_sent_to_impl_thread = true. We need to force that
// call to happen again now so that the commit request is sent to the impl
// thread.
if (main().animate_requested) {
// Forces SetNeedsAnimate to consider posting a commit task.
main().animate_requested = false;
SetNeedsAnimate();
}

if (!updated && can_cancel_this_commit) {
TRACE_EVENT_INSTANT0("cc", "EarlyOut_NoUpdates", TRACE_EVENT_SCOPE_THREAD);
bool did_handle = true;
Expand All @@ -939,6 +928,18 @@ void ThreadProxy::BeginMainFrame(
return;
}

// Before calling animate, we set main().animate_requested to false. If it is
// true
// now, it means SetNeedAnimate was called again, but during a state when
// main().commit_request_sent_to_impl_thread = true. We need to force that
// call to
// happen again now so that the commit request is sent to the impl thread.
if (main().animate_requested) {
// Forces SetNeedsAnimate to consider posting a commit task.
main().animate_requested = false;
SetNeedsAnimate();
}

scoped_refptr<ContextProvider> offscreen_context_provider;
if (main().renderer_capabilities_main_thread_copy.using_offscreen_context3d &&
layer_tree_host()->needs_offscreen_context()) {
Expand Down

0 comments on commit 0cbb85c

Please sign in to comment.