New issue
Advanced search Search tips

Issue 682436 link

Starred by 1 user
Status: Fixed
Owner:
khushals...@chromium.org
Closed: Jan 2017
Cc:
danakj@chromium.org
vmp...@chromium.org
enne@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 677556



Sign in to add a comment

CC cleanup list after removing blimp/

Project Member Reported by khushals...@chromium.org, Jan 18 2017 
This is a tracking bug for all cc cleanup to be done after removing code added for blimp/.

I have a patch up which removes all blimp specific code and the serialization code (https://codereview.chromium.org/2646623002). In terms of the refactors we made in cc. There are basically 2 major ones:

1) Adding the LayerTreeHost abstraction and having the default path use LayerTreeHostInProcess.
2) Splitting out LayerTree from LayerTreeHost to isolate all data tracking in the host.

I was thinking about undoing 1) and renaming LTHInProcess back to LTH. I don't feel strongly about 2). I did feel it was better to break that monolithic class but am open to restructuring it or undoing it altogether if people feel that would be better.

Comment 1 by danakj@chromium.org, Jan 18 2017 

For 2, my thoughts are.. I like the splitting of the UIResourceManager and I'd be sad to see it go. We may want to share it between trees one day.

The LayerTree is a singleton member of LayerTreeHost and won't be shared without the remote LTH so is less motivating to keep split to me. Did you see an improvement in ability to test code from the split? The cc::Layer classes still see the LayerTreeHost not just the LayerTree (unlike LayerImpls). Guess I'm wondering what you see as improved here without the motivation to share the code between multiple implementations or instances.

Comment 2 by khushals...@chromium.org, Jan 18 2017 

Good point about the UIResourceManager. I remember sievers@ also suggesting something that required sharing it between hosts.

Actually for the LayerTree, I did make an attempt to decouple Layers from it but I felt I was just adding unnecessary plumbing because most calls end up going to Proxy. So yeah, not much benefit in terms of testing. The only improvement I can think of is that it isolated data bits that we were just plumbing through to the impl side's LayerTree. But with the MutatorHost also there, it doesn't map directly to the LayerTreeImpl either in anyway.

So do we want to keep the UIResourceManager but merge LayerTree back into the host. Does that sound good?

Comment 3 by danakj@chromium.org, Jan 18 2017 

I would be happy with that

Comment 5 by nyquist@chromium.org, Jan 20 2017

Blocking: 677556
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 25 2017 

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

commit e0e4486e66cb8c74dc8616373b1cf598dfca3fe6
Author: khushalsagar <khushalsagar@chromium.org>
Date: Wed Jan 25 03:15:03 2017

cc: Remove the LayerTreeHost abstraction.

The LayerTreeHost abstraction and InProcess implementation was added to
allow blimp to provide a remote implementation. This is no longer needed
so the patch reverts the abstraction to make LayerTreeHost concrete
again and renames LTHInProcess back to LTH.

BUG= 682436 
TBR=pdr@chromium.org, danakj@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/BUILD.gn
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/layers/layer_unittest.cc
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/layers/picture_layer_unittest.cc
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/layers/scrollbar_layer_unittest.cc
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/layers/texture_layer_unittest.cc
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/surfaces/surface_reference_owner.h
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/test/fake_layer_tree_host.cc
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/test/fake_layer_tree_host.h
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/test/layer_tree_test.cc
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/test/layer_tree_test.h
[rename] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/trees/layer_tree_host.h
[delete] https://crrev.com/ad74cd3f5bb1ba01c0767a21edbb88057d4c20e9/cc/trees/layer_tree_host_in_process.h
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/trees/layer_tree_host_unittest_animation.cc
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/trees/layer_tree_host_unittest_scroll.cc
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/trees/proxy_impl.cc
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/trees/proxy_impl.h
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/trees/proxy_main.cc
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/trees/proxy_main.h
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/trees/single_thread_proxy.cc
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/cc/trees/single_thread_proxy.h
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/third_party/WebKit/Source/platform/testing/WebLayerTreeViewImplForTesting.cpp
[modify] https://crrev.com/e0e4486e66cb8c74dc8616373b1cf598dfca3fe6/ui/compositor/compositor.cc
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 26 2017 

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

