New issue
Advanced search Search tips

Issue 683735 link

Starred by 2 users
Status: Fixed
Owner: ----
Closed: Jan 2017
Cc:
danakj@chromium.org
samans@chromium.org
xing...@intel.com
rjkroege@chromium.org
sadrul@chromium.org
kylec...@chromium.org
mustash-bugs@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

GpuCompositorFrameSink should not depend on DisplayCompositor

Project Member Reported by fsam...@chromium.org, Jan 22 2017 
We'd like to use GpuCompositorFrameSink in Chrome (replace OffscreenCanvasCompositorFrameSink, for example) so ideally, it should not depend on DisplayCompositor just yet. This probably means the following:

1. GpuCompositorFrameSink takes in a SurfaceManager directly instead of a DisplayCompsoitor.

2. DisplayCompositor is a GpuCompositorFrameSinkDelegate for OnClientConnectionLost and OnPrivateConnectionLost.

Comment 1 by fsam...@chromium.org, Jan 22 2017 

Additionally, DisplayCompositor::AddSurfaceReferences and DisplayCompositor::RemoveSurfaceReferences should probably go away.

Comment 2 by fsam...@chromium.org, Jan 22 2017 

Once that is done, I think ReferencedSurfaceTracker related code should move to CompositorFrameSinkSupport so that we can use it in Chrome.

Comment 3 by fsam...@chromium.org, Jan 22 2017

Blocking: 601863

Comment 4 by fsam...@chromium.org, Jan 23 2017

Status: Available (was: Untriaged)

Comment 5 by xing...@intel.com, Jan 24 2017

Cc: xing...@intel.com

Comment 6 by kylec...@chromium.org, Jan 24 2017 

DisplayCompositor::AddSurfaceReferences() / RemoveSurfaceReferences() should be trivial to get rid of now. They just call through to SurfaceManager. I can do that if you want.

What about DisplayCompositor::OnCompositorFrameSinkClientConnectionLost() / OnCompositorFrameSinkPrivateConnectionLost() though?
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 26 2017 

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

commit 81183e45cf14c416dfce69b990a0c71a4e1c3399
Author: xing.xu <xing.xu@intel.com>
Date: Thu Jan 26 04:16:19 2017

Decouple GpuCompositorFrameSink from DisplayCompositor

In order to reuse GpuCompositorFrameSink in other parts of Chrome
(e.g. OffscreenCanvas), this CL decouples
GpuCompositorFrameSinkSupport from DisplayCompositor.

This CL also moves GpuCompositorFrameSInk out of
services/ui/surfaces for now as well because we would like to use
this code in Chrome and this is currently an incomplete internal
implementation detail of Mus. Once code is unified between Chrome
and Mus, components/display_compositor can probably move to
services/ui/gpu/display_compositor.

BUG= 683735 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

[modify] https://crrev.com/81183e45cf14c416dfce69b990a0c71a4e1c3399/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/81183e45cf14c416dfce69b990a0c71a4e1c3399/cc/surfaces/surface_manager.h
[modify] https://crrev.com/81183e45cf14c416dfce69b990a0c71a4e1c3399/cc/surfaces/surface_reference_manager.h
[modify] https://crrev.com/81183e45cf14c416dfce69b990a0c71a4e1c3399/components/display_compositor/BUILD.gn
[modify] https://crrev.com/81183e45cf14c416dfce69b990a0c71a4e1c3399/components/display_compositor/DEPS
[rename] https://crrev.com/81183e45cf14c416dfce69b990a0c71a4e1c3399/components/display_compositor/gpu_compositor_frame_sink.cc
[rename] https://crrev.com/81183e45cf14c416dfce69b990a0c71a4e1c3399/components/display_compositor/gpu_compositor_frame_sink.h
[add] https://crrev.com/81183e45cf14c416dfce69b990a0c71a4e1c3399/components/display_compositor/gpu_compositor_frame_sink_delegate.h
[rename] https://crrev.com/81183e45cf14c416dfce69b990a0c71a4e1c3399/components/display_compositor/gpu_display_compositor_frame_sink.cc
[rename] https://crrev.com/81183e45cf14c416dfce69b990a0c71a4e1c3399/components/display_compositor/gpu_display_compositor_frame_sink.h
[rename] https://crrev.com/81183e45cf14c416dfce69b990a0c71a4e1c3399/components/display_compositor/gpu_offscreen_compositor_frame_sink.cc
[add] https://crrev.com/81183e45cf14c416dfce69b990a0c71a4e1c3399/components/display_compositor/gpu_offscreen_compositor_frame_sink.h
[modify] https://crrev.com/81183e45cf14c416dfce69b990a0c71a4e1c3399/services/ui/surfaces/BUILD.gn
[modify] https://crrev.com/81183e45cf14c416dfce69b990a0c71a4e1c3399/services/ui/surfaces/display_compositor.cc
[modify] https://crrev.com/81183e45cf14c416dfce69b990a0c71a4e1c3399/services/ui/surfaces/display_compositor.h
[delete] https://crrev.com/46c207422a4667f35177488a6af0e90dfbe03532/services/ui/surfaces/gpu_offscreen_compositor_frame_sink.h
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 26 2017 

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

