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

Issue 672071 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[meta] Mus CompositorFrameSink needs to support copy requests

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

Issue description

copy requests are texture readback requests. We need an API to request readback that is IPC friendly. This is a metabug to capture that work.
 
Owner: samans@chromium.org
Status: Assigned (was: Untriaged)
There's a lot of work here including cast functionality but the basics should be relatively simple. Assigning to samans@
Blocking:
Components: Internals>Compositing
Project Member

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

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

commit bf8eacc94d36e3718e5b74d5915f50422932e04d
Author: samans <samans@chromium.org>
Date: Fri Jan 27 22:59:25 2017

Implemented StructTraits for cc::TextureMailbox

cc::CopyOutputRequest, which we soon need to send over mojo, has a
cc::TextureMailbox. This CL creates a mojo struct for cc::TextureMailbox
to be used in the mojo struct of cc::CopyOutputRequest in a future CL.
We don't send shared_bitmap_ over IPC, and it's not a priority because
cc::CopyOutputRequest doesn't need it.

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

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

[modify] https://crrev.com/bf8eacc94d36e3718e5b74d5915f50422932e04d/cc/BUILD.gn
[modify] https://crrev.com/bf8eacc94d36e3718e5b74d5915f50422932e04d/cc/ipc/BUILD.gn
[modify] https://crrev.com/bf8eacc94d36e3718e5b74d5915f50422932e04d/cc/ipc/struct_traits_unittest.cc
[add] https://crrev.com/bf8eacc94d36e3718e5b74d5915f50422932e04d/cc/ipc/texture_mailbox.mojom
[add] https://crrev.com/bf8eacc94d36e3718e5b74d5915f50422932e04d/cc/ipc/texture_mailbox.typemap
[add] https://crrev.com/bf8eacc94d36e3718e5b74d5915f50422932e04d/cc/ipc/texture_mailbox_struct_traits.h
[modify] https://crrev.com/bf8eacc94d36e3718e5b74d5915f50422932e04d/cc/ipc/traits_test_service.mojom
[modify] https://crrev.com/bf8eacc94d36e3718e5b74d5915f50422932e04d/cc/ipc/typemaps.gni
[add] https://crrev.com/bf8eacc94d36e3718e5b74d5915f50422932e04d/cc/resources/DEPS
[modify] https://crrev.com/bf8eacc94d36e3718e5b74d5915f50422932e04d/cc/resources/texture_mailbox.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 31 2017

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

commit 5846cb2c8ad77620776878f93a2b25be3b26449f
Author: samans <samans@chromium.org>
Date: Tue Jan 31 03:09:47 2017

Replace source pointer in cc::CopyOutputRequest with a base::UnguessableToken

Soon we will be sending cc::CopyOutputRequests over IPC, but a copy
request can contain a source pointer which basically indicates who is
the creator of the request (no dereferencing occurs), and sending this
raw pointer over IPC is not a good idea. This CL replaces the raw
pointer with a base::UnguessableToken which acts a source ID.

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

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

