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

Issue 627283 link

Starred by 3 users
Status: Fixed
Owner:
samans@chromium.org
Closed: Apr 2017
Cc:
danakj@chromium.org
█ junov@chromium.org
█ xlai@chromium.org
█ vollick@chromium.org
penghuang@chromium.org
rjkroege@chromium.org
vmi...@chromium.org
sadrul@chromium.org
█ briander...@chromium.org
piman@chromium.org
tsepez@chromium.org
dcheng@chromium.org
█ jbau...@chromium.org
mustash-bugs@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
mus

Blocking:
issue 619138



Sign in to add a comment

Surface IDs should be allocated from the renderer

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

Given we have off screen canvas now, and with work on a display compositor API in progress, we should revisit the way we deal with surface ID allocation.

We currently allocate surface IDs in the browser process, So far that's been fine, but with the introduction of offscreen canvas, we have now introduced sync IPCs to allocate surface IDs unnecessarily (surface IDs were intended to be allocated partially by a client, I think).

Surfaces should be allocated in the renderer and each client (renderer) should get a unique browser-allocated "client ID". This would remove the need for a sync IPC from the renderer to allocate an ID.

Comment 1 by fsam...@chromium.org, Jul 11 2016

Cc: rjkroege@chromium.org junov@chromium.org
Owner: fsam...@chromium.org
Status: Started (was: Untriaged)

Comment 2 by fsam...@chromium.org, Jul 11 2016

Cc: xlai@chromium.org

Comment 3 by xlai@chromium.org, Jul 12 2016

Blocking: 619138

Comment 4 by fsam...@chromium.org, Jul 12 2016

Cc: tsepez@chromium.org dcheng@chromium.org
Antoine and I discussed this a bit offline. I'd like to replace output_surface_id with SurfaceId for IPC routng of SwapCompositorFrame IPCs. output_surface_id exists to make sure we discard returned resources for previous gpu process instances. Thus, the Surface ID needs to change as a new GPU process is created. This matches what we'd like to accomplish with the display compositor in mus.

Thus, surface ID will now contain four components:

gpu_id: This uniquely identifies the version of the gpu process. This will be allocated on EstablishGpuChannel. This will be 0 for software compositing.

client_id: This uniquely defines a client (view). Note that multiple clients may live in a single process. This ID will be allocated with the client. In Chrome, that's ViewMsg_New most likely.

local_id: This uniquely identifies a surface within a client. This ID will be allocated by the client.

nonce: This is a randomly generated value by the client to ensure IDs are unguessable.

Comment 5 by fsam...@chromium.org, Jul 12 2016 

Putting more thought into this, I actually don't think we need the gpu_id with mojo. In Mus currently, each OutputSurface has a separate MessagePipe and it is not possible to send a new OutputSurface a message from an old OutputSurface. Thus, we've decided not to get rid of output_surface_id for now and introduce gpu_id.

I do still want to rename id_namespace to client_id though.
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 13 2016 

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

commit 27866427f8e663f7b43e698006bb495b97726bfa
Author: fsamuel <fsamuel@chromium.org>
Date: Wed Jul 13 04:06:59 2016

Update Surface ID Terminology

This CL renames id_namespace to client_id to capture
the fact that clients may be things other than renderers.

This CL also passes SurfaceId by const reference instead
of value now that SurfaceId is a 128-bit quantity.

