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

Issue 603181 link

Starred by 2 users

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

Replace CallFunctionForEveryLayer with LayerListIterator

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

Issue description

This includes two changes:

1) Use LayerListIterator on main thread;

2) Remove CallFunctionForEveryLayer for both main and impl, replace them with the iterator.


 

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

Blocking: 603613
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 14 2016

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

commit ad45a5f2de581606e585b21a120f61a159de4550
Author: rjkroege <rjkroege@chromium.org>
Date: Thu Apr 14 23:43:46 2016

Revert of cc: Add a main thread LayerListIterator (patchset #3 id:40001 of https://codereview.chromium.org/1887703002/ )

Reason for revert:
Per: https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/47790,

I think that this CL breaks cc unit tests.

[ RUN      ] LayerTreeHostCopyRequestTestLostOutputSurface.RunMultiThread_DirectRenderer

c:\b\build\slave\win_builder__dbg_\build\src\cc\test\layer_tree_test.cc(899): error: Failed

Test timed out

[  FAILED  ] LayerTreeHostCopyRequestTestLostOutputSurface.RunMultiThread_DirectRenderer (5006 ms)

Original issue's description:
> cc: Add a main thread LayerListIterator
>
> Make LayerListIterator a template class for Layer and LayerImpl.
>
> Add tests for LayerListIterator<Layer>.
>
> BUG= 603181 
> CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
>
> Committed: https://crrev.com/7a1807b49236a338953fea5e7f7f6f6f9cbb0aa8
> Cr-Commit-Position: refs/heads/master@{#387422}

TBR=ajuma@chromium.org,vollick@chromium.org,jaydasika@chromium.org,sunxd@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 603181 

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

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

[modify] https://crrev.com/ad45a5f2de581606e585b21a120f61a159de4550/cc/layers/layer_list_iterator.cc
[modify] https://crrev.com/ad45a5f2de581606e585b21a120f61a159de4550/cc/layers/layer_list_iterator.h
[modify] https://crrev.com/ad45a5f2de581606e585b21a120f61a159de4550/cc/layers/layer_list_iterator_unittest.cc
[modify] https://crrev.com/ad45a5f2de581606e585b21a120f61a159de4550/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/ad45a5f2de581606e585b21a120f61a159de4550/cc/trees/layer_tree_host.h
[modify] https://crrev.com/ad45a5f2de581606e585b21a120f61a159de4550/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/ad45a5f2de581606e585b21a120f61a159de4550/cc/trees/layer_tree_impl.h

Comment 4 by sunxd@chromium.org, Apr 16 2016

The test seems flacky: it failed before this CL (https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/47743), I will try to re-commit on Monday and see if it breaks again.

I think we need a patch to fix the flacky test.
Project Member

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

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

commit 0e1785c782ce5d569d21beb8770e7a204a2729a2
Author: sunxd <sunxd@chromium.org>
Date: Mon Apr 18 15:10:11 2016

cc: Add a main thread LayerListIterator

Make LayerListIterator a template class for Layer and LayerImpl.

Add tests for LayerListIterator<Layer>.

Noted that cc_unittests:
LayerTreeHostCopyRequestTestLostOutputSurface.
RunMultiThread_DirectRenderer might be flacky.

BUG= 603181 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/7a1807b49236a338953fea5e7f7f6f6f9cbb0aa8
Cr-Commit-Position: refs/heads/master@{#387422}

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

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

[modify] https://crrev.com/0e1785c782ce5d569d21beb8770e7a204a2729a2/cc/layers/layer_list_iterator.cc
[modify] https://crrev.com/0e1785c782ce5d569d21beb8770e7a204a2729a2/cc/layers/layer_list_iterator.h
[modify] https://crrev.com/0e1785c782ce5d569d21beb8770e7a204a2729a2/cc/layers/layer_list_iterator_unittest.cc
[modify] https://crrev.com/0e1785c782ce5d569d21beb8770e7a204a2729a2/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/0e1785c782ce5d569d21beb8770e7a204a2729a2/cc/trees/layer_tree_host.h
[modify] https://crrev.com/0e1785c782ce5d569d21beb8770e7a204a2729a2/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/0e1785c782ce5d569d21beb8770e7a204a2729a2/cc/trees/layer_tree_impl.h

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

The patch didn't trigger another failure of the test: https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/47876

I believe the test is flacky, might worth to open another bug for it.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 18 2016

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

commit 3cbb6e9a48bd1579069eabcc7d3cc2ff9ef21cad
Author: sunxd <sunxd@chromium.org>
Date: Mon Apr 18 17:17:39 2016

Revert of cc: Add a main thread LayerListIterator (patchset #6 id:100001 of https://codereview.chromium.org/1887703002/ )

Reason for revert:
Possible failure to android_clang_buildbot (https://uberchromegw.corp.google.com/i/chromium.linux/builders/Android%20Clang%20Builder%20(dbg)/builds/78715/steps/compile/logs/stdio)

Original issue's description:
> cc: Add a main thread LayerListIterator
>
> Make LayerListIterator a template class for Layer and LayerImpl.
>
> Add tests for LayerListIterator<Layer>.
>
> Noted that cc_unittests:
> LayerTreeHostCopyRequestTestLostOutputSurface.
> RunMultiThread_DirectRenderer might be flacky.
>
> BUG= 603181 
> CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
>
> Committed: https://crrev.com/7a1807b49236a338953fea5e7f7f6f6f9cbb0aa8
> Cr-Commit-Position: refs/heads/master@{#387422}
>
> Committed: https://crrev.com/0e1785c782ce5d569d21beb8770e7a204a2729a2
> Cr-Commit-Position: refs/heads/master@{#387908}

TBR=ajuma@chromium.org,vollick@chromium.org,jaydasika@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 603181 

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

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

[modify] https://crrev.com/3cbb6e9a48bd1579069eabcc7d3cc2ff9ef21cad/cc/layers/layer_list_iterator.cc
[modify] https://crrev.com/3cbb6e9a48bd1579069eabcc7d3cc2ff9ef21cad/cc/layers/layer_list_iterator.h
[modify] https://crrev.com/3cbb6e9a48bd1579069eabcc7d3cc2ff9ef21cad/cc/layers/layer_list_iterator_unittest.cc
[modify] https://crrev.com/3cbb6e9a48bd1579069eabcc7d3cc2ff9ef21cad/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/3cbb6e9a48bd1579069eabcc7d3cc2ff9ef21cad/cc/trees/layer_tree_host.h
[modify] https://crrev.com/3cbb6e9a48bd1579069eabcc7d3cc2ff9ef21cad/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/3cbb6e9a48bd1579069eabcc7d3cc2ff9ef21cad/cc/trees/layer_tree_impl.h

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 18 2016

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

commit f354ca2b2497a54b41591a7856fa0dde445d7beb
Author: sunxd <sunxd@chromium.org>
Date: Mon Apr 18 18:16:25 2016

cc: Add a main thread LayerListIterator

Make LayerListIterator a template class for Layer and LayerImpl.

Add tests for LayerListIterator<Layer>.

Noted that cc_unittests:
LayerTreeHostCopyRequestTestLostOutputSurface.
RunMultiThread_DirectRenderer might be flacky.

BUG= 603181 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/7a1807b49236a338953fea5e7f7f6f6f9cbb0aa8
Cr-Commit-Position: refs/heads/master@{#387422}

Committed: https://crrev.com/0e1785c782ce5d569d21beb8770e7a204a2729a2
Cr-Commit-Position: refs/heads/master@{#387908}

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

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

[modify] https://crrev.com/f354ca2b2497a54b41591a7856fa0dde445d7beb/cc/layers/layer_list_iterator.cc
[modify] https://crrev.com/f354ca2b2497a54b41591a7856fa0dde445d7beb/cc/layers/layer_list_iterator.h
[modify] https://crrev.com/f354ca2b2497a54b41591a7856fa0dde445d7beb/cc/layers/layer_list_iterator_unittest.cc
[modify] https://crrev.com/f354ca2b2497a54b41591a7856fa0dde445d7beb/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/f354ca2b2497a54b41591a7856fa0dde445d7beb/cc/trees/layer_tree_host.h
[modify] https://crrev.com/f354ca2b2497a54b41591a7856fa0dde445d7beb/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/f354ca2b2497a54b41591a7856fa0dde445d7beb/cc/trees/layer_tree_impl.h

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

Blocking: -603613
Project Member

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

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

commit 859104151c4a26dc2c9d1e3337253ef8ddc126f3
Author: sunxd <sunxd@chromium.org>
Date: Mon Apr 25 16:07:17 2016

cc: Make CallFunctionForEveryLayer use LayerListIterator

Now that we have a main thread and impl thread layer list iterator. We
should use the iterator to perform those methods on every layer. The old
traversal way should be abandoned.

CallFunctionForEveryLayer is expected to call function for all mask_layers
and replica_layers.

BUG= 603181 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

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

[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/debug/debug_rect_history.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/debug/invalidation_benchmark.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/debug/rasterize_and_record_benchmark.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/debug/rasterize_and_record_benchmark_impl.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/layers/layer_proto_converter.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/trees/draw_property_utils.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/trees/layer_tree_host_common.h
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/trees/layer_tree_host_unittest_serialization.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/trees/layer_tree_impl.cc

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

Status: Fixed (was: Assigned)
Project Member

Comment 12 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/+/859104151c4a26dc2c9d1e3337253ef8ddc126f3

commit 859104151c4a26dc2c9d1e3337253ef8ddc126f3
Author: sunxd <sunxd@chromium.org>
Date: Mon Apr 25 16:07:17 2016

cc: Make CallFunctionForEveryLayer use LayerListIterator

Now that we have a main thread and impl thread layer list iterator. We
should use the iterator to perform those methods on every layer. The old
traversal way should be abandoned.

CallFunctionForEveryLayer is expected to call function for all mask_layers
and replica_layers.

BUG= 603181 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

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

[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/debug/debug_rect_history.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/debug/invalidation_benchmark.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/debug/rasterize_and_record_benchmark.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/debug/rasterize_and_record_benchmark_impl.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/layers/layer_proto_converter.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/trees/draw_property_utils.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/trees/layer_tree_host_common.h
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/trees/layer_tree_host_unittest_serialization.cc
[modify] https://crrev.com/859104151c4a26dc2c9d1e3337253ef8ddc126f3/cc/trees/layer_tree_impl.cc

Sign in to add a comment