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

Issue 742374 link

Starred by 1 user
Status: Fixed
Owner:
kylec...@chromium.org
Closed: Jul 2017
Cc:
danakj@chromium.org
█ fsam...@chromium.org
rjkroege@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Have HostFrameSinkManager mediate all in process frame sinks

Project Member Reported by kylec...@chromium.org, Jul 13 2017 
In order to move forward with making surface references possible without using Mojo, we need to make HostFrameSinkManager handle the following:

-Create/Destroy CompositorFrameSinkSupports
-Register frame sink hierarchy

This will involve adding a dependency from //components/viz/host back to //components/viz/service again, since HostFrameSinkManager will need to have access to FrameSinkManager to do these things.

Comment 1 by danakj@chromium.org, Jul 13 2017 

Another option would be to make an interface in host/ to do this. Implement something in content using service/ that does it, and pass that to host/.

Comment 2 by kylec...@chromium.org, Jul 13 2017 

An interface would also work. We have the opposite problem too, things in //components/viz/service would need to depend on //components/viz/host, which might necessitate using interfaces to avoid circular deps.
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 14 2017 

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

commit a09001664d28ea938508e2c9081578f86d33c94f
Author: kylechar <kylechar@chromium.org>
Date: Fri Jul 14 17:35:25 2017

Change CompositorFrameSinkSupport creation.

Make the creation of CompositorFrameSinkSupport go through
HostFrameSinkManager. This takes us one step closer to viz architecture
without using Mojo anywhere.

FrameSinkManager is injected into HostFrameSinkManager if we want this
path to be available. Every callsite in the browser process that creates
a CompositorFrameSinkSupport goes through HostFrameSinkManager, with the
exception of Android webview which doesn't use HostFrameSinkManager at
all.

This requires adding a dep from //components/viz/host to
//components/viz/service again. It also requires adding an interface for
DirectLayerTreeFrameSink.

Bug:  742374 
Change-Id: I22b9fb855b7a16716dccd4d5233c72412f9b2259
Reviewed-on: https://chromium-review.googlesource.com/570504
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486795}
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/components/viz/host/BUILD.gn
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/components/viz/host/host_frame_sink_manager.h
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/components/viz/host/host_frame_sink_manager_unittests.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/components/viz/service/BUILD.gn
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/components/viz/service/frame_sinks/compositor_frame_sink_support.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/components/viz/service/frame_sinks/compositor_frame_sink_support.h
[add] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/components/viz/service/frame_sinks/compositor_frame_sink_support_manager.h
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/components/viz/service/frame_sinks/direct_layer_tree_frame_sink.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/components/viz/service/frame_sinks/direct_layer_tree_frame_sink.h
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/components/viz/service/frame_sinks/direct_layer_tree_frame_sink_unittest.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/components/viz/service/frame_sinks/frame_sink_manager_impl.h
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/content/browser/browser_main_loop.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/content/browser/compositor/surface_utils.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/content/browser/compositor/surface_utils.h
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/content/browser/compositor/test/no_transport_image_transport_factory.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/content/browser/renderer_host/offscreen_canvas_provider_impl_unittest.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/ui/android/BUILD.gn
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/ui/android/DEPS
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/ui/android/delegated_frame_host_android.h
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/ui/aura/BUILD.gn
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/ui/aura/DEPS
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/ui/aura/local/layer_tree_frame_sink_local.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/ui/aura/local/layer_tree_frame_sink_local.h
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/ui/aura/local/window_port_local.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/ui/compositor/test/context_factories_for_test.cc
[modify] https://crrev.com/a09001664d28ea938508e2c9081578f86d33c94f/ui/compositor/test/in_process_context_factory.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 14 2017 

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

commit 0d2bc35e6aec09c236175ae62569c9a8ccc5071a
Author: kylechar <kylechar@chromium.org>
Date: Fri Jul 14 19:59:06 2017

Fix HostFrameSinkManagerTest.

There is a use after free in recent commit. Fix it.

Bug:  742374 
Change-Id: Iab561ccdc7f863a77a1c70460a87b6714894e954
Reviewed-on: https://chromium-review.googlesource.com/571323
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486857}
[modify] https://crrev.com/0d2bc35e6aec09c236175ae62569c9a8ccc5071a/components/viz/host/host_frame_sink_manager_unittests.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 25 2017 

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

commit 820fe6d53a96fc3f24a0dcbea8d369bf91b1e109
Author: kylechar <kylechar@chromium.org>
Date: Tue Jul 25 00:10:59 2017

Register frame sink hierarchy through HostFrameSinkManager.

This CL changes calls to (R|Unr)egisterFrameSinkHierarchy() to go
through HostFrameSinkManager instead of FrameSinkManagerImpl. By
default, HostFrameSinkManager just calls into FrameSinkManagerImpl so
the change should be a no-op change.

Change the internal workings of HostFrameSinkManager to accommodate
this.  Originally, the frame sink hierarchy unregisters itself on
destruction of a mojom::CompositorFrameSink. This isn't compatible with
how CompositorFrameSinkSupport works so remove that code. There
shouldn't be any ordering requirements for the order of
creating/destroying the CompositorFrameSink/CompositorFrameSinkSupport
vs registering frame sink hierarchy.

Also improve the two tests in HostFrameSinkManagerTests.

Bug:  742374 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I537f36305d9060223975ce6e54b33bf760b844d4
Reviewed-on: https://chromium-review.googlesource.com/583630
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489153}
[modify] https://crrev.com/820fe6d53a96fc3f24a0dcbea8d369bf91b1e109/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/820fe6d53a96fc3f24a0dcbea8d369bf91b1e109/components/viz/host/host_frame_sink_manager.h
[modify] https://crrev.com/820fe6d53a96fc3f24a0dcbea8d369bf91b1e109/components/viz/host/host_frame_sink_manager_unittests.cc
[modify] https://crrev.com/820fe6d53a96fc3f24a0dcbea8d369bf91b1e109/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/820fe6d53a96fc3f24a0dcbea8d369bf91b1e109/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/820fe6d53a96fc3f24a0dcbea8d369bf91b1e109/ui/compositor/compositor.cc

Comment 6 by kylec...@chromium.org, Jul 26 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment