Surface Synchronization: Avoid spurious commits

Prior to this CL, an impl-side child allocated LocalSurfaceId would cause
a spurious commit on the main thread as the new LocalSurfaceId propagates its
way back to the main thread from the parent renderer.

LayerTreeHost::SetLocalSurfaceIdFromParent really only cares about changes
to the parent_sequence_number and embed_token to propagate to the impl thread.
Other changes are irrelevant and should not cause commits.

This CL fixes the issue by early exiting in
LayerTreeHost::SetLocalSurfaceIdFromParent. This problem was cause by looking
at SurfaceId flows. This will likely improve scrolling performance a bit on
Android.

Bug: 672962
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ide4f9df9d89c70216e9a54dfc6ccd298cf046821
Reviewed-on: https://chromium-review.googlesource.com/1126325Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572622}

cc/trees/layer_tree_host.cc

@@ -1212,8 +1212,13 @@ void LayerTreeHost::SetContentSourceId(uint32_t id) {
 
 void LayerTreeHost::SetLocalSurfaceIdFromParent(
     const viz::LocalSurfaceId& local_surface_id_from_parent) {
-  if (local_surface_id_from_parent_ == local_surface_id_from_parent)
+  if (local_surface_id_from_parent_.parent_sequence_number() ==
+          local_surface_id_from_parent.parent_sequence_number() &&
+      local_surface_id_from_parent_.embed_token() ==
+          local_surface_id_from_parent.embed_token()) {
     return;
+  }
+
   TRACE_EVENT_WITH_FLOW2(
       TRACE_DISABLED_BY_DEFAULT("viz.surface_id_flow"),
       "LocalSurfaceId.Submission.Flow",

cc/trees/layer_tree_host_unittest.cc

@@ -7758,6 +7758,48 @@ class LayerTreeHostTestLocalSurfaceId : public LayerTreeHostTest {
 };
 SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestLocalSurfaceId);
 
+// Makes sure that LayerTreeHost does not pick up changes to
+// viz::LocalSurfaceIds that only involve the child sequence number.
+class LayerTreeHostTestLocalSurfaceIdSkipChildNum : public LayerTreeHostTest {
+ protected:
+  void InitializeSettings(LayerTreeSettings* settings) override {
+    settings->enable_surface_synchronization = true;
+  }
+
+  void BeginTest() override {
+    expected_local_surface_id_ = allocator_.GetCurrentLocalSurfaceId();
+    EXPECT_TRUE(child_allocator_.UpdateFromParent(expected_local_surface_id_));
+    child_local_surface_id_ = child_allocator_.GenerateId();
+    EXPECT_NE(expected_local_surface_id_, child_local_surface_id_);
+    PostSetLocalSurfaceIdToMainThread(expected_local_surface_id_);
+    PostSetLocalSurfaceIdToMainThread(child_local_surface_id_);
+  }
+
+  DrawResult PrepareToDrawOnThread(LayerTreeHostImpl* host_impl,
+                                   LayerTreeHostImpl::FrameData* frame_data,
+                                   DrawResult draw_result) override {
+    EXPECT_EQ(DRAW_SUCCESS, draw_result);
+    // We should not be picking up the newer |child_local_surface_id_|.
+    EXPECT_EQ(expected_local_surface_id_,
+              host_impl->active_tree()->local_surface_id_from_parent());
+    return draw_result;
+  }
+
+  void DisplayReceivedLocalSurfaceIdOnThread(
+      const viz::LocalSurfaceId& local_surface_id) override {
+    EXPECT_EQ(expected_local_surface_id_, local_surface_id);
+    EndTest();
+  }
+
+  void AfterTest() override {}
+
+  viz::LocalSurfaceId expected_local_surface_id_;
+  viz::LocalSurfaceId child_local_surface_id_;
+  viz::ParentLocalSurfaceIdAllocator allocator_;
+  viz::ChildLocalSurfaceIdAllocator child_allocator_;
+};
+SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestLocalSurfaceIdSkipChildNum);
+
 // Makes sure that viz::LocalSurfaceId allocation requests propagate all the way
 // to LayerTreeFrameSink.
 class LayerTreeHostTestRequestNewLocalSurfaceId : public LayerTreeHostTest {