New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 4 users
Status: Fixed
Owner:
Closed: May 9
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment
Transition OffscreenCanvas to new FrameSinkManagerHost Architecture
Project Member Reported by fsam...@chromium.org, Nov 11 2016 Back to list
OffscreenCanvas needs to be transitioned to the new display compositor architecture to allow us to move the display compositor to the gpu process.

This roughly captures what I think we need to do. We should stay in sync with the offscreen canvas team to make sure we converge on a good API for both Mus and Offscreen Canvas.

1. Each HTMLCanvasElement should get a unique routing ID and thus FrameSinkId = (client ID, routing ID).

2. OffscreenCanvas should use SurfaceIdAllocator in the renderer and allocate a new LocalFrameId on resize.

3. MojoCompositorFrameSink::SubmitCompositorFrame should change to take in a LocalFrameId. We can make the LocalFrameId optional until we transition to the new API (note that you cannot transmit an invalid LocalFrameId over IPC for security reasons: that ID must be unguessable across IPC boundaries).


4. OffscreenCanvasSurfaceImpl::GetSurfaceId should go away.

5. OffscreenCanvasSurfaceImpl ~= what I've been calling "DisplayCompositor". It provides access to surface references and allocates new SurfaceIds. We should instead use the DisplayCompositor interface for that.