commit c850f7de840cdb2c21ee10166a76c2acbf04cc01
Author: dbeam <dbeam@chromium.org>
Date: Thu Jan 26 05:29:39 2017

Revert of Decouple GpuCompositorFrameSink from DisplayCompositor (patchset #7 id:120001 of https://codereview.chromium.org/2654693003/ )

Reason for revert:
Broke Win 64 builder's compile step:
https://build.chromium.org/p/chromium/builders/Win%20x64/builds/7950

[23910/48389] CXX obj/content/app/content_main_runner_child/content_main_runner.obj
FAILED: obj/content/app/content_main_runner_child/content_main_runner.obj
ninja -t msvc -e environment.x64 -- C:\b\c\cipd\goma/gomacc.exe "C:\b\depot_tools\win_toolchain\vs_files\d3cb0e37bdd120ad0ac4650b674b09e81be45616\VC\bin\amd64/cl.exe" /nologo /showIncludes /FC @obj/content/app/content_main_runner_child/content_main_runner.obj.rsp /c ../../content/app/content_main_runner.cc /Foobj/content/app/content_main_runner_child/content_main_runner.obj /Fd"obj/content/app/content_main_runner_child_cc.pdb"
c:\b\c\b\win_x64_archive\src\content\browser\gpu\gpu_process_host.h(32): fatal error C1083: Cannot open include file: 'services/ui/gpu/interfaces/gpu_host.mojom.h': No such file or directory

Original issue's description:
> Decouple GpuCompositorFrameSink from DisplayCompositor
>
> In order to reuse GpuCompositorFrameSink in other parts of Chrome
> (e.g. OffscreenCanvas), this CL decouples
> GpuCompositorFrameSinkSupport from DisplayCompositor.
>
> This CL also moves GpuCompositorFrameSInk out of
> services/ui/surfaces for now as well because we would like to use
> this code in Chrome and this is currently an incomplete internal
> implementation detail of Mus. Once code is unified between Chrome
> and Mus, components/display_compositor can probably move to
> services/ui/gpu/display_compositor.
>
> BUG= 683735 
> CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
>
> Review-Url: https://codereview.chromium.org/2654693003
> Cr-Commit-Position: refs/heads/master@{#446223}
> Committed: https://chromium.googlesource.com/chromium/src/+/81183e45cf14c416dfce69b990a0c71a4e1c3399

TBR=fsamuel@chromium.org,ben@chromium.org,kylechar@chromium.org,jam@chromium.org,danakj@chromium.org,xing.xu@intel.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 683735 

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

[modify] https://crrev.com/c850f7de840cdb2c21ee10166a76c2acbf04cc01/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/c850f7de840cdb2c21ee10166a76c2acbf04cc01/cc/surfaces/surface_manager.h
[modify] https://crrev.com/c850f7de840cdb2c21ee10166a76c2acbf04cc01/cc/surfaces/surface_reference_manager.h
[modify] https://crrev.com/c850f7de840cdb2c21ee10166a76c2acbf04cc01/components/display_compositor/BUILD.gn
[modify] https://crrev.com/c850f7de840cdb2c21ee10166a76c2acbf04cc01/components/display_compositor/DEPS
[delete] https://crrev.com/fc1f65c1189b644cd27c8a5fa47861dd4f68859d/components/display_compositor/gpu_compositor_frame_sink_delegate.h
[delete] https://crrev.com/fc1f65c1189b644cd27c8a5fa47861dd4f68859d/components/display_compositor/gpu_offscreen_compositor_frame_sink.h
[modify] https://crrev.com/c850f7de840cdb2c21ee10166a76c2acbf04cc01/services/ui/surfaces/BUILD.gn
[modify] https://crrev.com/c850f7de840cdb2c21ee10166a76c2acbf04cc01/services/ui/surfaces/display_compositor.cc
[modify] https://crrev.com/c850f7de840cdb2c21ee10166a76c2acbf04cc01/services/ui/surfaces/display_compositor.h
[rename] https://crrev.com/c850f7de840cdb2c21ee10166a76c2acbf04cc01/services/ui/surfaces/gpu_compositor_frame_sink.cc
[rename] https://crrev.com/c850f7de840cdb2c21ee10166a76c2acbf04cc01/services/ui/surfaces/gpu_compositor_frame_sink.h
[rename] https://crrev.com/c850f7de840cdb2c21ee10166a76c2acbf04cc01/services/ui/surfaces/gpu_display_compositor_frame_sink.cc
[rename] https://crrev.com/c850f7de840cdb2c21ee10166a76c2acbf04cc01/services/ui/surfaces/gpu_display_compositor_frame_sink.h
[rename] https://crrev.com/c850f7de840cdb2c21ee10166a76c2acbf04cc01/services/ui/surfaces/gpu_offscreen_compositor_frame_sink.cc
[add] https://crrev.com/c850f7de840cdb2c21ee10166a76c2acbf04cc01/services/ui/surfaces/gpu_offscreen_compositor_frame_sink.h

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

Components: Internals>Compositing
Project Member

Comment 10 by bugdroid1@chromium.org, Jan 27 2017 

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

commit c6c67d47aec05f4dc57da4ac6bd92f617ecac9e1
Author: xing.xu <xing.xu@intel.com>
Date: Fri Jan 27 02:21:20 2017

Decouple GpuCompositorFrameSink from DisplayCompositor

In order to reuse GpuCompositorFrameSink in other parts of Chrome
(e.g. OffscreenCanvas), this CL decouples
GpuCompositorFrameSinkSupport from DisplayCompositor.

This CL also moves GpuCompositorFrameSInk out of
services/ui/surfaces for now as well because we would like to use
this code in Chrome and this is currently an incomplete internal
implementation detail of Mus. Once code is unified between Chrome
and Mus, components/display_compositor can probably move to
services/ui/gpu/display_compositor.

BUG= 683735 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2654693003
Cr-Original-Commit-Position: refs/heads/master@{#446223}
Committed: https://chromium.googlesource.com/chromium/src/+/81183e45cf14c416dfce69b990a0c71a4e1c3399
Review-Url: https://codereview.chromium.org/2654693003
Cr-Commit-Position: refs/heads/master@{#446540}

[modify] https://crrev.com/c6c67d47aec05f4dc57da4ac6bd92f617ecac9e1/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/c6c67d47aec05f4dc57da4ac6bd92f617ecac9e1/cc/surfaces/surface_manager.h
[modify] https://crrev.com/c6c67d47aec05f4dc57da4ac6bd92f617ecac9e1/cc/surfaces/surface_reference_manager.h
[modify] https://crrev.com/c6c67d47aec05f4dc57da4ac6bd92f617ecac9e1/components/display_compositor/BUILD.gn
[modify] https://crrev.com/c6c67d47aec05f4dc57da4ac6bd92f617ecac9e1/components/display_compositor/DEPS
[rename] https://crrev.com/c6c67d47aec05f4dc57da4ac6bd92f617ecac9e1/components/display_compositor/gpu_compositor_frame_sink.cc
[rename] https://crrev.com/c6c67d47aec05f4dc57da4ac6bd92f617ecac9e1/components/display_compositor/gpu_compositor_frame_sink.h
[add] https://crrev.com/c6c67d47aec05f4dc57da4ac6bd92f617ecac9e1/components/display_compositor/gpu_compositor_frame_sink_delegate.h
[rename] https://crrev.com/c6c67d47aec05f4dc57da4ac6bd92f617ecac9e1/components/display_compositor/gpu_display_compositor_frame_sink.cc
[rename] https://crrev.com/c6c67d47aec05f4dc57da4ac6bd92f617ecac9e1/components/display_compositor/gpu_display_compositor_frame_sink.h
[rename] https://crrev.com/c6c67d47aec05f4dc57da4ac6bd92f617ecac9e1/components/display_compositor/gpu_offscreen_compositor_frame_sink.cc
[add] https://crrev.com/c6c67d47aec05f4dc57da4ac6bd92f617ecac9e1/components/display_compositor/gpu_offscreen_compositor_frame_sink.h
[modify] https://crrev.com/c6c67d47aec05f4dc57da4ac6bd92f617ecac9e1/services/ui/surfaces/BUILD.gn
[modify] https://crrev.com/c6c67d47aec05f4dc57da4ac6bd92f617ecac9e1/services/ui/surfaces/display_compositor.cc
[modify] https://crrev.com/c6c67d47aec05f4dc57da4ac6bd92f617ecac9e1/services/ui/surfaces/display_compositor.h
[delete] https://crrev.com/03d77dd3e66409c87e9654f3db112fe7b298f925/services/ui/surfaces/gpu_offscreen_compositor_frame_sink.h

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

Status: Fixed (was: Available)
I'm marking this as FIXED! Thank you for your help Xing :-)

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

Blocking: -601863

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

Components: -Internals>MUS Internals>Services>WindowService
►

Sign in to add a comment