New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 604467 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature

Blocking:
issue 557194



Sign in to add a comment

Use LayerById instead of tree hierarchy information in LayerTreeTest's descendants for impl thread

Project Member Reported by sunxd@chromium.org, Apr 18 2016

Issue description

In LayerTreeTest's descendants, some of the tests push a layer tree from main thread to impl thread, and try to access an individual layer by children pointer. This should be converted to access the layer by layer_id, as we expect TreeSynchronizer to pass a layer list to impl thread.
 

Comment 1 by sunxd@chromium.org, Apr 18 2016

Summary: Use LayerById instead of tree hierarchy information in LayerTreeTest's descendants for impl thread (was: Use LayerById instead of tree hierarchy information in LayerTreeTest's descendants)

Comment 2 by sunxd@chromium.org, Apr 20 2016

Here are all the files that refer to a descendant of LayerTreeTest:
	src/cc/layers/heads_up_display_unittest.cc 
	src/cc/trees/layer_tree_host_unittest_animation.cc
	src/cc/trees/layer_tree_host_common_perftest.cc
	src/cc/trees/layer_tree_host_unittest_context.cc
	src/cc/trees/layer_tree_host_unittest_copyrequest.cc
	src/cc/trees/layer_tree_host_unittest_damage.cc
	src/cc/trees/layer_tree_host_unittest_occlusion.cc
	src/cc/trees/layer_tree_host_perftest.cc
	src/cc/trees/layer_tree_host_unittest_picture.cc
	src/cc/trees/layer_tree_host_unittest_scroll.cc
	src/cc/trees/layer_tree_host_unittest.cc
------------------above taken by sunxd------------------------
	src/cc/trees/layer_tree_host_unittest_video.cc
	src/cc/layers/scrollbar_layer_unittest.cc
	src/cc/layers/surface_layer_unittest.cc
	src/cc/layers/texture_layer_unittest.cc
	src/cc/output/delegating_renderer_unittest.cc
	src/cc/test/layer_tree_pixel_test.h
	src/cc/trees/layer_tree_host_unittest_proxy.cc
	src/cc/trees/remote_channel_unittest.cc
	src/cc/trees/threaded_channel_unittest.cc

Comment 3 by sunxd@chromium.org, Apr 20 2016

These are not reference, sorry, these are subclasses.
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bd6f15aa1d4256566d77feac67b1cc2ef211b879

commit bd6f15aa1d4256566d77feac67b1cc2ef211b879
Author: jaydasika <jaydasika@chromium.org>
Date: Thu Apr 21 19:45:37 2016

cc : Replace LayerImpl::children calls with LayerById

Replacing because these tests expect layer tree hierarchy to be pushed
during commit and we won't be doing this after switching to layer lists.

BUG= 604467 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

