Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4457)

Unified Diff: cc/trees/layer_tree_host_unittest_context.cc

Issue 23804004: [cc] Update UIResource test threading (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Consolidate common code Created 7 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: cc/trees/layer_tree_host_unittest_context.cc
diff --git a/cc/trees/layer_tree_host_unittest_context.cc b/cc/trees/layer_tree_host_unittest_context.cc
index c250b0c6bb9154ff79f0f48bfd0b94fdc355c9c2..b6060a96d495eae56e3b7c02a6969f0822fac2f1 100644
--- a/cc/trees/layer_tree_host_unittest_context.cc
+++ b/cc/trees/layer_tree_host_unittest_context.cc
@@ -1637,27 +1637,102 @@ class UIResourceLostTest : public LayerTreeHostContextTest {
virtual void BeginTest() OVERRIDE { PostSetNeedsCommitToMainThread(); }
virtual void AfterTest() OVERRIDE {}
+ // This is called on the main thread after each commit and
+ // DidActivateTreeOnThread, with the value of time_step_ at the time
+ // of the call to DidActivateTreeOnThread. Similar tests will do
+ // work on the main thread in DidCommit but that is unsuitable because
+ // the main thread work for these tests must happen after
+ // DidActivateTreeOnThread, which happens after DidCommit with impl-side
+ // painting.
+ virtual void StepCompleteOnMainThread(int time_step) = 0;
+
+ // Called after DidActivateTreeOnThread. If this is done during the commit,
+ // the call to StepCompleteOnMainThread will not occur until after
+ // the commit completes, because the main thread is blocked.
+ void PostStepCompleteToMainThread() {
+ proxy()->MainThreadTaskRunner()->PostTask(
+ FROM_HERE,
+ base::Bind(
+ &UIResourceLostTest::StepCompleteOnMainThreadInternal,
+ base::Unretained(this),
+ time_step_));
+ }
+
+ void PostLoseContextToImplThread() {
+ EXPECT_TRUE(layer_tree_host()->proxy()->IsMainThread());
+ base::SingleThreadTaskRunner* task_runner =
+ HasImplThread() ? ImplThreadTaskRunner()
+ : base::MessageLoopProxy::current();
+ task_runner->PostTask(
+ FROM_HERE,
+ base::Bind(
+ &LayerTreeHostContextTest::LoseContext,
+ base::Unretained(this)));
+ }
+
protected:
int time_step_;
scoped_ptr<FakeScopedUIResource> ui_resource_;
+
+ private:
+ void StepCompleteOnMainThreadInternal(int step) {
+ EXPECT_TRUE(layer_tree_host()->proxy()->IsMainThread());
+ StepCompleteOnMainThread(step);
+ }
};
-// Losing context after an UI resource has been created.
-class UIResourceLostAfterCommit : public UIResourceLostTest {
+class UIResourceLostTestSimple : public UIResourceLostTest {
public:
+ // This is called when the commit is complete and the new layer tree has been
+ // activated.
+ virtual void StepCompleteOnImplThread(LayerTreeHostImpl* impl) = 0;
+
virtual void CommitCompleteOnThread(LayerTreeHostImpl* impl) OVERRIDE {
- LayerTreeHostContextTest::CommitCompleteOnThread(impl);
- switch (time_step_) {
+ if (!layer_tree_host()->settings().impl_side_painting) {
+ StepCompleteOnImplThread(impl);
+ PostStepCompleteToMainThread();
+ ++time_step_;
+ }
+ }
+
+ virtual void DidActivateTreeOnThread(LayerTreeHostImpl* impl) OVERRIDE {
+ if (layer_tree_host()->settings().impl_side_painting) {
+ StepCompleteOnImplThread(impl);
+ PostStepCompleteToMainThread();
+ ++time_step_;
+ }
+ }
+};
+
+// Losing context after an UI resource has been created.
+class UIResourceLostAfterCommit : public UIResourceLostTestSimple {
+ public:
+ virtual void StepCompleteOnMainThread(int step) OVERRIDE {
+ EXPECT_TRUE(layer_tree_host()->proxy()->IsMainThread());
+ switch (step) {
case 0:
ui_resource_ = FakeScopedUIResource::Create(layer_tree_host());
// Expects a valid UIResourceId.
EXPECT_NE(0, ui_resource_->id());
PostSetNeedsCommitToMainThread();
break;
+ case 4:
+ // Release resource before ending the test.
+ ui_resource_.reset();
+ EndTest();
+ break;
+ case 5:
+ // Make sure no extra commits happened.
+ NOTREACHED();
+ }
+ }
+
+ virtual void StepCompleteOnImplThread(LayerTreeHostImpl* impl) OVERRIDE {
+ LayerTreeHostContextTest::CommitCompleteOnThread(impl);
+ switch (time_step_) {
case 1:
// The resource should have been created on LTHI after the commit.
- if (!layer_tree_host()->settings().impl_side_painting)
- EXPECT_NE(0u, impl->ResourceIdForUIResource(ui_resource_->id()));
+ EXPECT_NE(0u, impl->ResourceIdForUIResource(ui_resource_->id()));
PostSetNeedsCommitToMainThread();
break;
case 2:
@@ -1669,31 +1744,10 @@ class UIResourceLostAfterCommit : public UIResourceLostTest {
EXPECT_EQ(1, ui_resource_->lost_resource_count);
// Resource Id on the impl-side have been recreated as well. Note
// that the same UIResourceId persists after the context lost.
- if (!layer_tree_host()->settings().impl_side_painting)
- EXPECT_NE(0u, impl->ResourceIdForUIResource(ui_resource_->id()));
+ EXPECT_NE(0u, impl->ResourceIdForUIResource(ui_resource_->id()));
PostSetNeedsCommitToMainThread();
break;
- case 4:
- // Release resource before ending test.
- ui_resource_.reset();
- EndTest();
- break;
- }
- }
-
- virtual void DidActivateTreeOnThread(LayerTreeHostImpl* impl) OVERRIDE {
- LayerTreeHostContextTest::DidActivateTreeOnThread(impl);
- switch (time_step_) {
- case 1:
- if (layer_tree_host()->settings().impl_side_painting)
- EXPECT_NE(0u, impl->ResourceIdForUIResource(ui_resource_->id()));
- break;
- case 3:
- if (layer_tree_host()->settings().impl_side_painting)
- EXPECT_NE(0u, impl->ResourceIdForUIResource(ui_resource_->id()));
- break;
}
- ++time_step_;
}
};
@@ -1708,34 +1762,18 @@ SINGLE_AND_MULTI_THREAD_TEST_F(UIResourceLostAfterCommit);
// test_id1_ to have been created.
// 3. Create one resource -> Delete that same resource -> Context Lost => Expect
// the resource to not exist in the manager.
-class UIResourceLostBeforeCommit : public UIResourceLostTest {
+class UIResourceLostBeforeCommit : public UIResourceLostTestSimple {
public:
UIResourceLostBeforeCommit()
: test_id0_(0),
test_id1_(0) {}
- virtual void CommitCompleteOnThread(LayerTreeHostImpl* impl) OVERRIDE {
- LayerTreeHostContextTest::CommitCompleteOnThread(impl);
- switch (time_step_) {
+ virtual void StepCompleteOnMainThread(int step) OVERRIDE {
+ switch (step) {
case 0:
- // Sequence 1:
ui_resource_ = FakeScopedUIResource::Create(layer_tree_host());
- LoseContext();
- // Resource Id on the impl-side should no longer be valid after
- // context is lost.
- EXPECT_EQ(0u, impl->ResourceIdForUIResource(ui_resource_->id()));
- break;
- case 1:
- // The resources should have been recreated.
- EXPECT_EQ(2, ui_resource_->resource_create_count);
- // "resource lost" callback was called once for the resource in the
- // resource map.
- EXPECT_EQ(1, ui_resource_->lost_resource_count);
- // Resource Id on the impl-side have been recreated as well. Note
- // that the same UIResourceId persists after the context lost.
- if (!layer_tree_host()->settings().impl_side_painting)
- EXPECT_NE(0u, impl->ResourceIdForUIResource(ui_resource_->id()));
- PostSetNeedsCommitToMainThread();
+ // Lose the context on the impl thread before the commit.
+ PostLoseContextToImplThread();
break;
case 2:
// Sequence 2:
@@ -1748,21 +1786,10 @@ class UIResourceLostBeforeCommit : public UIResourceLostTest {
test_id1_ = ui_resource_->id();
// Sanity check that two resource creations return different ids.
EXPECT_NE(test_id0_, test_id1_);
- // Lose the context before commit.
- LoseContext();
+ // Lose the context on the impl thread before the commit.
+ PostLoseContextToImplThread();
break;
case 3:
- if (!layer_tree_host()->settings().impl_side_painting) {
- // The previous resource should have been deleted.
- EXPECT_EQ(0u, impl->ResourceIdForUIResource(test_id0_));
- // The second resource should have been created.
- EXPECT_NE(0u, impl->ResourceIdForUIResource(test_id1_));
- }
-
- // The second resource called the resource callback once and since the
- // context is lost, a "resource lost" callback was also issued.
- EXPECT_EQ(2, ui_resource_->resource_create_count);
- EXPECT_EQ(1, ui_resource_->lost_resource_count);
// Clear the manager of resources.
ui_resource_.reset();
PostSetNeedsCommitToMainThread();
@@ -1777,45 +1804,79 @@ class UIResourceLostBeforeCommit : public UIResourceLostTest {
// destructor (so usually ui_resource_.reset()). But here we need
// ui_resource_ for the next step, so call DeleteUIResource directly.
layer_tree_host()->DeleteUIResource(test_id0_);
- LoseContext();
+ // Delete the resouce and then lose the context.
+ PostLoseContextToImplThread();
break;
case 5:
- // Expect the resource callback to have been called once.
- EXPECT_EQ(1, ui_resource_->resource_create_count);
- // No "resource lost" callbacks.
- EXPECT_EQ(0, ui_resource_->lost_resource_count);
- if (!layer_tree_host()->settings().impl_side_painting) {
- // The UI resource id should not be valid
- EXPECT_EQ(0u, impl->ResourceIdForUIResource(test_id0_));
- }
- PostSetNeedsCommitToMainThread();
- break;
- case 6:
+ // Release resource before ending the test.
ui_resource_.reset();
EndTest();
break;
+ case 6:
+ // Single thread proxy issues extra commits after context lost.
+ // http://crbug.com/287250
+ if (HasImplThread())
+ NOTREACHED();
+ break;
+ case 8:
+ NOTREACHED();
}
}
- virtual void DidActivateTreeOnThread(LayerTreeHostImpl* impl) OVERRIDE {
- LayerTreeHostContextTest::DidActivateTreeOnThread(impl);
+ virtual void StepCompleteOnImplThread(LayerTreeHostImpl* impl) OVERRIDE {
+ LayerTreeHostContextTest::CommitCompleteOnThread(impl);
switch (time_step_) {
case 1:
- if (layer_tree_host()->settings().impl_side_painting)
- EXPECT_NE(0u, impl->ResourceIdForUIResource(ui_resource_->id()));
+ // Sequence 1 (continued):
+ if (HasImplThread()) {
+ // The resources should have been recreated.
+ EXPECT_EQ(2, ui_resource_->resource_create_count);
+ // The "resource lost" callback was called once for the resource in
+ // the resource map.
+ EXPECT_EQ(1, ui_resource_->lost_resource_count);
+ } else {
+ // The extra commit that happens at context lost in the single thread
+ // proxy changes the timing so that the resource recreation callback
+ // is skipped.
+ // http://crbug.com/287250
+ EXPECT_EQ(1, ui_resource_->resource_create_count);
+ EXPECT_EQ(0, ui_resource_->lost_resource_count);
+ }
+ // Resource Id on the impl-side have been recreated as well. Note
+ // that the same UIResourceId persists after the context lost.
+ EXPECT_NE(0u, impl->ResourceIdForUIResource(ui_resource_->id()));
+ PostSetNeedsCommitToMainThread();
break;
case 3:
- if (layer_tree_host()->settings().impl_side_painting) {
- EXPECT_EQ(0u, impl->ResourceIdForUIResource(test_id0_));
- EXPECT_NE(0u, impl->ResourceIdForUIResource(test_id1_));
+ // Sequence 2 (continued):
+ // The previous resource should have been deleted.
+ EXPECT_EQ(0u, impl->ResourceIdForUIResource(test_id0_));
+ // The second resource should have been created.
+ EXPECT_NE(0u, impl->ResourceIdForUIResource(test_id1_));
+ if (HasImplThread()) {
+ // The second resource called the resource callback once and since the
+ // context is lost, a "resource lost" callback was also issued.
+ EXPECT_EQ(2, ui_resource_->resource_create_count);
+ EXPECT_EQ(1, ui_resource_->lost_resource_count);
+ } else {
+ // The extra commit that happens at context lost in the single thread
+ // proxy changes the timing so that the resource recreation callback
+ // is skipped.
+ // http://crbug.com/287250
+ EXPECT_EQ(1, ui_resource_->resource_create_count);
+ EXPECT_EQ(0, ui_resource_->lost_resource_count);
}
break;
case 5:
- if (layer_tree_host()->settings().impl_side_painting)
- EXPECT_EQ(0u, impl->ResourceIdForUIResource(test_id0_));
+ // Sequence 3 (continued):
+ // Expect the resource callback to have been called once.
+ EXPECT_EQ(1, ui_resource_->resource_create_count);
+ // No "resource lost" callbacks.
+ EXPECT_EQ(0, ui_resource_->lost_resource_count);
+ // The UI resource id should not be valid
+ EXPECT_EQ(0u, impl->ResourceIdForUIResource(test_id0_));
break;
}
- ++time_step_;
}
private:
@@ -1828,34 +1889,43 @@ SINGLE_AND_MULTI_THREAD_TEST_F(UIResourceLostBeforeCommit);
// Losing UI resource before the pending trees is activated but after the
// commit. Impl-side-painting only.
class UIResourceLostBeforeActivateTree : public UIResourceLostTest {
- virtual void CommitCompleteOnThread(LayerTreeHostImpl* impl) OVERRIDE {
- LayerTreeHostContextTest::CommitCompleteOnThread(impl);
- switch (time_step_) {
+ virtual void StepCompleteOnMainThread(int step) OVERRIDE {
+ EXPECT_TRUE(layer_tree_host()->proxy()->IsMainThread());
+ switch (step) {
case 0:
ui_resource_ = FakeScopedUIResource::Create(layer_tree_host());
PostSetNeedsCommitToMainThread();
break;
- case 2:
- PostSetNeedsCommitToMainThread();
- break;
case 3:
test_id_ = ui_resource_->id();
ui_resource_.reset();
PostSetNeedsCommitToMainThread();
break;
- case 4:
- PostSetNeedsCommitToMainThread();
- break;
case 5:
+ // Release resource before ending the test.
+ ui_resource_.reset();
EndTest();
break;
+ case 6:
+ // Make sure no extra commits happened.
+ NOTREACHED();
}
}
- virtual void WillActivateTreeOnThread(LayerTreeHostImpl* impl) OVERRIDE {
+ virtual void CommitCompleteOnThread(LayerTreeHostImpl* impl) OVERRIDE {
+ LayerTreeHostContextTest::CommitCompleteOnThread(impl);
switch (time_step_) {
- case 0:
+ case 2:
+ PostSetNeedsCommitToMainThread();
+ break;
+ case 4:
+ PostSetNeedsCommitToMainThread();
break;
+ }
+ }
+
+ virtual void WillActivateTreeOnThread(LayerTreeHostImpl* impl) OVERRIDE {
+ switch (time_step_) {
case 1:
// The resource creation callback has been called.
EXPECT_EQ(1, ui_resource_->resource_create_count);
@@ -1888,6 +1958,8 @@ class UIResourceLostBeforeActivateTree : public UIResourceLostTest {
EXPECT_EQ(0u, impl->ResourceIdForUIResource(test_id_));
break;
}
+
+ PostStepCompleteToMainThread();
++time_step_;
}
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698