New issue
Advanced search Search tips

Issue 826886 link

Starred by 3 users
Status: Fixed
Owner:
danakj@chromium.org
Closed: Jun 2018
Cc:
danakj@chromium.org
kylec...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 730660



Sign in to add a comment

TextureLayerImpl should handle losing software CompositorFrameSink (LayerTreeFrameSink)

Project Member Reported by danakj@chromium.org, Mar 28 2018 
Currently TextureLayerImpl will in ReleaseResources()

1. drop its TransferableResource if it did not import it to the ResourceProvider.
2. remove it from the ResourceProvider otherwise

In both of these cases the resource is returned to the client with its ReleaseCallback running.

However this poses a problem for VizDisplayCompositor software mode. In this case, the CompositorFrameSink can be lost, while the TextureLayer client will not know since it's not generating resources with a ContextProvider (which it would see lost and recreate resources).

IMO the answer here is to let TextureLayerImpl persist software-backed resources across a new LayerTreeFrameSink. So that is:

1. don't drop the TransferableResource if |is_software|
2. remove the resource from ResourceProvider (cuz the RP will be deleted), but in such a way that the ReleaseCallback is not run, and ownership of the TransferableResource is returned to the TextureLayerImpl.

Comment 1 by danakj@chromium.org, Mar 28 2018

Blocking: 730660
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 25 2018 

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

commit a32578c5e375ec83675435fbacdbc5697f2595dd
Author: danakj <danakj@chromium.org>
Date: Wed Apr 25 21:18:36 2018

viz: Cleanup format specification and separate from ResourceProvider

This removes a bunch of complexity around formats that is superfluous,
and removes format selection from the LayerTreeResourceProvider,
eliminating a use of the ContextProvider from it. This might help
with implmenting 826886, or just be a cleanup that I found while
trying it out.

The RasterBufferProvider impls were using a format specified in the
LayerTreeSettings if it was supported, based on the opacity of the
content. However the only possible format that the settings would
specify other than the default was RGBA_4444 which is always
supported if specified. And opacity doesn't actually play into any
decisions made anymore.

So stop passing around opacity, and ask viz::PlatformColor for a
format to use in the RasterBufferProvider impls, with the caveat that
RGBA_4444 should be preferred (for gpu compositing) if specified by
the settings.

R=kylechar@chromium.org
TBR=piman

Bug:  826886 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I74200eb54bef57fa663eede861661c876a87a408
Reviewed-on: https://chromium-review.googlesource.com/1024703
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553748}
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/base/switches.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/base/switches.h
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/layers/heads_up_display_layer_impl.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/raster/bitmap_raster_buffer_provider.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/raster/bitmap_raster_buffer_provider.h
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/raster/gpu_raster_buffer_provider.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/raster/gpu_raster_buffer_provider.h
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/raster/one_copy_raster_buffer_provider.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/raster/one_copy_raster_buffer_provider.h
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/raster/raster_buffer_provider.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/raster/raster_buffer_provider.h
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/raster/raster_buffer_provider_perftest.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/raster/raster_buffer_provider_unittest.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/raster/zero_copy_raster_buffer_provider.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/raster/zero_copy_raster_buffer_provider.h
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/resources/display_resource_provider.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/resources/layer_tree_resource_provider.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/resources/layer_tree_resource_provider.h
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/test/fake_raster_buffer_provider.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/test/fake_raster_buffer_provider.h
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/test/layer_tree_pixel_resource_test.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/tiles/tile_manager.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/tiles/tile_manager.h
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/tiles/tile_manager_unittest.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/trees/layer_tree_host_pixeltest_tiles.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/trees/layer_tree_settings.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/cc/trees/layer_tree_settings.h
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/components/viz/common/resources/platform_color.h
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/components/viz/common/resources/resource_format_utils.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/components/viz/common/resources/resource_format_utils.h
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/components/viz/service/display/gl_renderer.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/components/viz/service/display/gl_renderer_unittest.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/components/viz/service/display/skia_renderer.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/components/viz/service/display/skia_renderer.h
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/a32578c5e375ec83675435fbacdbc5697f2595dd/ui/compositor/compositor.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 27 2018 

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