commit 8476b653814ed190107d9c60e5fb0142be79deb9
Author: khushalsagar <khushalsagar@chromium.org>
Date: Thu Jan 26 18:09:28 2017

cc: Remove internal Inputs struct from LayerTree

This internal division of data was done to allow blimp to easily
identify the data that needed to be serialized. Since that is no longer
necessary, the patch reverts that before merging the LayerTree back
into the host.

BUG= 682436 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

[modify] https://crrev.com/8476b653814ed190107d9c60e5fb0142be79deb9/cc/trees/layer_tree.cc
[modify] https://crrev.com/8476b653814ed190107d9c60e5fb0142be79deb9/cc/trees/layer_tree.h
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 27 2017 

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

commit b69ba945a0cf9cb00e9fe4bec3037d4255a60902
Author: khushalsagar <khushalsagar@chromium.org>
Date: Fri Jan 27 22:20:07 2017

cc: Merge LayerTree into the LayerTreeHost.

The class was split so the code could be shared with the LayerTreeHost
implementation in blimp. Since this is no longer necessary, this merges
the LayerTree back into the host.

BUG= 682436 
TBR=piman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/BUILD.gn
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/debug/invalidation_benchmark.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/debug/invalidation_benchmark.h
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/debug/micro_benchmark.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/debug/micro_benchmark.h
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/debug/micro_benchmark_controller.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/debug/rasterize_and_record_benchmark.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/debug/rasterize_and_record_benchmark.h
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/debug/unittest_only_benchmark.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/debug/unittest_only_benchmark.h
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/layers/heads_up_display_unittest.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/layers/layer.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/layers/layer.h
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/layers/layer_list_iterator_unittest.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/layers/layer_position_constraint_unittest.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/layers/layer_unittest.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/layers/painted_scrollbar_layer.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/layers/picture_layer.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/layers/picture_layer_unittest.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/layers/scrollbar_layer_unittest.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/layers/surface_layer_unittest.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/layers/texture_layer_unittest.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/layers/ui_resource_layer.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/output/bsp_tree_perftest.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/test/fake_layer_tree_host.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/test/fake_layer_tree_host.h
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/test/layer_tree_pixel_test.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/test/layer_tree_test.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/test/layer_tree_test.h
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/test/push_properties_counting_layer.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/draw_property_utils.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/draw_property_utils.h
[delete] https://crrev.com/f998a5e1f13847384ddc3574d549b2759fcca04c/cc/trees/layer_tree.cc
[delete] https://crrev.com/f998a5e1f13847384ddc3574d549b2759fcca04c/cc/trees/layer_tree.h
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host.h
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_common.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_common.h
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_common_perftest.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_perftest.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_pixeltest_filters.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_pixeltest_readback.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_pixeltest_scrollbars.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_pixeltest_tiles.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_unittest_animation.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_unittest_context.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_unittest_copyrequest.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_unittest_damage.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_unittest_occlusion.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_unittest_picture.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_unittest_proxy.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_unittest_scroll.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/layer_tree_host_unittest_video.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/property_tree_builder.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/tree_synchronizer.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/tree_synchronizer.h
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/cc/trees/tree_synchronizer_unittest.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/content/renderer/gpu/gpu_benchmarking_extension.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/content/renderer/pepper/pepper_compositor_host.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/content/renderer/render_view_browsertest.cc
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/third_party/WebKit/Source/platform/testing/WebLayerTreeViewImplForTesting.cpp
[modify] https://crrev.com/b69ba945a0cf9cb00e9fe4bec3037d4255a60902/ui/compositor/compositor.cc

Comment 10 by khushals...@chromium.org, Jan 27 2017

Status: Fixed (was: Assigned)
And done.
►

Sign in to add a comment