New issue
Advanced search Search tips

Issue 670710 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocking:
issue 732594



Sign in to add a comment

ui::Compositor should not know about privileged ContextFactory APIs

Project Member Reported by fsam...@chromium.org, Dec 2 2016

Issue description

In Mus+Ash, ui::Compositor is used by clients. If we wish to use ui::Compositor in clients, then ui::Compositor should not have access to privileged APIs. We should instead provide another mechanism to set privileged APIs.

Privileged APIs in ContextFactory include:

CreateReflector (should be window server)
RemoveReflector (should be window server)
AllocateFrameSinkId (should be window server)

GetSurfaceManager (should be gpu)
SetDisplayVisible (should be gpu)
ResizeDisplay (should be gpu)
SetDisplayColorSpace (should be gpu)
SetAuthoritativeVSyncInterval (should be gpu)
SetDisplayVSyncParameters (should be gpu)
SetOutputIsSecure (should be gpu).

It seems like we can break this class into three classes: client, window server, and gpu responsibilities.




 
Privileged API access should probably be pulled entirely out of ui::Compositor.
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 14 2016

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

commit a0bcfe1819277f44264ef31fdbdc3b59f1970144
Author: fsamuel <fsamuel@chromium.org>
Date: Wed Dec 14 06:21:49 2016

ui + mus: Split ContextFactory into ContextFactory(Client) and ContextFactoryPrivate

ui::ContextFactory is an interface that contains both privileged and
unprivileged operations. This CL dices ContextFactory into two, clearly
delineating operations that can be performed by Mus clients, and operations that
are reserved for mus-ws and mus-gpu and should be refactored.

Mus clients other than Chrome do not need to inject a
ContextFactoryPrivate into ui::Compositor. Chrome still needs
ContextFactoryPrivate because it has its own display compositor for
now.

Once we complete the unified display compositor work across all Chrome
clients, ContextFactoryPrivate can go away entirely.

BUG=670710
TBR=reveman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ash/display/mirror_window_controller.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ash/shell.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ash/shell/content/client/shell_browser_main_parts.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ash/shell_init_params.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ash/test/ash_test_helper.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/chrome/browser/ui/ash/ash_init.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/chrome/browser/ui/views/chrome_views_delegate.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/chrome/browser/ui/views/chrome_views_delegate.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/chrome/test/base/view_event_test_base.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/chrome/test/base/view_event_test_platform_part.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/chrome/test/base/view_event_test_platform_part_chromeos.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/chrome/test/base/view_event_test_platform_part_default.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/components/exo/surface.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/components/exo/surface_unittest.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/browser_main_loop.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/compositor/image_transport_factory.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/compositor/reflector_impl_unittest.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/compositor/software_browser_compositor_output_surface_unittest.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/compositor/software_output_device_ozone_unittest.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/compositor/surface_utils.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/compositor/test/no_transport_image_transport_factory.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/compositor/test/no_transport_image_transport_factory.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/context_factory.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/frame_host/render_widget_host_view_child_frame_unittest.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/frame_host/render_widget_host_view_guest_unittest.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/media/capture/desktop_capture_device_aura_unittest.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/public/browser/context_factory.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/content/public/test/test_renderer_host.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/extensions/shell/browser/shell_browser_main_parts.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/mash/test/mash_test_suite.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/aura/env.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/aura/env.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/aura/mus/mus_context_factory.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/aura/mus/mus_context_factory.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/aura/test/aura_test_base.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/aura/test/aura_test_helper.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/aura/test/aura_test_helper.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/aura/window_tree_host.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/compositor/compositor.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/compositor/compositor.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/compositor/compositor_unittest.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/compositor/layer_animator_unittest.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/compositor/layer_owner_unittest.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/compositor/layer_unittest.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/compositor/test/context_factories_for_test.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/compositor/test/context_factories_for_test.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/compositor/test/in_process_context_factory.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/compositor/test/test_compositor_host.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/compositor/test/test_compositor_host_android.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/compositor/test/test_compositor_host_mac.mm
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/compositor/test/test_compositor_host_ozone.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/compositor/test/test_compositor_host_win.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/compositor/test/test_compositor_host_x11.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/keyboard/keyboard_controller_unittest.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/snapshot/snapshot_aura_unittest.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/views/cocoa/bridged_native_widget.mm
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/views/mus/surface_context_factory.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/views/mus/surface_context_factory.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/views/test/scoped_views_test_helper.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/views/test/test_views_delegate.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/views/test/test_views_delegate_aura.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/views/test/test_views_delegate_mac.mm
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/views/test/views_test_helper.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/views/test/views_test_helper_aura.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/views/test/views_test_helper_aura.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/views/test/views_test_helper_mac.mm
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/views/views_delegate.cc
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/views/views_delegate.h
[modify] https://crrev.com/a0bcfe1819277f44264ef31fdbdc3b59f1970144/ui/views_content_client/views_content_client_main_parts.cc

Here's an easy cleanup:

GpuProcessTransportFactory is a ContextFactory and ContextFactoryPrivate.

GpuProcessTransportFactory::EstablishedGpuChannel sets up a vsync callback to call into ui::Compositor https://cs.chromium.org/chromium/src/content/browser/compositor/gpu_process_transport_factory.cc?rcl=1485087414&l=476

This calls into GpuProcessTransportFactory::SetDisplayVSyncParameters.

I think ideally GpuProcessTransportFactory should take care of everything without involving ui::Compositor.
Owner: samans@chromium.org
Status: Assigned (was: Untriaged)
Saman this seems like a good bug to work on in the background. Thanks!
Blocking: 601863
As far as I can tell, all of the calls on DisplayPrivate should come from mus-ws.
Components: Internals>Compositing
Labels: Type-Feature
Cc: weiliangc@chromium.org
Cc: varkha@chromium.org
Labels: -Pri-3 Pri-2
Blocking: 730193
Owner: ----
Status: Available (was: Assigned)
Blocking: -730193
This isn't needed for bringing up viz-desktop. There are some subpieces but we'll treat that as part of bringing up a mojo connection to display compositor from content which is 730213.

This bug can address getting UI compositor to deal with WindowServer on mus instead of directly mutating the display compositor.
Blocking: -601863 732594
Components: -Internals>MUS
Components: Internals>Services>WindowService
Labels: -Proj-Mustash-Mus-WS
Deprecating Proj-Mustash-Mus-WS in favor of component Internals>Services>WindowService.
Labels: -Proj-Mustash Proj-Mash-MultiProcess
Bug label cleanup.

Labels: -Proj-Mustash-Mus-GPU
Cleaning up old Proj-Mustash labels.

Sign in to add a comment