commit 2b8f93fa9923fa16053e3667fea2204effc57f27
Author: danakj <danakj@chromium.org>
Date: Fri Apr 27 19:31:29 2018

cc: Remove sync_query from LayerTreeResourceProvider

Instead of duplicating this bit off the compositor ContextProvider,
query it directy on the ContextProvider that will be held differently
as a result of its value.

R=piman@chromium.org, pdr

Bug:  826886 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ia9a4f7ca4d9cbf10121f7d729d4068a6f9b93e90
Reviewed-on: https://chromium-review.googlesource.com/1026839
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554452}
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/raster/one_copy_raster_buffer_provider.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/raster/one_copy_raster_buffer_provider.h
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/raster/raster_buffer_provider_perftest.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/raster/raster_buffer_provider_unittest.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/raster/staging_buffer_pool.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/raster/staging_buffer_pool.h
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/raster/staging_buffer_pool_unittest.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/resources/display_resource_provider_unittest.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/resources/layer_tree_resource_provider.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/resources/layer_tree_resource_provider.h
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/resources/layer_tree_resource_provider_unittest.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/resources/resource_pool_unittest.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/resources/resource_provider_unittest.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/resources/video_resource_updater_unittest.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/test/fake_resource_provider.h
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/test/layer_tree_pixel_resource_test.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/test/pixel_test.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/test/pixel_test.h
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/components/viz/service/display/gl_renderer_unittest.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/components/viz/service/display/overlay_unittest.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/content/renderer/media/media_factory.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/third_party/blink/public/platform/web_video_frame_submitter.h
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/third_party/blink/renderer/platform/exported/web_video_frame_submitter.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.h
[modify] https://crrev.com/2b8f93fa9923fa16053e3667fea2204effc57f27/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc
Project Member

Comment 4 by bugdroid1@chromium.org, May 1 2018 

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

commit 8e3eab9e87f949d94c605ebb5f3717d9161433a6
Author: danakj <danakj@chromium.org>
Date: Tue May 01 17:26:00 2018

Remove YUV format decisions from LayerTreeResourceProvider

Put it in VideoResourceUpdater where it is used instead, by reading
the context capabilities there.

R=piman@chromium.org
TBR=pdr

Bug:  826886 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I5c7d5d460091d472eacee527c0af0f37ba1657b0
Reviewed-on: https://chromium-review.googlesource.com/1033421
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555088}
[modify] https://crrev.com/8e3eab9e87f949d94c605ebb5f3717d9161433a6/cc/layers/video_layer_impl.cc
[modify] https://crrev.com/8e3eab9e87f949d94c605ebb5f3717d9161433a6/cc/resources/layer_tree_resource_provider.cc
[modify] https://crrev.com/8e3eab9e87f949d94c605ebb5f3717d9161433a6/cc/resources/layer_tree_resource_provider.h
[modify] https://crrev.com/8e3eab9e87f949d94c605ebb5f3717d9161433a6/cc/resources/video_resource_updater.cc
[modify] https://crrev.com/8e3eab9e87f949d94c605ebb5f3717d9161433a6/cc/resources/video_resource_updater.h
[modify] https://crrev.com/8e3eab9e87f949d94c605ebb5f3717d9161433a6/cc/resources/video_resource_updater_unittest.cc
[modify] https://crrev.com/8e3eab9e87f949d94c605ebb5f3717d9161433a6/cc/test/fake_resource_provider.h
[modify] https://crrev.com/8e3eab9e87f949d94c605ebb5f3717d9161433a6/components/viz/common/resources/resource_settings.h
[modify] https://crrev.com/8e3eab9e87f949d94c605ebb5f3717d9161433a6/components/viz/service/display/renderer_pixeltest.cc
[modify] https://crrev.com/8e3eab9e87f949d94c605ebb5f3717d9161433a6/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc

Comment 5 by danakj@chromium.org, May 1 2018

Owner: danakj@chromium.org
Status: Started (was: Available)
Project Member

Comment 6 by bugdroid1@chromium.org, May 17 2018 

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

