Aura: Support --enable-partial-swap with --composite-to-mailbox

Aura: Support --enable-partial-swap with --composite-to-mailbox

Plumbs through a damage rect and reuses existing browser code for
two frame damage tracking.

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

[jonathan.backer, 2013-06-10 14:22:58]

sievers: Could you PTAL? I'll bug piman for OWNERS once you sign off.

I verified that without --enable-partial-swap, we're calculating and sending
full frame damage to the browser (i.e. this CL just works). Plus,
CopySubBufferDamage early exits in this case, so there is no perf hit.

[jonathan.backer, 2013-06-10 14:40:29]

Looks like https://codereview.chromium.org/16304003/ might throw a wrench in the
renderer side :D

[jonathan.backer, 2013-06-10 14:50:17]

On 2013/06/10 14:40:29, jonathan.backer wrote:
> Looks like https://codereview.chromium.org/16304003/ might throw a wrench in
the
> renderer side :D

I took a look at https://codereview.chromium.org/16304003/ and it will
trivialize the client side changes (just need to remove a DCHECK).

The approach to sending the damage rect to the browser is identical (augmenting
the same struct with a damage rect). So these CLs are compatible.

[no sievers, 2013-06-10 18:28:59]

Yes, looks like that patch makes it a bit easier. RWHVAura looks fine otherwise.

https://codereview.chromium.org/16730003/diff/2001/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/16730003/diff/2001/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_aura.cc:1637: void
RenderWidgetHostViewAura::BuffersSwapped(
Should we get rid of this then and make the texture image transport path go
through the other function too?

https://codereview.chromium.org/16730003/diff/2001/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_aura.cc:1667: const
gfx::Size& size,
nit: 'size' -> 'surface_size'

[jonathan.backer, 2013-06-10 19:11:12]

+piman 

Not sure what to do now. Does anyone know the ETA on aelias@s patch
(https://codereview.chromium.org/16304003/)?

I don't want to cause unnecessary churn. However, we're close to enabling
--composite-to-mailbox on CrOS and more bake time before M29 branch on CrOS is
worth it (i.e. with just 14 days for branch I don't want to wait more than a
couple).

https://codereview.chromium.org/16730003/diff/2001/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/16730003/diff/2001/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_aura.cc:1637: void
RenderWidgetHostViewAura::BuffersSwapped(
On 2013/06/10 18:28:59, Daniel Sievers wrote:
> Should we get rid of this then and make the texture image transport path go
> through the other function too?

Great suggestion. Done.

There's very little overhead on full frame update (a little CPU for SkRegion) by
unifying codepaths so its worth it.

https://codereview.chromium.org/16730003/diff/2001/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_aura.cc:1667: const
gfx::Size& size,
On 2013/06/10 18:28:59, Daniel Sievers wrote:
> nit: 'size' -> 'surface_size'

Done.

[piman, 2013-06-10 19:56:53]

LGTM, thanks!

[aelias_OOO_until_Jul13, 2013-06-10 22:20:12]

https://codereview.chromium.org/16730003/diff/23001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/16730003/diff/23001/cc/output/gl_renderer.cc#...
cc/output/gl_renderer.cc:1964: output_surface_->PostSubBuffer(GetFrameDamage(),
latency_info);
To be clearer about what I was asking for earlier, could you do here:

gfx::Rect frame_damage = GetFrameDamage();
if (frame_damage.size() == SurfaceSize())
  output_surface_->SwapBuffers(latency_info);
else
  output_surface_->PostSubBuffer(GetFrameDamage(), latency_info);

That would match the behavior after my patch.  The reason is that I don't have
access to the "using_partial_swap" setting from the output surface, so I would
just choose based on the rect size.

[aelias_OOO_until_Jul13, 2013-06-10 23:20:19]

https://codereview.chromium.org/16730003/diff/23001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/16730003/diff/23001/cc/output/gl_renderer.cc#...
cc/output/gl_renderer.cc:1964: output_surface_->PostSubBuffer(GetFrameDamage(),
latency_info);
On second thought, since this doesn't truly need to be a big blocker, I changed
my other patch to pass another boolean in GLFrameData to preserve the existing
behavior.  We can clean it up later once Mesa is confirmed to work with a
postSubBuffer/prepareTexture mix.

[commit-bot: I haz the power, 2013-06-11 18:44:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/backer@chromium.org/16730003/31001

[commit-bot: I haz the power, 2013-06-11 18:53:32]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[kenrb, 2013-06-11 19:27:35]

lgtm from security for IPC change

[commit-bot: I haz the power, 2013-06-11 19:29:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/backer@chromium.org/16730003/31001

[commit-bot: I haz the power, 2013-06-12 00:31:00]

Retried try job too often on win_rel for step(s) chrome_frame_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-06-12 11:27:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/backer@chromium.org/16730003/31001

[commit-bot: I haz the power, 2013-06-12 17:47:57]

Change committed as 205861