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

Issue 629940 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Tie SurfaceFactory to a single client

Project Member Reported by fsam...@chromium.org, Jul 20 2016

Issue description

SurfaceManager and SurfaceFactory set up is very complex due to the requirement that there may be more than one SurfaceFactoryClient per client ID, and a child compositor may be ready before a parent. We should try to clean up code using surfaces to avoid all this complexity in cc.



 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 20 2016

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

commit 182c30acd4d03bc820af9f2ecc49e3e2cd314b08
Author: fsamuel <fsamuel@chromium.org>
Date: Wed Jul 20 22:25:25 2016

mus: Each ServerWindowSurface should have a different client ID

We would like each SurfaceFactory to be tied to a single client ID
and single SurfaceFactoryClient to simplify cc code and unify a bunch
of code paths. Mus Window Server was an exception here. Two SurfaceFactories
shared a single client ID. This has been changed.

BUG= 629940 

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

[modify] https://crrev.com/182c30acd4d03bc820af9f2ecc49e3e2cd314b08/services/ui/ws/server_window_surface.cc
[modify] https://crrev.com/182c30acd4d03bc820af9f2ecc49e3e2cd314b08/services/ui/ws/server_window_surface.h
[modify] https://crrev.com/182c30acd4d03bc820af9f2ecc49e3e2cd314b08/services/ui/ws/server_window_surface_manager.cc
[modify] https://crrev.com/182c30acd4d03bc820af9f2ecc49e3e2cd314b08/services/ui/ws/server_window_surface_manager.h

Owner: fsam...@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 22 2016

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

commit 0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f
Author: fsamuel <fsamuel@chromium.org>
Date: Fri Jul 22 21:50:18 2016

ui::ContextFactory should not create SurfaceIdAllocators

In the near future, SurfaceIdAllocator will live in clients
and so ui::ContextFactory should not create a
SurfaceIdAllocator directly. Instead, ui::ContextFactory
allocates a new surface client ID, which can be used to
create a SurfaceIdAllocator in clients.

This CL also moves calls to RegisterSurfaceClientId and
InvalidateSurfaceClientId close to
RegisterSurfaceClientFactory and
UnregisterSurfaceClientFactory respectively. In a
subsequent CL, these two will be merged together into
SurfaceFactory.

BUG= 627283 ,  629940 
TBR=reveman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/android_webview/browser/hardware_renderer.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/android_webview/browser/surfaces_instance.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/android_webview/browser/surfaces_instance.h
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/ash/sysui/stub_context_factory.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/ash/sysui/stub_context_factory.h
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/cc/surfaces/display_unittest.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/cc/surfaces/surface_id_allocator.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/cc/surfaces/surface_id_allocator.h
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/cc/test/test_delegating_output_surface.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/components/exo/surface.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/content/browser/compositor/surface_utils.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/content/browser/compositor/surface_utils.h
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/content/browser/renderer_host/compositor_impl_android.h
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/content/browser/renderer_host/offscreen_canvas_surface_impl.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/content/renderer/android/synchronous_compositor_output_surface.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/content/test/test_render_view_host.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/services/ui/surfaces/display_compositor.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/services/ui/ws/server_window_surface.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/ui/compositor/compositor.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/ui/compositor/compositor.h
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/ui/compositor/test/in_process_context_factory.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/ui/compositor/test/in_process_context_factory.h
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/ui/views/mus/surface_context_factory.cc
[modify] https://crrev.com/0fd36aebebd0d908cd9f7fcc51d45b68d1c73e9f/ui/views/mus/surface_context_factory.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 24 2016

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

commit 8395d6bb8c48e644a138c7fcaff518af127373c2
Author: fsamuel <fsamuel@chromium.org>
Date: Sun Jul 24 18:18:01 2016

cc: Remove unnecessary references to SurfaceManager in SurfaceIdAllocator

This is a trivial cleanup removing references to SurfaceManager from
SurfaceIdAllocator. SurfaceIds will be allocated in clients in the future
whereas SurfaceManager is a service-side concept.

BUG= 627283 ,  629940 
TBR=danakj@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel

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

[modify] https://crrev.com/8395d6bb8c48e644a138c7fcaff518af127373c2/cc/surfaces/surface_id_allocator.cc
[modify] https://crrev.com/8395d6bb8c48e644a138c7fcaff518af127373c2/cc/surfaces/surface_id_allocator.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 26 2016

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

commit d03176d916bea43a30b051d826fc4efd011ab866
Author: fsamuel <fsamuel@chromium.org>
Date: Tue Jul 26 11:48:06 2016

cc: Register namespace hierarchy after ui::Compositor's SurfaceFactory available

In the future, we'd like to merge Surface client ID registration and
SurfaceFactory creation. SurfaceFactory is tied to an OutputSurface. Thus, we
cannot register a namespace hierarchy relationship until after we create an
OutputSurface for the ui::Compositor. However, we create a new OutputSurface
every time the GPU process is restarted. Thus, we need a mechanism to store
these relationships between clients across GPU restarts.

This is particularly important if the display compositor lives in the same
process GPU and these relationships would be lost. This CL stores the
relationship between DelegatedFrameHost and ui::Compositor in ui::Compositor.

Note that depending on the final shape of the display compositor host API,
we may wish to centralize all hierarchy relationship registration including
OOPIF.

BUG= 629940 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel

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

[modify] https://crrev.com/d03176d916bea43a30b051d826fc4efd011ab866/cc/trees/layer_tree_host.h
[modify] https://crrev.com/d03176d916bea43a30b051d826fc4efd011ab866/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/d03176d916bea43a30b051d826fc4efd011ab866/ui/compositor/compositor.cc
[modify] https://crrev.com/d03176d916bea43a30b051d826fc4efd011ab866/ui/compositor/compositor.h
[modify] https://crrev.com/d03176d916bea43a30b051d826fc4efd011ab866/ui/compositor/compositor_unittest.cc

Status: Fixed (was: Started)
Blocking:

Sign in to add a comment