BUG= 627283 
TBR=sadrul@chromium.org, dcheng@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/android_webview/browser/hardware_renderer.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/android_webview/browser/surfaces_instance.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/android_webview/browser/surfaces_instance.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/ipc/cc_param_traits.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/ipc/cc_param_traits_macros.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/ipc/struct_traits_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/ipc/surface_id.mojom
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/ipc/surface_id_struct_traits.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/ipc/surface_sequence.mojom
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/ipc/surface_sequence_struct_traits.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/layers/surface_layer.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/layers/surface_layer.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/layers/surface_layer_impl.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/layers/surface_layer_impl.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/layers/surface_layer_impl_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/layers/surface_layer_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/proto/layer_tree_host.proto
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/quads/surface_draw_quad.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/quads/surface_draw_quad.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/display.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/display.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/display_scheduler.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/display_scheduler.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/display_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_aggregator_perftest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_aggregator_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_damage_observer.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_display_output_surface.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_display_output_surface_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_factory_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_id.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_id_allocator.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_id_allocator.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_manager.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_manager_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_sequence.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surfaces_pixeltest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/test/pixel_test_delegating_output_surface.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/trees/layer_tree_host.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/trees/layer_tree_host_unittest_serialization.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/components/exo/surface.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/components/exo/surface.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/frame_host/cross_process_frame_connector.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/frame_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/offscreen_canvas_surface_impl.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_view_host_impl.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_input_event_router.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/common/view_messages.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/renderer/android/synchronous_compositor_output_surface.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/renderer/gpu/render_widget_compositor.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/renderer/render_widget.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/renderer/render_widget.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/test/test_render_view_host.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/test/test_render_view_host.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/services/ui/surfaces/display_compositor.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/services/ui/surfaces/surfaces_state.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/services/ui/surfaces/surfaces_state.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/services/ui/ws/server_window_surface.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/services/ui/ws/server_window_surface_manager.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/services/ui/ws/server_window_surface_manager.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/ui/compositor/compositor.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/ui/compositor/layer.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/ui/compositor/layer.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/ui/compositor/test/in_process_context_factory.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/ui/compositor/test/in_process_context_factory.h
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 13 2016

Labels: merge-merged-2795
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/27866427f8e663f7b43e698006bb495b97726bfa

commit 27866427f8e663f7b43e698006bb495b97726bfa
Author: fsamuel <fsamuel@chromium.org>
Date: Wed Jul 13 04:06:59 2016

Update Surface ID Terminology

This CL renames id_namespace to client_id to capture
the fact that clients may be things other than renderers.

This CL also passes SurfaceId by const reference instead
of value now that SurfaceId is a 128-bit quantity.

BUG= 627283 
TBR=sadrul@chromium.org, dcheng@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/android_webview/browser/hardware_renderer.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/android_webview/browser/surfaces_instance.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/android_webview/browser/surfaces_instance.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/ipc/cc_param_traits.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/ipc/cc_param_traits_macros.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/ipc/struct_traits_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/ipc/surface_id.mojom
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/ipc/surface_id_struct_traits.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/ipc/surface_sequence.mojom
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/ipc/surface_sequence_struct_traits.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/layers/surface_layer.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/layers/surface_layer.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/layers/surface_layer_impl.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/layers/surface_layer_impl.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/layers/surface_layer_impl_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/layers/surface_layer_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/proto/layer_tree_host.proto
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/quads/surface_draw_quad.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/quads/surface_draw_quad.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/display.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/display.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/display_scheduler.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/display_scheduler.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/display_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_aggregator_perftest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_aggregator_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_damage_observer.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_display_output_surface.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_display_output_surface_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_factory_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_id.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_id_allocator.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_id_allocator.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_manager.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_manager_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_sequence.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surface_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/surfaces/surfaces_pixeltest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/test/pixel_test_delegating_output_surface.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/trees/layer_tree_host.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/cc/trees/layer_tree_host_unittest_serialization.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/components/exo/surface.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/components/exo/surface.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/frame_host/cross_process_frame_connector.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/frame_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/offscreen_canvas_surface_impl.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_view_host_impl.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_input_event_router.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/common/view_messages.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/renderer/android/synchronous_compositor_output_surface.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/renderer/gpu/render_widget_compositor.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/renderer/render_widget.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/renderer/render_widget.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/test/test_render_view_host.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/content/test/test_render_view_host.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/services/ui/surfaces/display_compositor.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/services/ui/surfaces/surfaces_state.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/services/ui/surfaces/surfaces_state.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/services/ui/ws/server_window_surface.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/services/ui/ws/server_window_surface_manager.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/services/ui/ws/server_window_surface_manager.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/ui/compositor/compositor.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/ui/compositor/layer.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/ui/compositor/layer.h
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/ui/compositor/test/in_process_context_factory.cc
[modify] https://crrev.com/27866427f8e663f7b43e698006bb495b97726bfa/ui/compositor/test/in_process_context_factory.h
Project Member

