Fix GetSnapshotFromRenderer stalling when capturing while between navigating to certain pages.

Fix GetSnapshotFromRenderer stalling when capturing while between navigating to certain pages.

BUG=271056
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=216867

[justinlin, 2013-08-09 22:42:50]

Hi Nat, PTAL, thanks.

I don't presume to fully understand much of the code around here, but it does
end up failing this DCHECK and crashing the renderer for the
RenderWidget::GetSnapshotFromRenderer function. It ends up calling
"webwidget_->paint(canvas, rect)", which ends up in this code. Returning false
here does fix the issue we're experiencing.

[nduca, 2013-08-09 22:44:26]

send this to brianderson... :)

[justinlin, 2013-08-09 22:46:27]

Hi Brian, PTAL. See first comment for some more details. Thanks!

[brianderson, 2013-08-09 23:06:47]

This seems like a reasonable thing to do, assuming we can always fail
CompositeAndReadback while loading a page.

The synchronous CompositeAndReadback has to make a commit, which would result in
a deadlock with the current way deferred commits work if we attempted a
readback.

lgtm, given that this will not regress the current behavior that readbacks
aren't allowed during page load.

Note: We could support CompositeAndReadback during page load by unblocking only
the commits associated with a readback, but that will be more involved. Will
this be a requirement?

[jamesr, 2013-08-09 23:13:25]

If we're in this state the only sort of pixels we can possibly get out with the
main thread blocked are opaque white pixels.

[justinlin, 2013-08-09 23:32:56]

This fallback snapshotting method only ends up in here due to
"webwidget_->paint()", so I don't think there's much need to improve it for us 
besides have it not deadlock.

[commit-bot: I haz the power, 2013-08-09 23:38:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/justinlin@chromium.org/22763003/1

[nduca, 2013-08-09 23:39:06]

justin, better to wait for jamesr to lg you.

[jamesr, 2013-08-09 23:41:44]

I think this is fine - just observing that there's no better behavior here than
failing in some fashion or another.  Brian's review is sufficient to land, IMO.

[justinlin, 2013-08-09 23:43:37]

On 2013/08/09 23:13:25, jamesr wrote:
> If we're in this state the only sort of pixels we can possibly get out with
the
> main thread blocked are opaque white pixels.

Oops, missed this. I think failing the readback or getting a white frame or two
is acceptable given this situation. We should seldom be in this state anyway.

[commit-bot: I haz the power, 2013-08-09 23:51:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/justinlin@chromium.org/22763003/1

[commit-bot: I haz the power, 2013-08-10 15:20:18]

Change committed as 216867