Skip to content

Commit

Permalink
cc: Fix race in ThreadProxy unittests.
Browse files Browse the repository at this point in the history
This fixes the data race in ThreadProxyTestCommitWaitsForActivation.
Reported by TSAN.

BUG=437454
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

Cr-Commit-Position: refs/heads/master@{#369940}
  • Loading branch information
khushalsagar authored and Commit bot committed Jan 16, 2016
1 parent 14702a5 commit 45c0ff1
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 19 deletions.
13 changes: 13 additions & 0 deletions cc/test/layer_tree_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,13 @@ void LayerTreeTest::PostCompositeImmediatelyToMainThread() {
main_thread_weak_ptr_));
}

void LayerTreeTest::PostNextCommitWaitsForActivationToMainThread() {
main_task_runner_->PostTask(
FROM_HERE,
base::Bind(&LayerTreeTest::DispatchNextCommitWaitsForActivation,
main_thread_weak_ptr_));
}

void LayerTreeTest::WillBeginTest() {
layer_tree_host_->SetVisible(true);
}
Expand Down Expand Up @@ -800,6 +807,12 @@ void LayerTreeTest::DispatchCompositeImmediately() {
layer_tree_host_->Composite(base::TimeTicks::Now());
}

void LayerTreeTest::DispatchNextCommitWaitsForActivation() {
DCHECK(!task_runner_provider() || task_runner_provider()->IsMainThread());
if (layer_tree_host_)
layer_tree_host_->SetNextCommitWaitsForActivation();
}

void LayerTreeTest::RunTest(CompositorMode mode, bool delegating_renderer) {
mode_ = mode;
if (mode_ == CompositorMode::Threaded) {
Expand Down
2 changes: 2 additions & 0 deletions cc/test/layer_tree_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ class LayerTreeTest : public testing::Test, public TestHooks {
void PostSetVisibleToMainThread(bool visible);
void PostSetNextCommitForcesRedrawToMainThread();
void PostCompositeImmediatelyToMainThread();
void PostNextCommitWaitsForActivationToMainThread();

void DoBeginTest();
void Timeout();
Expand Down Expand Up @@ -123,6 +124,7 @@ class LayerTreeTest : public testing::Test, public TestHooks {
void DispatchSetNextCommitForcesRedraw();
void DispatchDidAddAnimation();
void DispatchCompositeImmediately();
void DispatchNextCommitWaitsForActivation();

virtual void AfterTest() = 0;
virtual void WillBeginTest();
Expand Down
38 changes: 19 additions & 19 deletions cc/trees/layer_tree_host_unittest_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -293,46 +293,46 @@ class ProxyMainThreadedCommitWaitsForActivation : public ProxyMainThreaded {
// completion event is cleared.
EXPECT_FALSE(GetProxyImplForTest()->HasCommitCompletionEvent());
EXPECT_FALSE(GetProxyImplForTest()->GetNextCommitWaitsForActivation());

// Set next commit waits for activation and start another commit.
commits_completed_++;
PostNextCommitWaitsForActivationToMainThread();
PostSetNeedsCommitToMainThread();
break;
case 1:
// The second commit should be held until activation.
EXPECT_TRUE(GetProxyImplForTest()->HasCommitCompletionEvent());
EXPECT_TRUE(GetProxyImplForTest()->GetNextCommitWaitsForActivation());

// Start another commit to verify that this is not held until
// activation.
commits_completed_++;
PostSetNeedsCommitToMainThread();
break;
case 2:
// The third commit should not wait for activation.
EXPECT_FALSE(GetProxyImplForTest()->HasCommitCompletionEvent());
EXPECT_FALSE(GetProxyImplForTest()->GetNextCommitWaitsForActivation());

commits_completed_++;
}
}

void DidActivateSyncTree() override {
// The next_commit_waits_for_activation should have been cleared after the
// sync tree is activated.
EXPECT_FALSE(GetProxyImplForTest()->GetNextCommitWaitsForActivation());
if (commits_completed_ == 3)
EndTest();
}

void DidCommit() override {
switch (commits_completed_) {
case 0:
// The first commit has been completed. Set next commit waits for
// activation and start another commit.
commits_completed_++;
proxy()->SetNextCommitWaitsForActivation();
proxy()->SetNeedsCommit();
case 1:
// Start another commit to verify that this is not held until
// activation.
commits_completed_++;
proxy()->SetNeedsCommit();
case 2:
commits_completed_++;
EndTest();
}
void AfterTest() override {
// It is safe to read commits_completed_ on the main thread now since
// AfterTest() runs after the LayerTreeHost is destroyed and the impl thread
// tear down is finished.
EXPECT_EQ(3, commits_completed_);
}

void AfterTest() override { EXPECT_EQ(3, commits_completed_); }

private:
int commits_completed_;

Expand Down

0 comments on commit 45c0ff1

Please sign in to comment.