Comment 8 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 9 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

Comment 10 by dtapu...@chromium.org, Sep 2 2016

Components: MUS

Comment 11 by rjkroege@chromium.org, Oct 4 2016

Components: Internals>MUS
Labels: Proj-Mustash
Project Member

Comment 12 by bugdroid1@chromium.org, Nov 23 2016 

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

commit e3d2888a771050099facf2b18c3d72b4922c9581
Author: fsamuel <fsamuel@chromium.org>
Date: Wed Nov 23 02:37:39 2016

Display Compositor: Towards allocating SurfaceIds in the client

Currently, SurfaceIds are allocated in Mus or in the browser process.
This CL takes us a few steps towards being able to allocate SurfaceIds
(in particular, the LocalFrameId component of the SurfaceId) in the
client.

1. MojoCompositorFrameSink::SubmitCompositorFrame is updated to take in
a LocalFrameId. Offscreen canvas and the mus client library is
updated to use this new signature.

2. GpuCompositorFrameSink is updated to assume new LocalFrameIds will
be allocated in the client instead.

BUG= 664547 ,  627283 
TBR=piman@chromium.org for new DEPS on surface_id_allocator.h
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

[modify] https://crrev.com/e3d2888a771050099facf2b18c3d72b4922c9581/cc/ipc/mojo_compositor_frame_sink.mojom
[modify] https://crrev.com/e3d2888a771050099facf2b18c3d72b4922c9581/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.cc
[modify] https://crrev.com/e3d2888a771050099facf2b18c3d72b4922c9581/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.h
[modify] https://crrev.com/e3d2888a771050099facf2b18c3d72b4922c9581/services/ui/public/cpp/window_compositor_frame_sink.cc
[modify] https://crrev.com/e3d2888a771050099facf2b18c3d72b4922c9581/services/ui/public/cpp/window_compositor_frame_sink.h
[modify] https://crrev.com/e3d2888a771050099facf2b18c3d72b4922c9581/services/ui/surfaces/gpu_compositor_frame_sink.cc
[modify] https://crrev.com/e3d2888a771050099facf2b18c3d72b4922c9581/services/ui/surfaces/gpu_compositor_frame_sink.h
[modify] https://crrev.com/e3d2888a771050099facf2b18c3d72b4922c9581/services/ui/ws/frame_generator.cc
[modify] https://crrev.com/e3d2888a771050099facf2b18c3d72b4922c9581/services/ui/ws/frame_generator.h
[modify] https://crrev.com/e3d2888a771050099facf2b18c3d72b4922c9581/services/ui/ws/window_tree_client_unittest.cc
[modify] https://crrev.com/e3d2888a771050099facf2b18c3d72b4922c9581/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
[modify] https://crrev.com/e3d2888a771050099facf2b18c3d72b4922c9581/third_party/WebKit/public/blink_typemaps.gni
[modify] https://crrev.com/e3d2888a771050099facf2b18c3d72b4922c9581/ui/aura/mus/DEPS
[modify] https://crrev.com/e3d2888a771050099facf2b18c3d72b4922c9581/ui/aura/mus/window_compositor_frame_sink.cc
[modify] https://crrev.com/e3d2888a771050099facf2b18c3d72b4922c9581/ui/aura/mus/window_compositor_frame_sink.h

Comment 13 by fsam...@chromium.org, Dec 1 2016

Blocking: 601863

Comment 14 by fsam...@chromium.org, Dec 7 2016 

In our latest discussions, I think it makes sense for the embedder to allocate the LocalFrameId on resize, and the embeddee to use that LocalFrameId to generate the CompositorFrame.

Comment 15 by sadrul@chromium.org, Dec 7 2016 

Would it make sense for the embedder to allocate the first frame id too?

Comment 16 by piman@chromium.org, Dec 7 2016 

@#15: yes, in the sense that the first frame is a special case of resize, in a way.

Comment 17 by fsam...@chromium.org, Dec 7 2016 

Note, that for this approach to work, I think we have to implement late resolution of surface IDs in SurfaceAggregator.

SurfaceDrawQuad has two surface IDs, a primary and a secondary one. If the primary is not available in SurfaceAggregator then we use the secondary one.