6. Code shared between GpuCompositorFrameSink and OffscreenCanvasCompositorFrameSink should be refactored into cc/surfaces/compositor_frame_sink_support.{h,cc}


 
Note that RoutingID is allocated synchronously in the renderer today: RenderThreadImpl::GenerateRoutingID, but we only need a routing ID when we create the HTMLCanvasElement. We can resize, and not allocate a new routing ID.
Comment 2 by xlai@chromium.org, Nov 11 2016
I will try to complete the Part 1 in this CL (https://codereview.chromium.org/2479563005/). And to clarify, it is not "each HTMLCanvasElement"; rather, it is "each HTMLCanvasElement that has transferred control to Offscreen".
Comment 3 by junov@chromium.org, Nov 11 2016
Labels: OffScreenCanvas
Owner: xlai@chromium.org
Status: Assigned
@xlai: Assigning to you to take the lead on this. Feel free to farm-out work to the rest of the team if you need support.

@fsamuel: Do you have a deadline for us?  We are pretty loaded for m-57? Would M-58 be okay?

junov@: M58 would be too late for us, I think. I'd really prefer M57. Note that we can help here, but I'd just like folks to keep this in mind as they're working on offscreen canvas features.

For example, I'd like resize to be implemented using surface IDs allocated in the client instead of the browser. You're going to need to implement resize anyway, and this is only slightly more work. Thanks.

5-6 are things the Mus team can take care of.
Comment 5 by junov@chromium.org, Nov 14 2016
Blocking: 563816
If M-58 is too late, then let's make this bug required for shipping.  Basically, if we don't resolve this in time for M-57, then OffscreenCanvas won't ship in 57.

Project Member Comment 6 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

Project Member Comment 7 by bugdroid1@chromium.org, Nov 25 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2956c331e20ecb5d730d17fdf637ee72b6fd3a6c

commit 2956c331e20ecb5d730d17fdf637ee72b6fd3a6c
Author: xlai <xlai@chromium.org>
Date: Fri Nov 25 17:45:37 2016

Allow Blink to generate cc::FrameSinkId from RenderThread

This CL allows Blink to use generate cc::FrameSinkId, internally
implemented using RenderThread's functions.

Currently, Mus is working on making FrameSinkId being able to
generated on Renderer process, getting rid of the need of IPC
to browser just to get a FrameSinkId.

By making this generation function available in Blink, this CL
prepares for a refactoring effort on the current
OffscreenCanvas.commit() workflow--it would facilitate
simplification on CanvasSurfaceLayerBridge
that will remove the current sync IPC that relies on the
browser process to generate  FrameSinkId (which is part of
SurfaceId) for it.

BUG= 664547 

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

[modify] https://crrev.com/2956c331e20ecb5d730d17fdf637ee72b6fd3a6c/content/renderer/renderer_blink_platform_impl.cc
[modify] https://crrev.com/2956c331e20ecb5d730d17fdf637ee72b6fd3a6c/content/renderer/renderer_blink_platform_impl.h
[modify] https://crrev.com/2956c331e20ecb5d730d17fdf637ee72b6fd3a6c/third_party/WebKit/public/platform/Platform.h

Blocking: 601863
Blocking:
Components: MUS
Labels: displaycompositor
Now that this has landed: https://codereview.chromium.org/2543373002/ it ought to be possible to easily convert OffscreenCanvasCompositorFrameSink to use CompositorFrameSinkSupport. I'll take care of that bit of the work.



Project Member Comment 11 by bugdroid1@chromium.org, Dec 14 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d5db7e2580f918cbbaab7b39deddd3e9344946b8

commit d5db7e2580f918cbbaab7b39deddd3e9344946b8
Author: fsamuel <fsamuel@chromium.org>
Date: Wed Dec 14 19:01:24 2016

OffscreenCanvasCompositorFrameSink uses CompositorFrameSinkSupport

This CL is a mechanical refactor to transition
OffscreenCanvasCompositorFrameSink to use CompositorFrameSinkSupport,
enabling it to share code today with Mus and Exo.

BUG= 664547 

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

[modify] https://crrev.com/d5db7e2580f918cbbaab7b39deddd3e9344946b8/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.cc
[modify] https://crrev.com/d5db7e2580f918cbbaab7b39deddd3e9344946b8/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.h
[modify] https://crrev.com/d5db7e2580f918cbbaab7b39deddd3e9344946b8/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink_provider_impl.cc

Project Member Comment 12 by bugdroid1@chromium.org, Dec 17 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/05aa05390801f402c336ce2156014d7ef055e9ef

commit 05aa05390801f402c336ce2156014d7ef055e9ef
Author: xlai <xlai@chromium.org>
Date: Sat Dec 17 22:56:36 2016

Revamp OffscreenCanvas commit flow

This patch aims to simplify the existing work flow of OffscreenCanvas commit()
and help OffscreenCanvas transition to the new Display Compositor architecture.
This facilitates the next step of adding resizing functionality of
OffscreenCanvas.

Major changes:
1. OffscreenCanvas now use SurfaceIdAllocator in the renderer/worker, instead
of being in the browser. This is consistent with the new display compositor
architecture.
2. SurfaceLayer for HTMLCanvasElement is only created when Surface is created
in the browser; before that, a dummy SolidColorLayer is put as a placeholder.
3. CanvasSurfaceLayerBridge becomes the client of OffscreenCanvasSurfaceImpl,
so that it can receive IPC messages from browser.
4. FrameSinkId is now generated in renderer/main by CanvasSurfaceLayerBridge,
which remains unchanged throughout the lifecycle of canvas; LocalFrameId is
generated in renderer/worker by OffscreenCanvasFrameDispatcher. At this time it
is only generated once, but will be generated repeatedly when resizing
functionality is added in the future patches.

What is being simplified:
1. Sync IPC GetSurfaceId is removed, reducing overhead in HTMLCanvasElement.
transferControlToOffscreen() API call.
2. When OffscreenCanvas is transferred from main thread to worker thread, there
are now only 2 integers that need to be copied, which constitute the
FrameSinkId. Before this, there were 5 integers that need to be copied. As a
result, there are a lot less attributes that need to be kept by OffscreenCanvas
and CanvasSurfaceLayerBridge.

TBR=avi@chromium.org
BUG= 664547 ,  619138 ,  662498 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/content/browser/BUILD.gn
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink_provider_impl.cc
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink_provider_impl.h
[add] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/content/browser/renderer_host/offscreen_canvas_surface_factory_impl.cc
[add] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/content/browser/renderer_host/offscreen_canvas_surface_factory_impl.h
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/content/browser/renderer_host/offscreen_canvas_surface_impl.cc
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/content/browser/renderer_host/offscreen_canvas_surface_impl.h
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/content/browser/renderer_host/offscreen_canvas_surface_manager.cc
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/content/browser/renderer_host/offscreen_canvas_surface_manager.h
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/content/browser/renderer_host/offscreen_canvas_surface_manager_unittest.cc
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/content/public/app/mojo/content_browser_manifest.json
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.h
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueDeserializer.cpp
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueSerializer.cpp
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/third_party/WebKit/Source/core/html/HTMLCanvasElement.h
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.h
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/third_party/WebKit/Source/modules/canvas/HTMLCanvasElementModule.cpp
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.h
[delete] https://crrev.com/facf9c3631846243635ca6c98805bb443ad982ca/third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridgeTest.cpp
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.h
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/third_party/WebKit/public/blink_typemaps.gni
[modify] https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef/third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom

Comment 13 by xlai@chromium.org, Dec 19 2016
Owner: fsam...@chromium.org
So far, I think on OffscreenCanvas side, we have completed things that facilitate the ongoing transition work of Display Compositor architecture.
In particular, item  1, 2, 4 mentioned in this issue description has been done by refs/heads/master@{#439356}. I also saw that the Mus team has
completed 3, and probably a partial code on 5, 6.

I shall now pass this issue to fsamuel@ to double check if this issue is completed or what's remaining to be done for MUS team.
Comment 14 by xlai@chromium.org, Jan 3 2017
Blocking: -563816
The remaining part of this job is about refactoring and code re-organization outside OffscreenCanvas's core functions. IMHO, this should no longer block
launch of OffscreenCanvas.
Blocking:
Things to do:

1. OffscreenCanvasCompositorFrameSinkProviderImpl and DisplayCompositor should merge, and go to a central location that is always running. Maybe it should be owned by ContextFactoryPrivate for now?

2. OffscreenCanvasCompositorFrameSink and GpuCompositorFrameSink should merge and live in a central location. Perhaps, cc/surfaces? This is icky because it's not clear what's part of the client library and what's an internal implementation detail.
Can service side stuff go in components/display_compositor
#17: SGTM. I think I have a separate bug that says services/ui/surfaces => components/display_compositor.

Once Mus is a bit more mature, I think we want all ui and compositing things to live in services/ui, but we're not there yet.

I prefer to keep Mus+Ash specific code in services/ui for now perhaps for staging and experimentation purposes, and code that is baking in production outside of services/ui. Once Mus is more mature, and outside code code is well cooked, we can move it back to services/ui.
Components: Internals>Compositing
Labels: -Pri-3 Pri-2
Status: Available
I'm not actively working on this right now as I'm still working on surface synchronziaton. I'm marking this bug as Available. kylechar@, samans@, staraz@, this bug is fair game for anyone who wants to pick it up. I'm also marking this as P2 as I think we need to get this done this quarter.

Here are the remaining work items:

1. OffscreenCanvasCompositorFrameSinkProviderImpl and DisplayCompositor should merge, and go to a central location that is always running. Maybe it should be owned by ContextFactoryPrivate for now? As per danakj@'s suggestion, DisplayCompositor should probably also move to components/display_compositor. Someday it'll move back to services/ui/gpu/display_compositor once Mus is more baked.

2. OffscreenCanvasCompositorFrameSink and GpuCompositorFrameSink should merge and live in a central location. Perhaps, cc/surfaces? This is icky because it's not clear what's part of the client library and what's an internal implementation detail.
Owner: ----
+sadrul@ for reference
Project Member Comment 22 by bugdroid1@chromium.org, Feb 15 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ac6bbf510624dc7116c6ad4ce489a42b9e6111aa

commit ac6bbf510624dc7116c6ad4ce489a42b9e6111aa
Author: xing.xu <xing.xu@intel.com>
Date: Wed Feb 15 01:12:32 2017

Replace OffscreenCanvasSurfaceClient with DisplayCompositorClient

This is part of moving OffscreenCanvas into new display compositor.

BUG= 686861 , 664547 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

[modify] https://crrev.com/ac6bbf510624dc7116c6ad4ce489a42b9e6111aa/cc/ipc/BUILD.gn
[modify] https://crrev.com/ac6bbf510624dc7116c6ad4ce489a42b9e6111aa/components/display_compositor/BUILD.gn
[modify] https://crrev.com/ac6bbf510624dc7116c6ad4ce489a42b9e6111aa/content/browser/renderer_host/offscreen_canvas_surface_factory_impl.cc
[modify] https://crrev.com/ac6bbf510624dc7116c6ad4ce489a42b9e6111aa/content/browser/renderer_host/offscreen_canvas_surface_factory_impl.h
[modify] https://crrev.com/ac6bbf510624dc7116c6ad4ce489a42b9e6111aa/content/browser/renderer_host/offscreen_canvas_surface_impl.cc
[modify] https://crrev.com/ac6bbf510624dc7116c6ad4ce489a42b9e6111aa/content/browser/renderer_host/offscreen_canvas_surface_impl.h
[modify] https://crrev.com/ac6bbf510624dc7116c6ad4ce489a42b9e6111aa/content/browser/renderer_host/offscreen_canvas_surface_manager_unittest.cc
[modify] https://crrev.com/ac6bbf510624dc7116c6ad4ce489a42b9e6111aa/services/ui/gpu/interfaces/BUILD.gn
[modify] https://crrev.com/ac6bbf510624dc7116c6ad4ce489a42b9e6111aa/services/ui/surfaces/BUILD.gn
[modify] https://crrev.com/ac6bbf510624dc7116c6ad4ce489a42b9e6111aa/services/ui/ws/BUILD.gn
[modify] https://crrev.com/ac6bbf510624dc7116c6ad4ce489a42b9e6111aa/third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.h
[modify] https://crrev.com/ac6bbf510624dc7116c6ad4ce489a42b9e6111aa/third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom

Some additional context here: https://codereview.chromium.org/2688593002/

Owner: staraz@chromium.org
Status: Started
Owner: ----
Status: Available
Owner: kylec...@chromium.org
Project Member Comment 27 by bugdroid1@chromium.org, Mar 9 2017
Status: Started
Comment 29 Deleted
Comment 30 by xing...@intel.com, Mar 17 2017
I am also interested with this issue.
My two thoughts about this issue:
1), Where to put DisplayCompositor?
In Fady's WIP patch https://codereview.chromium.org/2688593002/, it is initialized by GpuProcessTransportFactory. 
In my WIP patch, https://codereview.chromium.org/2703503002/, it is initialized by RenderProcessHostImpl.

Because DisplayCompositor is used browser-wise, this is similar to GpuProcessTransportFactory. While RenderProcessHostImpl is used per site instance.
So GpuProcessTransportFactory will be a better place to put this.
But GpuProcessTransportFactory has no knowledge about all the related mojo IPC. So it has to setup the IPC stuff, such as create a mojo Service for DisplayCompositor?

2), Offscreen canvas is based on CanvasSurfaceLayerBridge. While CanvasSurfaceLayerBridge is based on GraphicsLayer. You know, in SPv2(http://dev.chromium.org/blink/slimming-paint), GraphicsLayer will be abandoned. So should we take this into consider before we going far?

BTW, I think OffscreenCanvas into new display compositor requires a architecture level design first in Ubuntu/Aura.
(1) RenderProcessHostImpl is the wrong place for DisplayCompositor. There needs to be a single instance.

(2) I'm pretty sure how offscreen canvas is implemented is an orthogonal problem to how it's MojoCompositorFrameSink connection is established. I'm not familiar with the details of SPv2 really.
Project Member Comment 32 by bugdroid1@chromium.org, Mar 28 2017
Project Member Comment 33 by bugdroid1@chromium.org, Apr 6 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3

commit 3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3
Author: kylechar <kylechar@chromium.org>
Date: Thu Apr 06 21:47:11 2017

Create //components/viz/frame_sinks and move code.

Move all of the existing code for what used to be called
DisplayCompositor and is now MojoFrameSinkManager from various places to
a single target under src/components/viz/. This is the chunk of code that
both mus and non-mus will use to implement the FrameSinkManager
interface.

BUG= 664547 

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

[modify] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/components/display_compositor/BUILD.gn
[add] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/components/viz/DEPS
[add] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/components/viz/frame_sinks/BUILD.gn
[add] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/components/viz/frame_sinks/DEPS
[rename] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/components/viz/frame_sinks/display_provider.h
[rename] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/components/viz/frame_sinks/gpu_compositor_frame_sink.cc
[rename] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/components/viz/frame_sinks/gpu_compositor_frame_sink.h
[rename] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/components/viz/frame_sinks/gpu_compositor_frame_sink_delegate.h
[rename] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/components/viz/frame_sinks/gpu_root_compositor_frame_sink.cc
[rename] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/components/viz/frame_sinks/gpu_root_compositor_frame_sink.h
[rename] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/components/viz/frame_sinks/mojo_frame_sink_manager.cc
[rename] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/components/viz/frame_sinks/mojo_frame_sink_manager.h
[modify] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/services/ui/gpu/BUILD.gn
[modify] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/services/ui/gpu/DEPS
[modify] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/services/ui/gpu/gpu_main.cc
[modify] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/services/ui/gpu/gpu_main.h
[modify] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/services/ui/surfaces/BUILD.gn
[modify] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/services/ui/surfaces/DEPS
[modify] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/services/ui/surfaces/mus_display_provider.cc
[modify] https://crrev.com/3cf0cf68b3930de4aaac19a00ed8b8f0fd0e6ee3/services/ui/surfaces/mus_display_provider.h

Project Member Comment 35 by bugdroid1@chromium.org, Apr 10 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/69bd9163b311213bb181832c104e4cab2c2e12b7

commit 69bd9163b311213bb181832c104e4cab2c2e12b7
Author: kylechar <kylechar@chromium.org>
Date: Mon Apr 10 23:45:48 2017

Introduce FrameSinkManagerHost in content/browser/.

FrameSinkManagerHost is an object that will live in the browser process
and manage CompositorFrameSinks. The browser process accesses
SurfaceManager directly to do this now. The goal is eliminate all
direct access to SurfaceManager and move it to another process.

FrameSinkManagerHost owns MojoFrameSinkManager in the short term but
still uses Mojo to access it. The only impact of this change is the
ownership of SurfaceManager is moved. Subsequent CLs can start using
FrameSinkManagerHost to manage CompositorFrameSinks and eliminate direct
access to SurfaceManager.

BUG=657959, 664547 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/BUILD.gn
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/DEPS
[add] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/frame_sink_manager_host.cc
[add] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/frame_sink_manager_host.h
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/image_transport_factory.h
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/surface_utils.cc
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/surface_utils.h
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/test/no_transport_image_transport_factory.cc
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/test/no_transport_image_transport_factory.h
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/renderer_host/compositor_impl_android.h

Project Member Comment 36 by bugdroid1@chromium.org, Apr 13 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/30b894d4f8a1c58183749fb7e0982f53dba0ba40

commit 30b894d4f8a1c58183749fb7e0982f53dba0ba40
Author: kylechar <kylechar@chromium.org>
Date: Thu Apr 13 12:09:25 2017

Convert offscreen canvas to use FrameSinkManagerHost.

This CL switches the content/browser/ part of offscreen canvas to use
FrameSinkManagerHost instead of directly accessing SurfaceMananger.

OffscreenCanvasProvider(Impl) exists for the renderer to request other
interfaces from the browser. The renderer uses it create both a
OffscreenCanvasSurface and MojoCompositorFrameSink. This is a
simplification of OffscreenCanvasCompositorFrameSinkProvider and
OffscreenCanvasSurfaceFactory interfaces.

RenderProcessHostImpl creates a single OffscreenCanvasProviderImpl on
demand and uses it for all offscreen canvas requests from the renderer.

The renderer will first request a OffscreenCanvasSurface and specify the
FrameSinkId. This always happens from the main renderer thread. A
OffscreenCanvasSurfaceImpl will be created for the browser side of this
connection.

The renderer will then likely request MojoCompositorFrameSink for the
FrameSinkId, from either the main thread or a worker thread. This
request is forwarded to MojoFrameSinkManager. This sets up two
connections. There is a MojoCompositorFrameSink from
MojoFrameSinkManager to the renderer. This is used to submit
CompositorFrames. There is MojoCompositorFrameSinkPrivate from
OffscreenCanvasSurfaceImpl to MojoFrameSinkManager. This gives the
browser control over the CompositorFrameSink created in
MojoFrameSinkManager.

The render can close the OffscreenCanvasSurface and
MojoCompositorFrameSink connections in any order. The assumption is they
will both be closed at approximately the same time.

The remaining direct access to SurfaceManager by offscreen canvas code
is blocked on other features being completed.

BUG= 664547 , 686861 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/30b894d4f8a1c58183749fb7e0982f53dba0ba40/content/browser/BUILD.gn
[delete] https://crrev.com/6044a51d3e0eff211412224d21f62777db7641a4/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.cc
[delete] https://crrev.com/6044a51d3e0eff211412224d21f62777db7641a4/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.h
[modify] https://crrev.com/30b894d4f8a1c58183749fb7e0982f53dba0ba40/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink_manager.cc
[modify] https://crrev.com/30b894d4f8a1c58183749fb7e0982f53dba0ba40/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink_manager.h
[delete] https://crrev.com/6044a51d3e0eff211412224d21f62777db7641a4/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink_provider_impl.cc
[delete] https://crrev.com/6044a51d3e0eff211412224d21f62777db7641a4/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink_provider_impl.h
[add] https://crrev.com/30b894d4f8a1c58183749fb7e0982f53dba0ba40/content/browser/renderer_host/offscreen_canvas_provider_impl.cc
[add] https://crrev.com/30b894d4f8a1c58183749fb7e0982f53dba0ba40/content/browser/renderer_host/offscreen_canvas_provider_impl.h
[delete] https://crrev.com/6044a51d3e0eff211412224d21f62777db7641a4/content/browser/renderer_host/offscreen_canvas_surface_factory_impl.cc
[delete] https://crrev.com/6044a51d3e0eff211412224d21f62777db7641a4/content/browser/renderer_host/offscreen_canvas_surface_factory_impl.h
[modify] https://crrev.com/30b894d4f8a1c58183749fb7e0982f53dba0ba40/content/browser/renderer_host/offscreen_canvas_surface_impl.cc
[modify] https://crrev.com/30b894d4f8a1c58183749fb7e0982f53dba0ba40/content/browser/renderer_host/offscreen_canvas_surface_impl.h
[modify] https://crrev.com/30b894d4f8a1c58183749fb7e0982f53dba0ba40/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/30b894d4f8a1c58183749fb7e0982f53dba0ba40/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/30b894d4f8a1c58183749fb7e0982f53dba0ba40/content/public/app/mojo/content_browser_manifest.json
[modify] https://crrev.com/30b894d4f8a1c58183749fb7e0982f53dba0ba40/third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp
[modify] https://crrev.com/30b894d4f8a1c58183749fb7e0982f53dba0ba40/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
[modify] https://crrev.com/30b894d4f8a1c58183749fb7e0982f53dba0ba40/third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom

Project Member Comment 37 by bugdroid1@chromium.org, Apr 18 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c09280da3c8a6c4c7e064ddbf7851498abb577d

commit 8c09280da3c8a6c4c7e064ddbf7851498abb577d
Author: kylechar <kylechar@chromium.org>
Date: Tue Apr 18 18:47:33 2017

Check renderer client id for offscreen canvas.

Add a check before creating an OffscreenCanvasSurfaceImpl or
MojoCompositorFrameSink for offscreen canvas. The renderer should only
be using FrameSinkIds within it's own namespace.

Without this check, a malicious renderer could learn about SurfaceIds
from other clients by guessing the FrameSinkId. After creating an
OffscreenCanvasSurfaceImpl any new SurfaceIds for the FrameSinkId get
forwarded back to the renderer.

BUG= 664547 
TBR=piman@chromium.org

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

[modify] https://crrev.com/8c09280da3c8a6c4c7e064ddbf7851498abb577d/content/browser/renderer_host/offscreen_canvas_provider_impl.cc
[modify] https://crrev.com/8c09280da3c8a6c4c7e064ddbf7851498abb577d/content/browser/renderer_host/offscreen_canvas_provider_impl.h
[modify] https://crrev.com/8c09280da3c8a6c4c7e064ddbf7851498abb577d/content/browser/renderer_host/render_process_host_impl.cc

Components: Internals>MUS
Labels: Type-Feature
Project Member Comment 39 by bugdroid1@chromium.org, Apr 24 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/88bb958c59b218a5fe595ffe3ace9f11f21e28db

commit 88bb958c59b218a5fe595ffe3ace9f11f21e28db
Author: kylechar <kylechar@chromium.org>
Date: Mon Apr 24 20:22:50 2017

Reintroduce OffscreenCanvasSurfaceClient interface.

This interface got switched out for DisplayCompositorClient, which
became FrameSinkManagerClient, in http://crrev.com/2695603002.

The two interfaces were very similar, both having just
OnSurfaceCreated() but with different intentions. FrameSinkManagerClient
is intended for the single GPU to browser connection.
OffscreenCanvasSurfaceClient is intended for multiple browser to
renderer connections. This reintroduces OffscreenCanvasSurfaceClient for
clarity.

BUG= 664547 

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

[modify] https://crrev.com/88bb958c59b218a5fe595ffe3ace9f11f21e28db/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink_manager_unittest.cc
[modify] https://crrev.com/88bb958c59b218a5fe595ffe3ace9f11f21e28db/content/browser/renderer_host/offscreen_canvas_provider_impl.cc
[modify] https://crrev.com/88bb958c59b218a5fe595ffe3ace9f11f21e28db/content/browser/renderer_host/offscreen_canvas_provider_impl.h
[modify] https://crrev.com/88bb958c59b218a5fe595ffe3ace9f11f21e28db/content/browser/renderer_host/offscreen_canvas_surface_impl.cc
[modify] https://crrev.com/88bb958c59b218a5fe595ffe3ace9f11f21e28db/content/browser/renderer_host/offscreen_canvas_surface_impl.h
[modify] https://crrev.com/88bb958c59b218a5fe595ffe3ace9f11f21e28db/content/test/BUILD.gn
[modify] https://crrev.com/88bb958c59b218a5fe595ffe3ace9f11f21e28db/third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.h
[modify] https://crrev.com/88bb958c59b218a5fe595ffe3ace9f11f21e28db/third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom

Summary: Transition OffscreenCanvas to new FrameSinkManagerHost Architecture (was: Transition OffscreenCanvas to new Display Compositor Architecture)
Cc: weiliangc@chromium.org
Project Member Comment 42 by bugdroid1@chromium.org, May 9
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c9d0c8089e8216975a893b85f078f0fb4d044daf

commit c9d0c8089e8216975a893b85f078f0fb4d044daf
Author: kylechar <kylechar@chromium.org>
Date: Tue May 09 17:06:18 2017

Offscreen canvas using FrameSinkManagerHost fully.

This CL contains a number of related changes to offscreen canvas code so
that it's using FrameSinkManagerHost for everything (except surface
sequences) instead of SurfaceManager.

