Enable some remote LTH tests in layer_tree_host_unittests.

cc/trees/layer_tree_host_unittest.cc

Index: cc/trees/layer_tree_host_unittest.cc
diff --git a/cc/trees/layer_tree_host_unittest.cc b/cc/trees/layer_tree_host_unittest.cc
index f6c81048aff7704e9d78fa63692e008fa462cbf6..ca0d0b994407ec7584185e4646d025d1d0c69f36 100644
--- a/cc/trees/layer_tree_host_unittest.cc
+++ b/cc/trees/layer_tree_host_unittest.cc
@@ -81,22 +81,22 @@ class LayerTreeHostTest : public LayerTreeTest {};
 
 class LayerTreeHostTestHasImplThreadTest : public LayerTreeHostTest {
  public:
-  LayerTreeHostTestHasImplThreadTest() : threaded_(false) {}
+  LayerTreeHostTestHasImplThreadTest() : single_threaded_(false) {}
 
   void RunTest(CompositorMode mode) override {
-    threaded_ = mode == CompositorMode::THREADED;
+    single_threaded_ = mode == CompositorMode::SINGLE_THREADED;
     LayerTreeHostTest::RunTest(mode);
   }
 
   void BeginTest() override {
-    EXPECT_EQ(threaded_, HasImplThread());
+    EXPECT_EQ(single_threaded_, !HasImplThread());
     EndTest();
   }
 
-  void AfterTest() override { EXPECT_EQ(threaded_, HasImplThread()); }
+  void AfterTest() override { EXPECT_EQ(single_threaded_, !HasImplThread()); }
 
  private:
-  bool threaded_;
+  bool single_threaded_;
 };
 
 SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestHasImplThreadTest);

[Khushal, 2016/10/14 20:12:31]

Looks like we didn't actually enable it for remote. :P

[xingliu, 2016/10/17 16:26:49]

My bad. Done.

@@ -118,7 +118,7 @@ class LayerTreeHostTestSetNeedsCommitInsideLayout : public LayerTreeHostTest {
   void AfterTest() override {}
 };
 
-SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestSetNeedsCommitInsideLayout);
+SINGLE_MULTI_AND_REMOTE_TEST_F(LayerTreeHostTestSetNeedsCommitInsideLayout);
 
 class LayerTreeHostTestFrameOrdering : public LayerTreeHostTest {
  protected:
@@ -193,7 +193,7 @@ class LayerTreeHostTestFrameOrdering : public LayerTreeHostTest {
   ImplOrder impl_ = IMPL_START;
 };
 
-SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestFrameOrdering);
+SINGLE_MULTI_AND_REMOTE_TEST_F(LayerTreeHostTestFrameOrdering);
 
 class LayerTreeHostTestSetNeedsUpdateInsideLayout : public LayerTreeHostTest {
  protected:
@@ -212,7 +212,7 @@ class LayerTreeHostTestSetNeedsUpdateInsideLayout : public LayerTreeHostTest {
   void AfterTest() override {}
 };
 
-SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestSetNeedsUpdateInsideLayout);
+SINGLE_MULTI_AND_REMOTE_TEST_F(LayerTreeHostTestSetNeedsUpdateInsideLayout);
 
 // Test if the LTHI receives ReadyToActivate notifications from the TileManager
 // when no raster tasks get scheduled.
@@ -256,7 +256,7 @@ class LayerTreeHostTestReadyToActivateEmpty : public LayerTreeHostTest {
   size_t required_for_activation_count_;
 };
 
-SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestReadyToActivateEmpty);
+SINGLE_MULTI_AND_REMOTE_TEST_F(LayerTreeHostTestReadyToActivateEmpty);
 
 // Test if the LTHI receives ReadyToActivate notifications from the TileManager
 // when some raster tasks flagged as REQUIRED_FOR_ACTIVATION got scheduled.
@@ -285,9 +285,9 @@ class LayerTreeHostTestReadyToActivateNonEmpty
   FakeContentLayerClient client_;
 };
 
-// Multi-thread only because in single thread the commit goes directly to the
-// active tree, so notify ready to activate is skipped.
-MULTI_THREAD_TEST_F(LayerTreeHostTestReadyToActivateNonEmpty);
+// No single thread test because the commit goes directly to the active tree in
+// single thread mode, so notify ready to activate is skipped.
+REMOTE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestReadyToActivateNonEmpty);
 
 // Test if the LTHI receives ReadyToDraw notifications from the TileManager when
 // no raster tasks get scheduled.
@@ -327,7 +327,7 @@ class LayerTreeHostTestReadyToDrawEmpty : public LayerTreeHostTest {
   size_t required_for_draw_count_;
 };
 
-SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestReadyToDrawEmpty);
+SINGLE_MULTI_AND_REMOTE_TEST_F(LayerTreeHostTestReadyToDrawEmpty);
 
 // Test if the LTHI receives ReadyToDraw notifications from the TileManager when
 // some raster tasks flagged as REQUIRED_FOR_DRAW got scheduled.
@@ -654,7 +654,7 @@ class LayerTreeHostTestSetNeedsCommit1 : public LayerTreeHostTest {
   int num_draws_;
 };
 
-SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestSetNeedsCommit1);
+SINGLE_MULTI_AND_REMOTE_TEST_F(LayerTreeHostTestSetNeedsCommit1);
 
 // A SetNeedsCommit should lead to 1 commit. Issuing a second commit after that
 // first committed frame draws should lead to another commit.
@@ -690,7 +690,7 @@ class LayerTreeHostTestSetNeedsCommit2 : public LayerTreeHostTest {
   int num_draws_;
 };
 