commit 5a196055274269b28b99be81b56faf3ef931f5b9
Author: danakj <danakj@chromium.org>
Date: Thu May 17 14:27:05 2018

Remove GL context use from LayerTreeResourceProvider.

Pass in a context for PrepareSendToParent(). Do ShallowFlushCHROMIUM()
directly at the callers of FlushPendingDeletes() instead of going
through ResourceProvider. And Finish() after destroying the
LayerTreeResourceProvider instead of in the destructor.

R=piman@chromium.org, chrishtr

Bug:  826886 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ic303e296a1c5eacb2b05a7134fb3e919ebd32a32
Reviewed-on: https://chromium-review.googlesource.com/1062591
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559524}
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/layers/heads_up_display_layer_impl.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/raster/raster_buffer_provider_perftest.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/raster/raster_buffer_provider_unittest.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/resources/display_resource_provider_unittest.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/resources/layer_tree_resource_provider.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/resources/layer_tree_resource_provider.h
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/resources/layer_tree_resource_provider_unittest.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/resources/resource_pool.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/resources/resource_pool.h
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/resources/resource_pool_unittest.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/resources/resource_provider_unittest.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/test/fake_picture_layer_tiling_client.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/test/fake_picture_layer_tiling_client.h
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/test/render_pass_test_utils.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/test/render_pass_test_utils.h
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/test/resource_provider_test_utils.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/test/resource_provider_test_utils.h
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/tiles/picture_layer_tiling_set_unittest.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/components/viz/service/display/gl_renderer_unittest.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/components/viz/service/display/overlay_unittest.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/components/viz/service/display/renderer_pixeltest.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/components/viz/service/display/software_renderer_unittest.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc
[modify] https://crrev.com/5a196055274269b28b99be81b56faf3ef931f5b9/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.h
Project Member

Comment 7 by bugdroid1@chromium.org, May 17 2018 

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

commit b148b4788bc90804d3848878db99aff8167c5096
Author: danakj <danakj@chromium.org>
Date: Thu May 17 15:33:52 2018

cc: Remove glFinish call during cc shutdown.

From piman:
I'm almost 100% positive we don't need this any more.
At the origin, when the renderer was rendering directly to a child
HWND/XWindow, and pre-ForceCompositingMode, we needed this to avoid
flashing when going out of compositing mode, but that constraint is
long long gone. On the Display compositor, we also need a synchronous
IPC on command buffer destruction, but that still exists regardless
of this.

R=piman@chromium.org

Bug:  826886 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I15ed61c1f661ab2f615cae6f69f48eeb51d8e7ee
Reviewed-on: https://chromium-review.googlesource.com/1062730
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559541}
[modify] https://crrev.com/b148b4788bc90804d3848878db99aff8167c5096/cc/trees/layer_tree_host_impl.cc
Project Member

Comment 8 by bugdroid1@chromium.org, May 17 2018 

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

commit 5b391e83af8b3748e49a19283aaa86d89dee7622
Author: danakj <danakj@chromium.org>
Date: Thu May 17 17:53:26 2018

Remove max_texture_size() from LayerTreeResourceProvider.

Store it on LayerTreeHostImpl, and pick one in
VideoFrameResourceProvider.

R=chrishtr@chromium.org, piman@chromium.org

Bug:  826886 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I910b8a624dbc1265087a91c45faeeb0d3ec575d8
Reviewed-on: https://chromium-review.googlesource.com/1062809
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559596}
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/layers/heads_up_display_layer_impl.cc
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/layers/picture_layer_impl.cc
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/layers/picture_layer_impl_unittest.cc
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/layers/video_layer_impl.cc
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/resources/display_resource_provider_unittest.cc
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/resources/layer_tree_resource_provider.cc
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/resources/layer_tree_resource_provider.h
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/resources/layer_tree_resource_provider_unittest.cc
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/resources/resource_provider_unittest.cc
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/resources/video_resource_updater.cc
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/resources/video_resource_updater.h
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/resources/video_resource_updater_unittest.cc
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/test/fake_resource_provider.h
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/test/pixel_test.cc
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/test/pixel_test.h
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/cc/trees/layer_tree_impl.h
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/components/viz/service/display/renderer_pixeltest.cc
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc
[modify] https://crrev.com/5b391e83af8b3748e49a19283aaa86d89dee7622/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.h
Project Member

