I was just playing around in the code. It turns out the implementation in RenderWidgetHostImpl has a number of problems:

1. WindowSnapshotReachedScreen() calls GetView()->CopyFromSurface() without first checking IsSurfaceAvailableForCopy().

2. WindowSnapshotReachedScreen() has a second code path that calls ui::GrabViewSnapshot() which, internally, makes a Layer-based CopyOutputRequest.

3. Both #1 and #2 will add CopyOutputRequests that will not be executed until the NEXT compositor frame is processed. It seems the code assumed they are executing on the current compositor frame from the renderer.

4. RenderWidgetHostImpl::OnSnapshotFromSurfaceReceived() includes retry logic for fail results. However: a) again, IsSurfaceAvailableForCopy() is not being checked, and b) there is no delay before the retry is made (the fail may have occurred with a synchronous call to the result callback, so the stack is just piling up without ever returning to the event loop).

IMHO, the owners of this code/feature should fix these problems before we even consider making changes to enable this in VIZ.

miu@ as you have insight into what is happening in this area, could you help triage this issue to an appropriate owner?

Original authors were mfomitchev@ and jbauman@, who don't work on Chrome anymore. :(

Based on recent discussions in other bugs, I'm hoping ccameron@ will know who owns this now. It seems there was some need for a duplicate snapshot API that was synchronized with the latency info (for testing reasons only?). FWIW, according to comment 1, I don't think the code is achieving what the original author's intent was.

Perhaps it's time to delete this code path and use the RWHV::CopyFromSurface() API instead?

FYI: I just saw this in an e-mail from kylechar@: "It's not expected that LatencyInfo signal will make it's way back to the browser process anymore since the display compositor isn't there."

I think that the goal of GetSnapshotFromBrowser is to take a snapshot using system APIs -- that is, to use something independent of compositing APIs to see what exactly ended up on-screen.

So ideally this isn't using CopyFromSurface.

(oh, and this is for testing -- performance is optional)

I think the presentation token / callback API would be ideal for GetSnapshotFromBrowser()? It provides a callback either (1) after the content has been presented to screen or (2) after the GPU swap has happened for the content if we can't get the actual presentation time.

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

commit b8aedbdc195d908871669fe9446ae20631a30227
Author: kylechar <kylechar@chromium.org>
Date: Thu Apr 26 20:53:58 2018

viz: Enable screenshot viz_content_browsertests.

Enable some disabled tests in viz_content_browsertests that relied on
DevTools Page.captureScreenshot. That call now works so the tests can be
re-enabled.

Bug:  785308 ,  810037 
Change-Id: Ia249682d8395ac18873b4c2b326e63ffc567d68f
Reviewed-on: https://chromium-review.googlesource.com/1030974
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554147}
[modify] https://crrev.com/b8aedbdc195d908871669fe9446ae20631a30227/testing/buildbot/filters/viz.content_browsertests.filter

saman fixed the root issue in  https://crbug.com/785308  and I've reenabled SnapshotBrowserTest.* for viz_content_browser_tests.