Correct. That was the plan when I moved the snapshots to its own array. I guess the next step will be to either create a dedicated class for it or just do as you suggested.

+samans@

An alternative solution:
 . RenderWidgetHostImpl tells the renderer to force-redraw, as it currently does [1] using chrome-ipc (but without the LatencyInfo, and instead with a force-redraw-id).
 . RenderViewImpl forces a redraw, and requests a presentation-feedback for that frame here [2]
 . From the presentation-callback, there are two options:
    (a) RenderViewImpl tells the browser that the force-redraw completed using a new chrome-ipc (which includes the force-redraw-id), or
    (b) RenderFrameMetadata has a |last_force_redraw_frame| field, which gets updated by LayerTreeHostImpl, and sent to the browser.

I think this gives us the same functionality, but the process becomes simpler. Thoughts?
  

[1] https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_impl.cc?g=0&l=1748
[2] https://cs.chromium.org/chromium/src/content/renderer/render_view_impl.cc?l=1234

I believe this would allow for a bunch of clean-ups in the viz code, and the awkward call to RenderWidgetHostImpl::OnGpuSwapBuffersCompleted() from various places (e.g. [1]).

[1] https://cs.chromium.org/chromium/src/content/browser/compositor/offscreen_browser_compositor_output_surface.cc?type=cs&sq=package:chromium&g=0&l=196

+1. Not only is the presentation feedback cleaner, but we might also be able to get rid of the hacky 10 frame delay in RenderWidgetHostImpl::ProcessSnapshotResponses if the presentation feedback is accurate enough. (https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_impl.cc?rcl=126c0fdfb84205f015cb18f5f9caa64a622fba28&l=2078)

I prefer option (a). I actually don't fully understand how option (b) works. RenderFrameMetadata is generated on the next draw, so are you saying we force a draw just so we can send another RenderFrameMetadata? I would rather just send an IPC right away.

Yea, I also prefer (a). I mentioned (b) because it felt like something that could be made to work, but I haven't fully though through the implications of that. You raise a good point though that this would require a yet another unnecessary submission from the renderer, which is still hacky.

Will you pick up this bug sadrul@? I can also work on it this upcoming sprint.

I am working on the browser/renderer parts right now (i.e. the changes in content/), but perhaps you could pick up the cleanup from viz after that?

Sure.

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

commit 3821fdf5c54add24d02adef248a311c293ea0bd0
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Wed Jun 13 19:50:51 2018

presentation feedback: Use a GLFence fallback.

Use a GLFence fallback (when available) if GPUTimer is not supported
for reporting presentation time.

BUG= 851504 
TBR=dcastagna@ for trivial mechanical change in media/gpu/

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ib7cb1361974e8f4e0cfa019ebb2fc2f620ac353a
Reviewed-on: https://chromium-review.googlesource.com/1098528
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566958}
[modify] https://crrev.com/3821fdf5c54add24d02adef248a311c293ea0bd0/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/3821fdf5c54add24d02adef248a311c293ea0bd0/gpu/command_buffer/service/gles2_cmd_decoder_passthrough_doers.cc
[modify] https://crrev.com/3821fdf5c54add24d02adef248a311c293ea0bd0/gpu/command_buffer/service/query_manager.cc
[modify] https://crrev.com/3821fdf5c54add24d02adef248a311c293ea0bd0/gpu/ipc/service/image_transport_surface_overlay_mac.mm
[modify] https://crrev.com/3821fdf5c54add24d02adef248a311c293ea0bd0/media/gpu/windows/dxva_picture_buffer_win.cc
[modify] https://crrev.com/3821fdf5c54add24d02adef248a311c293ea0bd0/ui/gl/gl_fence.cc
[modify] https://crrev.com/3821fdf5c54add24d02adef248a311c293ea0bd0/ui/gl/gl_fence.h
[modify] https://crrev.com/3821fdf5c54add24d02adef248a311c293ea0bd0/ui/gl/gl_surface_presentation_helper.cc
[modify] https://crrev.com/3821fdf5c54add24d02adef248a311c293ea0bd0/ui/gl/gl_surface_presentation_helper.h

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

commit fd767ebe787dde4a2ac10b4f9fa30a02608cc421
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Thu Jun 14 16:27:48 2018

latency: Remove snapshots from LatencyInfo.

This changes how snapshot-requests via Page.captureScreenshot in
devtools are handled. Instead of notifying the browser directly from the
display-compositor, the renderer requests a presentation-time for the
new force-redraw frame. Upon receipt of the presentation time, renderer
notifies the browser, which can then capture the screenshot (either via
chrome, or directly from the underlying platform window).

This allows removing the snapshots from LatencyInfo, which was an odd
fit. This also enables the gpu and display-compositor code to be unaware
of snapshot-requests.