Cr-Commit-Position: refs/heads/master@{#388860}

[modify] https://crrev.com/bd6f15aa1d4256566d77feac67b1cc2ef211b879/cc/layers/scrollbar_layer_unittest.cc
[modify] https://crrev.com/bd6f15aa1d4256566d77feac67b1cc2ef211b879/cc/layers/texture_layer_unittest.cc
[modify] https://crrev.com/bd6f15aa1d4256566d77feac67b1cc2ef211b879/cc/trees/layer_tree_host_unittest_video.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ae6803fb4dcb9a40bc50d889c349869c3dcd83c

commit 1ae6803fb4dcb9a40bc50d889c349869c3dcd83c
Author: sunxd <sunxd@chromium.org>
Date: Thu Apr 21 20:15:12 2016

cc: Remove LayerImpl::children() calls from descendants of LayerTreeTest

In LayerTreeTest, TreeSynchronizer pushes the layer structures from main
thread to impl thread. In the SPv2 plan, we will eventually have no tree
hierarchy on impl side. This means that the synchronizer no longer pushes
a tree structure, instead it pushes a list. We need to make the unit tests
use other ways to access layer_impls in the pushed impl tree.

BUG= 604467 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

Cr-Commit-Position: refs/heads/master@{#388875}

[modify] https://crrev.com/1ae6803fb4dcb9a40bc50d889c349869c3dcd83c/cc/trees/layer_tree_host_common_perftest.cc
[modify] https://crrev.com/1ae6803fb4dcb9a40bc50d889c349869c3dcd83c/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/1ae6803fb4dcb9a40bc50d889c349869c3dcd83c/cc/trees/layer_tree_host_unittest_animation.cc
[modify] https://crrev.com/1ae6803fb4dcb9a40bc50d889c349869c3dcd83c/cc/trees/layer_tree_host_unittest_context.cc
[modify] https://crrev.com/1ae6803fb4dcb9a40bc50d889c349869c3dcd83c/cc/trees/layer_tree_host_unittest_copyrequest.cc
[modify] https://crrev.com/1ae6803fb4dcb9a40bc50d889c349869c3dcd83c/cc/trees/layer_tree_host_unittest_damage.cc
[modify] https://crrev.com/1ae6803fb4dcb9a40bc50d889c349869c3dcd83c/cc/trees/layer_tree_host_unittest_occlusion.cc
[modify] https://crrev.com/1ae6803fb4dcb9a40bc50d889c349869c3dcd83c/cc/trees/layer_tree_host_unittest_picture.cc
[modify] https://crrev.com/1ae6803fb4dcb9a40bc50d889c349869c3dcd83c/cc/trees/layer_tree_host_unittest_scroll.cc

Comment 6 by sunxd@chromium.org, Apr 21 2016

I will verify if there is still any leftovers.

Comment 7 by ajuma@chromium.org, Apr 21 2016

LayerPositionConstraintTest also constructs a main-thread Layer tree and then access compositor layers using LayerImpl::children().

Comment 8 by sunxd@chromium.org, Apr 22 2016

layer_position_constraint_unittest.cc
damage_tracker_unittest.cc (traverse -> iterate)
layer_tree_host_impl_unittest.cc (traverse -> iterate)
layer_tree_host_unittest.cc:3798
tree_synchronizer_unittest.cc

Among them, it seems better that tree_synchronizer_unittest changes happen after we  push a layer list from main thread to impl thread.
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 23 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b2e13c9aac09a04f2bf498ead77ffa741d2c9eea

commit b2e13c9aac09a04f2bf498ead77ffa741d2c9eea
Author: sunxd <sunxd@chromium.org>
Date: Sat Apr 23 01:00:05 2016

cc: Remove LayerImpl::children() calls after tree synchronization

As described in crrev.com/1905713002, we need to remove children calls
because tree synchronization will push a layer list instead of layer
tree in the future.

This CL removes the leftover callers of that CL.

BUG= 604467 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

Cr-Commit-Position: refs/heads/master@{#389333}

[modify] https://crrev.com/b2e13c9aac09a04f2bf498ead77ffa741d2c9eea/cc/layers/layer_position_constraint_unittest.cc
[modify] https://crrev.com/b2e13c9aac09a04f2bf498ead77ffa741d2c9eea/cc/trees/damage_tracker_unittest.cc
[modify] https://crrev.com/b2e13c9aac09a04f2bf498ead77ffa741d2c9eea/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/b2e13c9aac09a04f2bf498ead77ffa741d2c9eea/cc/trees/layer_tree_host_unittest.cc

Comment 10 by sunxd@chromium.org, Apr 25 2016

Status: Fixed (was: Assigned)
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 25 2016

Labels: merge-merged-2716
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b2e13c9aac09a04f2bf498ead77ffa741d2c9eea

commit b2e13c9aac09a04f2bf498ead77ffa741d2c9eea
Author: sunxd <sunxd@chromium.org>
Date: Sat Apr 23 01:00:05 2016

cc: Remove LayerImpl::children() calls after tree synchronization

As described in crrev.com/1905713002, we need to remove children calls
because tree synchronization will push a layer list instead of layer
tree in the future.

This CL removes the leftover callers of that CL.

BUG= 604467 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

Cr-Commit-Position: refs/heads/master@{#389333}

[modify] https://crrev.com/b2e13c9aac09a04f2bf498ead77ffa741d2c9eea/cc/layers/layer_position_constraint_unittest.cc
[modify] https://crrev.com/b2e13c9aac09a04f2bf498ead77ffa741d2c9eea/cc/trees/damage_tracker_unittest.cc
[modify] https://crrev.com/b2e13c9aac09a04f2bf498ead77ffa741d2c9eea/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/b2e13c9aac09a04f2bf498ead77ffa741d2c9eea/cc/trees/layer_tree_host_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, May 6 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8587a0b3356d604d5f6f382a2a33f8dc31935dbb

commit 8587a0b3356d604d5f6f382a2a33f8dc31935dbb
Author: ajuma <ajuma@chromium.org>
Date: Fri May 06 17:14:31 2016

cc: Remove remaining calls to LayerImpl::child_at in LayerTreeTests

There were two LayerTreeTests still making calls to LayerImpl::child_at.
This CL replaces those calls with calls to LayerTreeImpl::LayerById.

BUG= 604467 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/1952413002
Cr-Commit-Position: refs/heads/master@{#392084}

[modify] https://crrev.com/8587a0b3356d604d5f6f382a2a33f8dc31935dbb/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/8587a0b3356d604d5f6f382a2a33f8dc31935dbb/cc/trees/layer_tree_host_unittest_scroll.cc

Sign in to add a comment