This probably doesn't give us the resize lock behavior we want. We want to be able to defer updates to an embedder for some fixed length of time (say up to 67ms) to give the embeddee time to generate a new frame.

This suggests that the "Compositor Lock" needs to live in the gpu process to me. If the primary surface ID is unavailable then delay using the newly submitted CompositorFrame for the timeout period or until the surface ID becomes available.

This makes lifetime management of CompositorFrames a bit more difficult. We need to maintain a list of (a single?) "pending" CompositorFrame that does not become "active" until a given timeout or when a given set of surface Ids become available.

Comment 18 by danakj@chromium.org, Dec 7 2016 

> Note, that for this approach to work, I think we have to implement late resolution of surface IDs in SurfaceAggregator.

Can you explain?

> This suggests that the "Compositor Lock" needs to live in the gpu process to me.

Can you explain this too? The compositor lock keeps the UI from resizing. How does a lock in the gpu process do that, and what does it do?

Comment 19 by fsam...@chromium.org, Dec 7 2016 

>> Note, that for this approach to work, I think we have to implement late resolution of surface IDs in SurfaceAggregator.

>Can you explain?

Basically my next sentence is the explanation: SurfaceDrawQuad has two surface IDs, a primary and a secondary one. If the primary is not available in SurfaceAggregator then we use the secondary one. But this doesn't quite reproduce the resize behavior we have today.

>> This suggests that the "Compositor Lock" needs to live in the gpu process to me.

>Can you explain this too? The compositor lock keeps the UI from resizing. How >does a lock in the gpu process do that, and what does it do?

The parent submits a CompositorFrame with surface IDs that do not yet have CompositorFrames in the gpu process. The gpu process puts that CompositorFrame aside for the time being until the pending surface Ids have been received or there's a timeout. Once resolved, we use the parent's new CompositorFrame.

Comment 20 by danakj@chromium.org, Dec 7 2016 

Ok thanks, things are bit fuzzy since the hackathon and trying to remember details.

What does "late resolution of surface IDs in SurfaceAggregator" mean? This is where we look up surface ids in the aggregator: https://cs.chromium.org/chromium/src/cc/surfaces/surface_aggregator.cc?rcl=1481098841&l=163

Are you just saying at that point it would if (!surface->HasFrame) grab the other surface Id and repeat with that? If so that makes sense.

> The gpu process puts that CompositorFrame aside for the time being until the pending surface Ids have been received or there's a timeout.

I see okay. It's not a lock I see why you used "quotes", its just delaying the ack to the UI compositor until it uses the frame and it waits for the required frames to do so.

We could do all of that in-process too I think FWIW if we wanna parallelize?

I took a quick look thru callers of lock functions on ui::Compositor to see if there's any landmines. The only interesting thing I found was that we also lock the compositor when we show a new tab. https://cs.chromium.org/chromium/src/content/browser/renderer_host/delegated_frame_host.cc?rcl=1481098841&l=96

That reminds me of https://bugs.chromium.org/p/chromium/issues/detail?id=671188 where I'm thinking about resize and showing tabs sharing mechanisms to prevent drawing the wrong frame. In that case the timeout leads to solid color as we have no fallback frame though.

Comment 21 by fsam...@chromium.org, Dec 7 2016 

I'm confused, how do we do this in process?

The parent doesn't know when the newly allocated LocalFrameId is ready, only the gpu process does.

Comment 22 by danakj@chromium.org, Dec 7 2016 

I mean when the display compositor is still in process in the browser before we move it.

Comment 23 by fsam...@chromium.org, Dec 7 2016 

Ahh yes, I agree. Although I'd like to make sure we agree on a mechanism that allows us to pull the display compositor into the gpu process too :-)

WDYT about effectively delaying the ACK to the parent compositor until a timeout or when the embedded surface ID becomes available, Dana and Antoine?

Comment 24 by danakj@chromium.org, Dec 7 2016 

I think it's fine, its the same throttling mechanism we use for the renderer, and I couldn't find any gotchas from looking at callers of lock methods on ui::Compositor. It would probably end up being simpler, as the compositorlock was designed for ui::compositor being the display compositor.