1. Merge OffscreenCanvasCompositorFrameSinkManager and
   OffscreenCanvasProviderImpl.
2. Offscreen canvases for a renderer are scoped to the
   OffscreenCanvasProviderImpl for that renderer.
3. OffscreenCanvasSurfaceImpls are owned by the
   OffscreenCanvasPorviderImpl instead of being StrongBinding<>s. This
   gives them a better defined life and ensures they're cleaned up with
   the rest of the renderer state.
4. Add tests for offscreen canvas code that exerecises Mojo connections.
   This should greatly improve the test coverage and better simulate
   connections from renderer to browser

BUG= 664547 

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

[modify] https://crrev.com/c9d0c8089e8216975a893b85f078f0fb4d044daf/content/browser/BUILD.gn
[delete] https://crrev.com/f6500c14fa6592426db531673f484877cacdd651/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink_manager.cc
[delete] https://crrev.com/f6500c14fa6592426db531673f484877cacdd651/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink_manager.h
[delete] https://crrev.com/f6500c14fa6592426db531673f484877cacdd651/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink_manager_unittest.cc
[modify] https://crrev.com/c9d0c8089e8216975a893b85f078f0fb4d044daf/content/browser/renderer_host/offscreen_canvas_provider_impl.cc
[modify] https://crrev.com/c9d0c8089e8216975a893b85f078f0fb4d044daf/content/browser/renderer_host/offscreen_canvas_provider_impl.h
[add] https://crrev.com/c9d0c8089e8216975a893b85f078f0fb4d044daf/content/browser/renderer_host/offscreen_canvas_provider_impl_unittest.cc
[modify] https://crrev.com/c9d0c8089e8216975a893b85f078f0fb4d044daf/content/browser/renderer_host/offscreen_canvas_surface_impl.cc
[modify] https://crrev.com/c9d0c8089e8216975a893b85f078f0fb4d044daf/content/browser/renderer_host/offscreen_canvas_surface_impl.h
[modify] https://crrev.com/c9d0c8089e8216975a893b85f078f0fb4d044daf/content/test/BUILD.gn

Status: Fixed
Blocking: -601863
Sign in to add a comment