aura: fast resizes for accelerated pages

aura: fast resizes for accelerated pages

This CL adds a compositor lock that we grab while waiting for the renderer's
frame. The lock expires with a short delay.
It makes a lot of pages resize interactively (e.g. NYT), the slow pages being not significantly worse.

BUG=58782
TEST=load www.nytimes.com, resize, see it's interactive. load gmail, resize, see it's not but it's still working


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=128585

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

[piman, 2012-03-23 05:43:40]

backer: please review
ben: OWNERS, but please give a careful eye.

I know the approach of a compositor lock may be controversial, let me know if
you can think of a better way. The results are worth working on it.

[jonathan.backer, 2012-03-23 15:47:57]

LGTM with nits.

https://chromiumcodereview.appspot.com/9838045/diff/1/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://chromiumcodereview.appspot.com/9838045/diff/1/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:480: if
(!resize_locks_.empty() && !compositor->DrawPending()) {
I'm not sure that !compositor->DrawPending() is strong enough synchronization.
But that was never the intent of the delayed ACK: it's throttling so that the
renderer does not generate more frames than the UI can display --- so it makes
perfect sense to fast track here.

https://chromiumcodereview.appspot.com/9838045/diff/1/ui/aura/root_window.cc
File ui/aura/root_window.cc (right):

https://chromiumcodereview.appspot.com/9838045/diff/1/ui/aura/root_window.cc#...
ui/aura/root_window.cc:75: root_window_->UnlockCompositor();
I think that you want to "root_window_ = NULL" here.

https://chromiumcodereview.appspot.com/9838045/diff/1/ui/aura/root_window.cc#...
ui/aura/root_window.cc:194: scheduled_while_locked_ = true;
nit: is it conceivable that the draw task had been posted prior to the lock?

how about draw_on_compositor_unlock_ ?

[Ben Goodger (Google), 2012-03-23 16:07:03]

lgtm

[piman, 2012-03-23 17:41:08]

https://chromiumcodereview.appspot.com/9838045/diff/1/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://chromiumcodereview.appspot.com/9838045/diff/1/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:480: if
(!resize_locks_.empty() && !compositor->DrawPending()) {
On 2012/03/23 15:48:00, jonathan.backer wrote:
> I'm not sure that !compositor->DrawPending() is strong enough synchronization.

So, we don't pipeline draws on the client side (browser process) for the
compositor. That is, between OnCompositorEnded() and the next Draw() (i.e.
!DrawPending()), the GPU process doesn't have any commands in flight for the UI,
and it's absolutely safe to allow the swap to carry on - we switched the texture
in the UI, so if we were to draw another frame while the renderer is working on
its next one, we won't display a partial frame.
If DrawPending() is true (e.g. we grabbed the lock between Draw() and
OnCompositorEnded(), or the lock has expired), then it's not safe and we have to
wait until the OnCompositingEnded() - which we'll do below.

> But that was never the intent of the delayed ACK: it's throttling so that the
> renderer does not generate more frames than the UI can display --- so it makes
> perfect sense to fast track here.

We're losing throttling while we're resizing, but that's intentional because
we've blocked the UI to let the renderer give us a frame ASAP. It's true that in
theory a renderer could send other frames before the resize, but we have to let
them through to allow the resize to come.

https://chromiumcodereview.appspot.com/9838045/diff/1/ui/aura/root_window.cc
File ui/aura/root_window.cc (right):

https://chromiumcodereview.appspot.com/9838045/diff/1/ui/aura/root_window.cc#...
ui/aura/root_window.cc:75: root_window_->UnlockCompositor();
On 2012/03/23 15:48:00, jonathan.backer wrote:
> I think that you want to "root_window_ = NULL" here.

Thanks for paying attention! You're absolutely right, I had it originally and
then I lost it when moving stuff around.
Fixed.

https://chromiumcodereview.appspot.com/9838045/diff/1/ui/aura/root_window.cc#...
ui/aura/root_window.cc:194: scheduled_while_locked_ = true;
On 2012/03/23 15:48:00, jonathan.backer wrote:
> nit: is it conceivable that the draw task had been posted prior to the lock?

Yes (imagine a caller doing ScheduleDraw() then GetCompositorLock() right away),
which is what this code catches.

> 
> how about draw_on_compositor_unlock_ ?

Done.

[commit-bot: I haz the power, 2012-03-23 17:46:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/9838045/3004

[commit-bot: I haz the power, 2012-03-23 22:06:30]

Change committed as 128585