Comment 25 by jbau...@chromium.org, Dec 7 2016 

So in the case where the SurfaceDrawQuad size is changing, we'd probably need to add resize gutters between the sizes of the two different surface. In exo there are more complicated transititions, like between filter modes and so on, that would look even worse if they're not consistent, though maybe we can just have some dummy black surface as a backup and hope that that never needs to be used.

I'm just worried that with enough clients we'll always be in some inconsistent state, even if clients that work with each other are close to synchronized. Like in the exo case where you have 100 surfaces updating a frame, and the child surface update is always 1ms behind the parent surface update. It's possible that there's never a time within a frame where they're all consistent.

Maybe if we're smart enough about not acking the parent frame, they'll be forced into synchronization this way. Also I guess if we don't mark a parent frame ready until all of its child frames are marked ready (recursively),  then at least we'll have a consistent tree, and maybe the limited height of the tree could prevent it from needing to wait too long in the case where the children redraw promptly.

We'll also have to make sure that surface sequences for dependencies of a frame aren't satisfied until that frame's actually used to draw. I put satisfies_sequences onto cc::CompositorFrameMetadata for this reason, so hopefully that's enough.

Comment 26 by fsam...@chromium.org, Dec 7 2016 

jbauman@ for your 100 surface concern, if we can't do that well in an out-of-process display compositor, doesn't that mean that we're probably skipping frames or something with an in-process display compositor?

Comment 27 by jbau...@chromium.org, Dec 7 2016 

In chrome now with exo we don't have to worry about the child surface update coming after the parent because it's a synchronous function call, so we don't have to worry about things running out of order. So that's the main distinction that I see causing issues in the future.

We also don't try to do anything smart yet with OOPIF, so that manifests now as a pretty predictable latency in resizing them.

Comment 28 by fsam...@chromium.org, Dec 7 2016 

Yea, but if we can never get to a consistent state within a timeout then things must be really slow anyway, and that would have caused us to skip frames in the current world with synchronous function calls, right?

Comment 29 by jbau...@chromium.org, Dec 7 2016 

No it's possible there could be a snapshot that's consistent now, but due to reordering in ipcs it could never happen at one time unless we're really smart about it.

Comment 30 by fsam...@chromium.org, Jan 26 2017

Components: Internals>Compositing

Comment 31 by fsam...@chromium.org, Feb 6 2017

Cc: briander...@chromium.org
I have a working prototype of a synchronization mechanism that also addresses the exo use case. I'll post a design doc to this bug shortly.

Comment 32 by fsam...@chromium.org, Feb 20 2017 

The basics of surface synchronization have landed in cc as of last week. I'll try to post a public draft design doc here this week. The design is relatively straightforward.

A cc::Surface can now hold up to two frames: pending frame and active frame. An active frame is a candidate for display. That is, it may be part of a display frame at some point in the future. A pending frame is a CompositorFrame that refers to surface IDs that have not yet resolved: SurfaceManager does not know about them or they don't yet have an active frame. 

SurfaceDependencyTracker sets a deadline as soon as a CompositorFrame with unresolved dependencies arrives in the display compositor.  This deadline behavior will be refined in the future. SurfaceDependencyTracker also tracks all surfaces with pending Frames, and when their blockers activate. When a pending CompositorFrame's dependencies all activate then it can activate.

If the deadline hits, then all pending frames are forcibly activated.

Comment 33 by fsam...@chromium.org, Apr 17 2017

Owner: samans@chromium.org
Status: Fixed (was: Started)
As far as this bug is concerned, surface IDs are allocated in the renderer now BUT they are not yet allocated by the parent FOR the renderer to submit yet. That part is blocked on surface sync (which is another bug). I'm assigning this to Saman (who fixed this, thanks Saman!) and marking as FIXED>

Comment 34 by fsam...@chromium.org, Jun 13 2017

Blocking: -601863

Comment 35 by lafo...@chromium.org, Feb 26 2018

Components: -Internals>MUS Internals>Services>WindowService

Comment 36 by lafo...@chromium.org, Feb 26 2018

Components: -MUS
►

Sign in to add a comment