-MULTI_THREAD_TEST_F(LayerTreeHostTestSetNeedsCommit2);
+REMOTE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestSetNeedsCommit2);
 
 // Verify that we pass property values in PushPropertiesTo.
 class LayerTreeHostTestPushPropertiesTo : public LayerTreeHostTest {
@@ -785,7 +785,7 @@ class LayerTreeHostTestPushPropertiesTo : public LayerTreeHostTest {
   int index_;
 };
 
-SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestPushPropertiesTo);
+SINGLE_MULTI_AND_REMOTE_TEST_F(LayerTreeHostTestPushPropertiesTo);
 
 class LayerTreeHostTestPushNodeOwnerToNodeIdMap : public LayerTreeHostTest {
  protected:
@@ -907,6 +907,9 @@ class LayerTreeHostTestPushNodeOwnerToNodeIdMap : public LayerTreeHostTest {
   scoped_refptr<Layer> child_;
 };
 
+// The remote LTH doesn't build property tree, see if we need to
+// compare the property trees between LTH in process and impl side tree for
+// remote LTH test. See crbug/655795.
 SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestPushNodeOwnerToNodeIdMap);

[Khushal, 2016/10/14 20:12:31]

Can you make the comment a bit more descriptive about what the problem is?

"This test compares the value of tree indexes from Layers on the engine to the
resulting PropertyTrees copied to the pending tree after the commit on the
client. This will result in a failure since while the client side Layers would
have the correct values for these indexes, the engine side Layers always hold
uninitialized indexes since PropertyTrees are never built on the engine. See
crbug.com/655795."

[xingliu, 2016/10/17 16:26:49]

Done.

 
 class LayerTreeHostTestSurfaceDamage : public LayerTreeHostTest {
@@ -988,6 +991,8 @@ class LayerTreeHostTestSurfaceDamage : public LayerTreeHostTest {
   scoped_refptr<Layer> grand_child_;
 };
 
+// Remote LTH test always re-sync tree hierachy, so render surface will always
+// be damaged, see crbug/605170.
 SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestSurfaceDamage);

[Khushal, 2016/10/14 20:12:31]

Again, a little more descriptive comment helps in addressing this later. :)

"This test changes properties on the Layer and ensures that the subtree damage
is tracked correctly on the resulting RenderSurfaceImpl for the corresponding
LayerImpl. Since the remote code path currently synchronizes the hierarchy
between the engine and client for every frame, all Layers on the client end up
being marked as damaged.
Enable this when crbug.com/605170 is fixed."

[xingliu, 2016/10/17 16:26:49]

Done.

 
 // Verify damage status of property trees is preserved after commit.
@@ -1199,6 +1204,8 @@ class LayerTreeHostTestEffectTreeSync : public LayerTreeHostTest {
   FilterOperations sepia_filter_;
 };
 
+// Remote LTH won't work because it doesn't build property tree. See
+// crbug/655795.
 SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestEffectTreeSync);

[Khushal, 2016/10/14 20:12:32]

Update the comment,
"This test verifies that correct values are retained on the impl thread in cases
where they are animated on that thread. Since remote mode doesn't support
threaded animations, we don't need to run this in remote mode."

Same comment for TransformTreeSync.

 
 class LayerTreeHostTestTransformTreeSync : public LayerTreeHostTest {
@@ -1276,6 +1283,8 @@ class LayerTreeHostTestTransformTreeSync : public LayerTreeHostTest {
   scoped_refptr<Layer> root_;
 };
 
+// Remote LTH won't work because it doesn't build property tree. See
+// crbug/655795.
 SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestTransformTreeSync);
 
 // Verify damage status is updated even when the transform tree doesn't need
@@ -1350,6 +1359,7 @@ class LayerTreeHostTestTransformTreeDamageIsUpdated : public LayerTreeHostTest {
   scoped_refptr<Layer> grand_child_;
 };
 
+// LTH remote test is not needed since we don't do animation.

[Khushal, 2016/10/14 20:12:31]

And here.

 SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestTransformTreeDamageIsUpdated);
 
 // Test that when mask layers switches layers, this gets pushed onto impl.
@@ -1420,6 +1430,9 @@ class LayerTreeHostTestSwitchMaskLayer : public LayerTreeHostTest {
   int index_;
 };
 
+// Remote LTH won't work because we update all layers in BeginMainFrame, some
+// layers are not pushed into the update list in LTH in process.
+// see crbug.com/650885.
 SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestSwitchMaskLayer);

[Khushal, 2016/10/14 20:12:32]

"This test also verifies that the Layers updated in a main frame correspond only
to the Layers that need updates computed using
draw_property_utils::FindLayersThatNeedUpdates. Since LayerTreeHostRemote
currently updates all Layers during the main frame, the test fails for remote
mode.
TODO(xingliu): Revisit enabling this when crbug.com/650885 is resolved."

[xingliu, 2016/10/17 16:26:49]

Done.

 
 // 1 setNeedsRedraw after the first commit has completed should lead to 1
@@ -1456,7 +1469,7 @@ class LayerTreeHostTestSetNeedsRedraw : public LayerTreeHostTest {
   int num_draws_;
 };
 
-MULTI_THREAD_TEST_F(LayerTreeHostTestSetNeedsRedraw);
+REMOTE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestSetNeedsRedraw);
 
 // After setNeedsRedrawRect(invalid_rect) the final damage_rect
 // must contain invalid_rect.
@@ -1514,7 +1527,7 @@ class LayerTreeHostTestSetNeedsRedrawRect : public LayerTreeHostTest {
   scoped_refptr<FakePictureLayer> root_layer_;
 };
 
-SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestSetNeedsRedrawRect);
+SINGLE_MULTI_AND_REMOTE_TEST_F(LayerTreeHostTestSetNeedsRedrawRect);
 
 // Ensure the texture size of the pending and active trees are identical when a
 // layer is not in the viewport and a resize happens on the viewport