Use Surfaces to hold delegated renderer contents.

Use Surfaces to hold delegated renderer contents.

With --use-surfaces, the frame data from a renderer is now put into a surface, and a SurfaceLayer is put in the browser compositor's layer tree. We still rely on the browser compositor redrawing to schedule a redraw of the entire surface tree.

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

[piman, 2014-07-25 01:27:43]

LGTM, but 2 things:
1- there's really a lot of duplication between the 2 subclasses. What is the
plan to reduce that?
2- when ccameron introduced DelegatedFrameHost, it was clear that the
abstractions between it and the RWHV* were quite convoluted / not very good, and
that we would need to fix those. I worry that now the bad abstractions are now
into the 2 subclasses, making it even harder to fix (we have 2 Delegated*Host
times 2 RWHV, or 4 cross-interactions.). What is the plan to address that?

https://codereview.chromium.org/417943004/diff/80001/content/browser/composit...
File content/browser/compositor/delegated_frame_surface_host.cc (right):

https://codereview.chromium.org/417943004/diff/80001/content/browser/composit...
content/browser/compositor/delegated_frame_surface_host.cc:399: if
(!surface_returned_resources_.empty()) {
This if can go away, the only caller is ReturnResources which makes sure at
least one resource is present before calling this. It's also consistent with the
DCHECK on l.402.

[ccameron, 2014-07-25 17:18:46]

Sorry, this really doesn't look like the right thing to do to me --
DelegatedFrameSurfaceHost and DelegatedFrameLayerHost are near-exact copy-pasted
versions of each other -- we don't want that kind of mass-duplication.

Also, we lose track of which interfaces which functions are for by running
everything through a base class.

Why can't DelegatedFrameHost, as it exists, just have "is_using_surfaces_" flag
somewhere, and just add a few branches inside it -- that would seem so much
better to me.

[jbauman, 2014-07-25 22:22:13]

PTAL. I moved the two implementations into one class for now, but I think
they'll diverge quite a bit in the future as we change how frames are scheduled.

[ccameron, 2014-07-25 22:28:05]

Thanks, this lgtm. Yeah, if it diverges terribly then it'll make sense to cut at
some point, but I think the places to cut will reveal themselves more clearly
using this approach.

https://codereview.chromium.org/417943004/diff/100001/content/browser/composi...
File content/browser/compositor/delegated_frame_host.h (right):

https://codereview.chromium.org/417943004/diff/100001/content/browser/composi...
content/browser/compositor/delegated_frame_host.h:259: 
Might wanna throw a comment saying "State for rendering into a surface" above
this block.

[jamesr, 2014-07-25 22:33:25]

lgtm

[jbauman, 2014-07-25 22:34:49]

The CQ bit was checked by jbauman@chromium.org

[commit-bot: I haz the power, 2014-07-25 22:36:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jbauman@chromium.org/417943004/120001

[piman, 2014-07-25 22:58:24]

lgtm

[commit-bot: I haz the power, 2014-07-26 05:02:57]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_rel_swarming on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel_sw...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win8_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win8_chromium_rel/bui...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)
  mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)
  mac_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)

[commit-bot: I haz the power, 2014-07-26 19:35:34]

Change committed as 285786