[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/cc/layers/layer.cc
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/cc/layers/layer_unittest.cc
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/cc/output/copy_output_request.cc
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/cc/output/copy_output_request.h
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/cc/surfaces/surface.cc
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/cc/surfaces/surface_factory_unittest.cc
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/content/browser/BUILD.gn
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/content/browser/media/capture/web_contents_video_capture_device.cc
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/content/browser/media/capture/web_contents_video_capture_device_unittest.cc
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/content/browser/renderer_host/render_widget_host_view_browsertest.cc
[rename] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/content/browser/renderer_host/render_widget_host_view_frame_subscriber.h
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/5846cb2c8ad77620776878f93a2b25be3b26449f/content/public/browser/BUILD.gn

Comment 6 by sadrul@chromium.org, Jan 31 2017

Cc: moshayedi@chromium.org
Project Member

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

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

commit 345632b5cbddaa65124eb98bce6fa933fa211c35
Author: samans <samans@chromium.org>
Date: Tue Jan 31 18:34:08 2017

Implemented StructTraits for cc::CopyOutputRequest

MojoCompositorFrameSink needs to support copy requests, which means we need
mojo definition and struct traits for cc::CopyOutputRequests in order to send it
over mojo.

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

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

[modify] https://crrev.com/345632b5cbddaa65124eb98bce6fa933fa211c35/cc/ipc/BUILD.gn
[add] https://crrev.com/345632b5cbddaa65124eb98bce6fa933fa211c35/cc/ipc/copy_output_request.mojom
[add] https://crrev.com/345632b5cbddaa65124eb98bce6fa933fa211c35/cc/ipc/copy_output_request.typemap
[add] https://crrev.com/345632b5cbddaa65124eb98bce6fa933fa211c35/cc/ipc/copy_output_request_struct_traits.h
[modify] https://crrev.com/345632b5cbddaa65124eb98bce6fa933fa211c35/cc/ipc/struct_traits_unittest.cc
[modify] https://crrev.com/345632b5cbddaa65124eb98bce6fa933fa211c35/cc/ipc/traits_test_service.mojom
[modify] https://crrev.com/345632b5cbddaa65124eb98bce6fa933fa211c35/cc/ipc/typemaps.gni
[add] https://crrev.com/345632b5cbddaa65124eb98bce6fa933fa211c35/cc/output/DEPS
[modify] https://crrev.com/345632b5cbddaa65124eb98bce6fa933fa211c35/cc/output/copy_output_request.cc
[modify] https://crrev.com/345632b5cbddaa65124eb98bce6fa933fa211c35/cc/output/copy_output_request.h

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 1 2017

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

commit 455475e2b6c5e7dffb2cc0ad8f86e37819deede7
Author: samans <samans@chromium.org>
Date: Wed Feb 01 20:12:30 2017

Implemented StructTraits for cc::CopyOutputResult

MojoCompositorFrameSink needs to support copy requests, which means we need
mojo definition and struct traits for cc::CopyOutputResult in order to send it
over mojo.

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

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

[modify] https://crrev.com/455475e2b6c5e7dffb2cc0ad8f86e37819deede7/cc/ipc/BUILD.gn
[add] https://crrev.com/455475e2b6c5e7dffb2cc0ad8f86e37819deede7/cc/ipc/copy_output_result.mojom
[add] https://crrev.com/455475e2b6c5e7dffb2cc0ad8f86e37819deede7/cc/ipc/copy_output_result.typemap
[add] https://crrev.com/455475e2b6c5e7dffb2cc0ad8f86e37819deede7/cc/ipc/copy_output_result_struct_traits.cc
[add] https://crrev.com/455475e2b6c5e7dffb2cc0ad8f86e37819deede7/cc/ipc/copy_output_result_struct_traits.h
[modify] https://crrev.com/455475e2b6c5e7dffb2cc0ad8f86e37819deede7/cc/ipc/struct_traits_unittest.cc
[modify] https://crrev.com/455475e2b6c5e7dffb2cc0ad8f86e37819deede7/cc/ipc/texture_mailbox_struct_traits.h
[modify] https://crrev.com/455475e2b6c5e7dffb2cc0ad8f86e37819deede7/cc/ipc/traits_test_service.mojom
[modify] https://crrev.com/455475e2b6c5e7dffb2cc0ad8f86e37819deede7/cc/ipc/typemaps.gni
[modify] https://crrev.com/455475e2b6c5e7dffb2cc0ad8f86e37819deede7/cc/output/copy_output_result.cc
[modify] https://crrev.com/455475e2b6c5e7dffb2cc0ad8f86e37819deede7/cc/output/copy_output_result.h
[modify] https://crrev.com/455475e2b6c5e7dffb2cc0ad8f86e37819deede7/gpu/ipc/common/mailbox_holder_struct_traits.h

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 5 2017

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

commit e0c6f958254ff01ada02bfc4c9ba99a602c67930
Author: samans <samans@chromium.org>
Date: Sun Feb 05 05:32:46 2017

Mojom structs for copy requests / results should map to unique_ptr

Currently, cc.mojom.CopyOutputRequest and cc.mojom.CopyOutputResult map
to cc::CopyOutputRequest and cc::CopyOutputResult respectively, but
they should map to std::unique_ptr<cc::CopyOutputRequest> and
std::unique_ptr<cc::CopyOutputResult>. This is because
cc::CopyOutputRequest and cc::CopyOutputResult can be only created
exclusively as unique_ptr and we are going to have problem passing
their objects over mojo.

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

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

[modify] https://crrev.com/e0c6f958254ff01ada02bfc4c9ba99a602c67930/cc/ipc/BUILD.gn
[modify] https://crrev.com/e0c6f958254ff01ada02bfc4c9ba99a602c67930/cc/ipc/copy_output_request.typemap
[add] https://crrev.com/e0c6f958254ff01ada02bfc4c9ba99a602c67930/cc/ipc/copy_output_request_struct_traits.cc
[modify] https://crrev.com/e0c6f958254ff01ada02bfc4c9ba99a602c67930/cc/ipc/copy_output_request_struct_traits.h
[modify] https://crrev.com/e0c6f958254ff01ada02bfc4c9ba99a602c67930/cc/ipc/copy_output_result.typemap
[modify] https://crrev.com/e0c6f958254ff01ada02bfc4c9ba99a602c67930/cc/ipc/copy_output_result_struct_traits.cc
[modify] https://crrev.com/e0c6f958254ff01ada02bfc4c9ba99a602c67930/cc/ipc/copy_output_result_struct_traits.h
[modify] https://crrev.com/e0c6f958254ff01ada02bfc4c9ba99a602c67930/cc/ipc/struct_traits_unittest.cc
[modify] https://crrev.com/e0c6f958254ff01ada02bfc4c9ba99a602c67930/cc/output/copy_output_request.h
[modify] https://crrev.com/e0c6f958254ff01ada02bfc4c9ba99a602c67930/cc/output/copy_output_result.cc
[modify] https://crrev.com/e0c6f958254ff01ada02bfc4c9ba99a602c67930/cc/output/copy_output_result.h

Project Member

Comment 10 by bugdroid1@chromium.org, Feb 15 2017

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

commit 05612a0cb0feafb73ec78c2aebef4c1af797380f
Author: samans <samans@chromium.org>
Date: Wed Feb 15 05:28:28 2017

CopyOutputResult must have a working release callback when received over mojo

Right now, CopyOutputResult when received over mojo has an empty release
callback. This CL introduces TextureMailboxReleaser, which is a mojo
interface that has a Release method with the same signature as
ReleaseCallback. On the sender side, the ReleaseCallback of the
CopyOutputResult is retained in a TextureMailboxReleaserImpl and
a TextureMailboxReleaserPtr will be sent to the receiver. When
the receiver calls the Release method, the sender calls the release
callback.

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

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

[modify] https://crrev.com/05612a0cb0feafb73ec78c2aebef4c1af797380f/cc/ipc/BUILD.gn
[modify] https://crrev.com/05612a0cb0feafb73ec78c2aebef4c1af797380f/cc/ipc/copy_output_result.mojom
[modify] https://crrev.com/05612a0cb0feafb73ec78c2aebef4c1af797380f/cc/ipc/copy_output_result_struct_traits.cc
[modify] https://crrev.com/05612a0cb0feafb73ec78c2aebef4c1af797380f/cc/ipc/copy_output_result_struct_traits.h
[modify] https://crrev.com/05612a0cb0feafb73ec78c2aebef4c1af797380f/cc/ipc/struct_traits_unittest.cc
[add] https://crrev.com/05612a0cb0feafb73ec78c2aebef4c1af797380f/cc/ipc/texture_mailbox_releaser.mojom

Project Member

Comment 12 by bugdroid1@chromium.org, Feb 17 2017

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

commit b4abfddd91ae9f4088f1544ec7ca8e95afbe7229
Author: samans <samans@chromium.org>
Date: Fri Feb 17 06:09:00 2017

MojoCompositorFrameSinkPrivate should support copy requests

Copy requests must be supported over mojo because some clients of
SurfaceFactory use them and SurfaceFactory will move to a separate
process. This CL adds methods for sending copy requests to
MojoCompositorFrameSinkPrivate.

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

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

[modify] https://crrev.com/b4abfddd91ae9f4088f1544ec7ca8e95afbe7229/cc/ipc/mojo_compositor_frame_sink.mojom
[modify] https://crrev.com/b4abfddd91ae9f4088f1544ec7ca8e95afbe7229/cc/surfaces/compositor_frame_sink_support.cc
[modify] https://crrev.com/b4abfddd91ae9f4088f1544ec7ca8e95afbe7229/cc/surfaces/compositor_frame_sink_support.h
[modify] https://crrev.com/b4abfddd91ae9f4088f1544ec7ca8e95afbe7229/components/display_compositor/gpu_compositor_frame_sink.cc
[modify] https://crrev.com/b4abfddd91ae9f4088f1544ec7ca8e95afbe7229/components/display_compositor/gpu_compositor_frame_sink.h

Status: Fixed (was: Assigned)
I'd say this is done. I'm marking as FIXED. We can revisit if we need to tweak the API or add more features.
Blocking: -601863

Sign in to add a comment