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

Issue 677704 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Last visit > 30 days ago
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Cleanup Window::RequestCompositorFrameSink API

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

Issue description

Copy-and-paste from comment I made in a CL:

Meta-API-boundary point:

   auto compositor_frame_sink = window->RequestCompositorFrameSink(
      compositor_frame_sink_type,
       gpu_->CreateContextProvider(gpu_->EstablishGpuChannelSync()),
       gpu_->gpu_memory_buffer_manager());

This API seems super weird. We are passing the Mus client library things it
should already know about! ui::Window (aura:Window now?) should know about gpu_,
it should know how to create a contextprovider, how to establish a gpu channel,
etc.

auto compositor_frame_sink = window->RequestCompositorFrameSink(); should be the
API to aspire for, in my opinion. WDYT?
 

Comment 1 by sadrul@chromium.org, Jan 19 2017

This code has since changed to be asynchronous (instead of using the sync version of the API, it uses the async API to get the gpu-channel first). We could move that out of MusContextFactory into WindowPortMus.

Technically though, both are part of the client-lib. So not sure it'd be a huge improvement.
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 20 2017

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

commit 9917dd563899eff0020ca4cad673832b620f86ec
Author: sadrul <sadrul@chromium.org>
Date: Fri Jan 20 18:15:54 2017

aura: Some change to how the client-lib is set up.

Instead of clients having to explicitly create and set up ui::Gpu,
aura::MusContextFactory instances before creating a WindowTreeClient
instance, have WindowTreeClient take care of creating and setting them
up. This allows removal of duplicate set-up code from several places.

BUG= 677704 

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

[modify] https://crrev.com/9917dd563899eff0020ca4cad673832b620f86ec/ash/mus/window_manager_application.cc
[modify] https://crrev.com/9917dd563899eff0020ca4cad673832b620f86ec/ash/mus/window_manager_application.h
[modify] https://crrev.com/9917dd563899eff0020ca4cad673832b620f86ec/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/9917dd563899eff0020ca4cad673832b620f86ec/mash/simple_wm/simple_wm.cc
[modify] https://crrev.com/9917dd563899eff0020ca4cad673832b620f86ec/mash/simple_wm/simple_wm.h
[modify] https://crrev.com/9917dd563899eff0020ca4cad673832b620f86ec/services/ui/demo/mus_demo.cc
[modify] https://crrev.com/9917dd563899eff0020ca4cad673832b620f86ec/services/ui/demo/mus_demo.h
[modify] https://crrev.com/9917dd563899eff0020ca4cad673832b620f86ec/services/ui/test_wm/test_wm.cc
[modify] https://crrev.com/9917dd563899eff0020ca4cad673832b620f86ec/services/ui/ws/window_server_test_base.cc
[modify] https://crrev.com/9917dd563899eff0020ca4cad673832b620f86ec/services/ui/ws/window_server_test_base.h
[modify] https://crrev.com/9917dd563899eff0020ca4cad673832b620f86ec/ui/aura/mus/window_tree_client.cc
[modify] https://crrev.com/9917dd563899eff0020ca4cad673832b620f86ec/ui/aura/mus/window_tree_client.h
[modify] https://crrev.com/9917dd563899eff0020ca4cad673832b620f86ec/ui/views/mus/mus_client.cc
[modify] https://crrev.com/9917dd563899eff0020ca4cad673832b620f86ec/ui/views/mus/mus_client.h

Blocking: 601863
Status: Available (was: Untriaged)
Components: Internals>Compositing
Owner: staraz@chromium.org
Status: Started (was: Available)
Please note that as mentioned in comment #1, this is mostly internal API in the client-lib. Unless rearranging code actually make things cleaner (instead of simply moving the complexity from one place to another), it may not necessarily be worth doing IMO.
Status: Assigned (was: Started)
Status: WontFix (was: Assigned)
Given we won't use the same WindowTreeClient in the renderer, this doesn't seem like a high value refactor anymore. I'm marking as WontFix.
Blocking: -601863
Components: -Internals>MUS Internals>Services>WindowService

Sign in to add a comment