BUG= 851504 

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I16d8cb070ce883e95ac40ed5fd05ebaf3e96f0b1
Reviewed-on: https://chromium-review.googlesource.com/1095562
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567297}
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/components/viz/service/display/display.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/components/viz/service/display/output_surface.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/components/viz/service/display/output_surface.h
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/components/viz/service/display_embedder/gl_output_surface.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/content/browser/compositor/gpu_browser_compositor_output_surface.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/content/browser/compositor/in_process_display_client.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/content/browser/compositor/offscreen_browser_compositor_output_surface.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/content/browser/compositor/software_browser_compositor_output_surface.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/content/browser/devtools/protocol/page_handler.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/content/browser/renderer_host/input/mouse_latency_browsertest.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/content/browser/snapshot_browsertest.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/content/common/view_messages.h
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/content/renderer/render_view_impl.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/content/renderer/render_view_impl.h
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/ui/latency/ipc/latency_info_param_traits.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/ui/latency/ipc/latency_info_param_traits_unittest.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/ui/latency/latency_info.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/ui/latency/latency_info.h
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/ui/latency/latency_tracker.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/ui/latency/latency_tracker.h
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/ui/latency/mojo/latency_info.mojom
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/ui/latency/mojo/latency_info_struct_traits.cc
[modify] https://crrev.com/fd767ebe787dde4a2ac10b4f9fa30a02608cc421/ui/latency/mojo/latency_info_struct_traits.h

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

commit 89a1fbded1bff5b1032edeff33369d59866bd4b1
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Thu Jun 14 17:00:41 2018

gpu: Remove snapshot related code.

With the change to using presentation-feedback for facilitating snapshot
requests (crrev.com/c/1095562), the snapshot-specific code in gpu can
now be removed.

BUG= 851504 

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I25b563f879bb66c345ba541df736efc65a834f0d
Reviewed-on: https://chromium-review.googlesource.com/1096669
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567311}
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/components/viz/service/display_embedder/skia_output_surface_impl_on_gpu.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/components/viz/service/display_embedder/skia_output_surface_impl_on_gpu.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/components/viz/test/test_context_support.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/components/viz/test/test_context_support.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/command_buffer/client/client_test_helper.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/command_buffer/client/context_support.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/command_buffer/client/gles2_implementation.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/command_buffer/client/gles2_implementation.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/command_buffer/client/gpu_control.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/command_buffer/client/raster_implementation.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/command_buffer/client/raster_implementation.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/command_buffer/client/raster_implementation_gles_unittest.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/command_buffer/tests/decoder_perftest.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/command_buffer/tests/gl_manager.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/command_buffer/tests/gl_manager.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/gles2_conform_support/egl/context.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/gles2_conform_support/egl/context.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/client/command_buffer_proxy_impl.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/client/command_buffer_proxy_impl.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/client/gpu_channel_host.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/client/gpu_channel_host.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/common/flush_params.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/common/gpu_messages.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/common/gpu_param_traits_macros.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/in_process_command_buffer.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/service/command_buffer_stub.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/service/command_buffer_stub.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/service/direct_composition_surface_win.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/service/direct_composition_surface_win.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/service/direct_composition_surface_win_unittest.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/service/gles2_command_buffer_stub.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/service/gles2_command_buffer_stub.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/service/gpu_channel.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/service/image_transport_surface_delegate.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/service/image_transport_surface_overlay_mac.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/service/image_transport_surface_overlay_mac.mm
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/service/pass_through_image_transport_surface.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/gpu/ipc/service/pass_through_image_transport_surface.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/ppapi/proxy/ppapi_command_buffer_proxy.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/ppapi/proxy/ppapi_command_buffer_proxy.h
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/ui/gl/gl_surface.cc
[modify] https://crrev.com/89a1fbded1bff5b1032edeff33369d59866bd4b1/ui/gl/gl_surface.h

\o/

Thanks for cleaning this up!

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

commit 813d5f30d262f5143be495d70ed9a9e3c95b77e7
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Mon Jun 18 20:02:54 2018

cc: SwapPromise cleanup.

DidNotSwap() no longer needs to return any value. crrev.com/567297
removed the last instance of a persistent SwapPromise that would
return KEEP_ACTIVE. Now that it's gone, the associated code can be
removed.

BUG= 851504 

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I12d5af548800ac8674c1ab11f38c9aeb73338941
Reviewed-on: https://chromium-review.googlesource.com/1103944
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568118}
[modify] https://crrev.com/813d5f30d262f5143be495d70ed9a9e3c95b77e7/cc/trees/latency_info_swap_promise.cc
[modify] https://crrev.com/813d5f30d262f5143be495d70ed9a9e3c95b77e7/cc/trees/latency_info_swap_promise.h
[modify] https://crrev.com/813d5f30d262f5143be495d70ed9a9e3c95b77e7/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/813d5f30d262f5143be495d70ed9a9e3c95b77e7/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/813d5f30d262f5143be495d70ed9a9e3c95b77e7/cc/trees/layer_tree_impl_unittest.cc
[modify] https://crrev.com/813d5f30d262f5143be495d70ed9a9e3c95b77e7/cc/trees/swap_promise.h
[modify] https://crrev.com/813d5f30d262f5143be495d70ed9a9e3c95b77e7/cc/trees/swap_promise_manager_unittest.cc
[modify] https://crrev.com/813d5f30d262f5143be495d70ed9a9e3c95b77e7/content/renderer/gpu/queue_message_swap_promise.cc
[modify] https://crrev.com/813d5f30d262f5143be495d70ed9a9e3c95b77e7/content/renderer/gpu/queue_message_swap_promise.h
[modify] https://crrev.com/813d5f30d262f5143be495d70ed9a9e3c95b77e7/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/813d5f30d262f5143be495d70ed9a9e3c95b77e7/content/test/layouttest_support.cc