Comment 9 by bugdroid1@chromium.org, May 30 2018 

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

commit 4b15f74baab5b7997bead470c2b4d495e4a3874c
Author: danakj <danakj@chromium.org>
Date: Wed May 30 22:52:22 2018

Remove the compositor ContextProvider from ClientResourceProvider

It was being used to report if it was there or not, and in some dead
functions. The test checking IsSoftware() didn't actually care, (it used
it to pass HARDWARE or SOFTWARE to a funciton that only checks for
RESOURCELESS_SOFTWARE) and the dead code is now removed.

TBR=jam

Bug:  826886 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ia5c19a3ac25dc3d2ae58e44cee25d2c44ec61dff
Reviewed-on: https://chromium-review.googlesource.com/1079688
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563045}
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/cc/BUILD.gn
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/cc/raster/raster_buffer_provider_perftest.cc
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/cc/raster/raster_buffer_provider_unittest.cc
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/cc/resources/resource_pool_unittest.cc
[delete] https://crrev.com/bce4015c3968a9b67041e8b97664986999fac714/cc/test/fake_resource_provider.h
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/cc/test/layer_test_common.cc
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/cc/test/pixel_test.cc
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/cc/tiles/picture_layer_tiling_set_unittest.cc
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/cc/trees/layer_tree_host_unittest_context.cc
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/components/viz/client/client_resource_provider.cc
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/components/viz/client/client_resource_provider.h
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/components/viz/client/client_resource_provider_unittest.cc
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/components/viz/service/display/display_resource_provider_unittest.cc
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/components/viz/service/display/gl_renderer_unittest.cc
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/components/viz/service/display/overlay_unittest.cc
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/components/viz/service/display/software_renderer_unittest.cc
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/components/viz/service/display/surface_aggregator_perftest.cc
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/components/viz/service/display/surface_aggregator_unittest.cc
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/media/renderers/video_resource_updater_unittest.cc
[modify] https://crrev.com/4b15f74baab5b7997bead470c2b4d495e4a3874c/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 1 2018 

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

commit 93f76ba62131839d65bec7f5339b99d64b93e71f
Author: danakj <danakj@chromium.org>
Date: Fri Jun 01 22:14:33 2018

Move the delegated_sync_points_required flag to LayerTreeSettings.

We want to make a ClientResourceProvider only once for the life of a
LayerTreeHost, so the flag should be a constant. Since
LayerTreeFrameSinks come and go, they can have different settings for
this flag technically (though they don't). Make it clear in the code
by making it const by putting it on LayerTreeSettings instead, and
query it in ui::Compositor from the ContextFactory.

R=piman@chromium.org

Bug:  826886 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ifcf0e07eff678eb0c2725dc86efc8dada1021537
Reviewed-on: https://chromium-review.googlesource.com/1082604
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563825}
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/cc/trees/layer_tree_frame_sink.h
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/cc/trees/layer_tree_settings.h
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/components/viz/service/frame_sinks/direct_layer_tree_frame_sink.cc
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/components/viz/test/test_layer_tree_frame_sink.cc
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/content/browser/compositor/test/test_image_transport_factory.cc
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/content/browser/compositor/test/test_image_transport_factory.h
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/content/browser/compositor/viz_process_transport_factory.h
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/ui/aura/mus/mus_context_factory.cc
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/ui/aura/mus/mus_context_factory.h
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/ui/compositor/compositor.cc
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/ui/compositor/compositor.h
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/ui/compositor/test/fake_context_factory.cc
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/ui/compositor/test/fake_context_factory.h
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/ui/compositor/test/in_process_context_factory.cc
[modify] https://crrev.com/93f76ba62131839d65bec7f5339b99d64b93e71f/ui/compositor/test/in_process_context_factory.h
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 7 2018 

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

commit b90335db6c9604dc72f85c239ca816b72434467a
Author: danakj <danakj@chromium.org>
Date: Thu Jun 07 23:42:29 2018

Keep ClientResourceProvider alive forever in LayerTreeHostImpl

Instead of creating and destroying the ClientResourceProvider with
the LayerTreeFrameSink, create it once and keep it around until the
LayerTreeHostImpl is destroyed. This allows ResourceIds to remain
valid when the LayerTreeFrameSink is replaced while software
compositing (which can happen if the viz service crashes).

We add a method ShutdownAndReleaseAllResources() on
ClientResourceProvider that will release any exported resources,
marking them as lost, instead of doing so in the destructor,
so that the owner of the class can control the timing more without
needing to heap-allocate the class.

We also take away the allowance for importing a ResourceId and
not removing it before destroying the ClientResourceProvider (or
calling ShutdownAndReleaseAllResources()). Instead we have it
DCHECK() if this occurs, reporting the stack trace from where
the resource was imported. We do this because marking resources
that are not exported as lost is silly, since they were imported
they could have been removed. A bunch of unit test changes are done
to remove resources that were imported because of this.

Two unit tests that tested the case of destroying the
ClientResourceProvider while imported resources were present are
removed since it is no longer supported.

When the CompositorFrameSink is lost, the LayerTreeHostImpl loses
the ability to receive back resources it had previously exported.
We add a method ReleaseAllExportedResources() on
ClientResourceProvider to handle this case, which removes the
export count on all resources, releasing any marked for deletion
previously. When software compositing, they do not need to be
marked lost, but when gpu compositing they do since the display
compositor modifies texture metadata in order to use them. Tests
for the new method are added, as well as to verify that in
LayerTreeHostImpl, when the CompositorFrameSink is replaced, that
the ResourceIds are not kept around.

The software-resources-aren't-lost distinction is not important in
this CL, since LayerTreeHostImpl destroys all its resources
regardless when it tears down the ResourcePool and
RasterBufferProvider. However for external clients which provide a
resource through TextureLayer, the ResourceId can remain valid in
software compositing.

R=piman@chromium.org

Bug:  826886 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I85a867c2830c31d39796ff971e01a221e5716812
Reviewed-on: https://chromium-review.googlesource.com/1083617
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565464}
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/cc/layers/nine_patch_layer_impl_unittest.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/cc/layers/ui_resource_layer_impl_unittest.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/cc/resources/resource_pool_unittest.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/cc/test/fake_ui_resource_layer_tree_host_impl.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/cc/test/pixel_test.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/cc/test/render_pass_test_utils.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/cc/test/render_pass_test_utils.h
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/cc/test/resource_provider_test_utils.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/cc/trees/layer_tree_host_unittest_context.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/components/viz/client/client_resource_provider.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/components/viz/client/client_resource_provider.h
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/components/viz/client/client_resource_provider_unittest.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/components/viz/service/display/display_resource_provider_unittest.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/components/viz/service/display/gl_renderer_unittest.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/components/viz/service/display/overlay_unittest.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/components/viz/service/display/software_renderer_unittest.cc
[modify] https://crrev.com/b90335db6c9604dc72f85c239ca816b72434467a/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 27 2018 

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

commit d0ed5774569fc56f4a0c74affafda8a7ad2a02aa
Author: danakj <danakj@chromium.org>
Date: Wed Jun 27 16:20:42 2018

Allow TextureLayer to hold onto software resources over viz restart

If the gpu process crashes, the viz service is gone and bitmaps need
to be re-registered, but they are still valid bitmaps. So we can keep
the ResourceId alive and not returned it as lost to the client in that
case.

R=kylechar@chromium.org

Bug:  826886 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I1ef6af3a90bf90310793fcdbbb70ea00a0d85d25
Reviewed-on: https://chromium-review.googlesource.com/1115591
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570798}
[modify] https://crrev.com/d0ed5774569fc56f4a0c74affafda8a7ad2a02aa/cc/layers/texture_layer_impl.cc
[modify] https://crrev.com/d0ed5774569fc56f4a0c74affafda8a7ad2a02aa/cc/layers/texture_layer_unittest.cc

Comment 13 by danakj@chromium.org, Jun 27 2018

Status: Fixed (was: Started)
►

Sign in to add a comment