[cros] Change how login-prompt-visible signal is emitted when accelerated compositing is enabled.

[cros] Change how login-prompt-visible signal is emitted when accelerated compositing is enabled.

We could not rely on NOTIFICATION_RENDER_WIDGET_HOST_DID_PAINT as it is not sent after page is loaded (even though we're updating content through JS) in case of accelerated compositing is enabled.

BUG=127517
TEST=Add --allow-webui-compositing flag, check that /tmp/uptime-login-prompt-visible is still generated.

[Nikita (slow), 2012-05-28 12:30:44]

Daniel, please take a look.

Reason for this change (also described in comments):

When accelerated compositing is disabled at login WebUI (current case) we're
relying at this sequence of events:

OnTabMainFrameRender(x3) > OnTabMainFrameLoaded() > LOGIN_USER_IMAGES_LOADED >
OnTabMainFrameRender()
so first paint event after user images are loaded is considered to be a moment
when login screen is visible.

When accelerated compositing is enabled at login WebUI:

OnTabMainFrameRender(x3) > OnTabMainFrameLoaded() > LOGIN_USER_IMAGES_LOADED
so in this case no more paint events are generated so this notification seems
like a closest approximation of the timing we need. I know it is not perfect but
not sure how improve that. What event should I observe instead?

[Daniel Erat, 2012-05-28 15:20:53]

I'm surprised that OnTabMainFrameRender() doesn't get called again after the
page's JS tells us that the images have been loaded.  Can you check with some
people working on the compositor to see if they have any ideas?

http://codereview.chromium.org/10443053/diff/1001/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/webui_login_view.cc (right):

http://codereview.chromium.org/10443053/diff/1001/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/webui_login_view.cc:356: return
command_line.HasSwitch(switches::kAllowWebUICompositing);
is there any risk that this might not get updated when accelerated compositing
is turned on by default?  if so, any way that this can be made safer (e.g. move
this utility function somewhere else so there's a single path used by all the
code that needs to check the flag)?  if the flag will be removed in the same
changelist that turns it on, it's probably okay.

[Nikita (slow), 2012-05-28 15:55:04]

http://codereview.chromium.org/10443053/diff/1001/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/webui_login_view.cc (right):

http://codereview.chromium.org/10443053/diff/1001/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/webui_login_view.cc:356: return
command_line.HasSwitch(switches::kAllowWebUICompositing);
On 2012/05/28 15:20:53, Daniel Erat wrote:
> is there any risk that this might not get updated when accelerated compositing
> is turned on by default?  if so, any way that this can be made safer (e.g.
move
> this utility function somewhere else so there's a single path used by all the
> code that needs to check the flag)?  if the flag will be removed in the same
> changelist that turns it on, it's probably okay.

I've actually thought that when accelerated compositing will  be enabled for all
WebUI, this flag meaning would get flipped so it will require update of this
code as well.

[Nikita (slow), 2012-05-28 16:02:18]

On 2012/05/28 15:20:53, Daniel Erat wrote:
> I'm surprised that OnTabMainFrameRender() doesn't get called again after the
> page's JS tells us that the images have been loaded.  Can you check with some
> people working on the compositor to see if they have any ideas?
> 

From backer@:
there is no backing store to update with accelerated compositing.

Any other event I could use?

[jonathan.backer, 2012-05-28 16:50:43]

On 2012/05/28 16:02:18, Nikita Kostylev wrote:
> On 2012/05/28 15:20:53, Daniel Erat wrote:
> > I'm surprised that OnTabMainFrameRender() doesn't get called again after the
> > page's JS tells us that the images have been loaded.  Can you check with
some
> > people working on the compositor to see if they have any ideas?
> > 
> 
> From backer@:
> there is no backing store to update with accelerated compositing.
> 
> Any other event I could use?

Update:

With accelerated compositing, RenderWidget::OnSwapBuffersComplete() will send a
ViewHostMsg_UpdateRect (under certain circumstances I don't yet understand),
which will filter down into a RWHVImpl::DidUpdateBackingStore() (even though no
backstore is updated).

Still investigating.

[jonathan.backer, 2012-05-28 18:09:29]

On 2012/05/28 16:50:43, jonathan.backer wrote:
> On 2012/05/28 16:02:18, Nikita Kostylev wrote:
> > On 2012/05/28 15:20:53, Daniel Erat wrote:
> > > I'm surprised that OnTabMainFrameRender() doesn't get called again after
the
> > > page's JS tells us that the images have been loaded.  Can you check with
> some
> > > people working on the compositor to see if they have any ideas?
> > > 
> > 
> > From backer@:
> > there is no backing store to update with accelerated compositing.
> > 
> > Any other event I could use?
> 
> Update:
> 
> With accelerated compositing, RenderWidget::OnSwapBuffersComplete() will send
a
> ViewHostMsg_UpdateRect (under certain circumstances I don't yet understand),
> which will filter down into a RWHVImpl::DidUpdateBackingStore() (even though
no
> backstore is updated).
> 
> Still investigating.
Update:

When we run without --enable-threaded-compositing, we get a
RWHVImpl::DidUpdateBackingStore() per frame on http://google.com. With that flag
(the default), we do not. AFAIK, there should be no difference from the
browser's perspective whether we use this flag or not. Investigating further, to
see if this is a bug.

[jonathan.backer, 2012-05-28 18:39:46]

On 2012/05/28 18:09:29, jonathan.backer wrote:
> On 2012/05/28 16:50:43, jonathan.backer wrote:
> > On 2012/05/28 16:02:18, Nikita Kostylev wrote:
> > > On 2012/05/28 15:20:53, Daniel Erat wrote:
> > > > I'm surprised that OnTabMainFrameRender() doesn't get called again after
> the
> > > > page's JS tells us that the images have been loaded.  Can you check with
> > some
> > > > people working on the compositor to see if they have any ideas?
> > > > 
> > > 
> > > From backer@:
> > > there is no backing store to update with accelerated compositing.
> > > 
> > > Any other event I could use?
> > 
> > Update:
> > 
> > With accelerated compositing, RenderWidget::OnSwapBuffersComplete() will
send
> a
> > ViewHostMsg_UpdateRect (under certain circumstances I don't yet understand),
> > which will filter down into a RWHVImpl::DidUpdateBackingStore() (even though
> no
> > backstore is updated).
> > 
> > Still investigating.
> Update:
> 
> When we run without --enable-threaded-compositing, we get a
> RWHVImpl::DidUpdateBackingStore() per frame on http://google.com. With that
flag
> (the default), we do not. AFAIK, there should be no difference from the
> browser's perspective whether we use this flag or not. Investigating further,
to
> see if this is a bug.

This behaviour is expected (not a bug). It's a result of 
http://codereview.chromium.org/8528006/ and
http://codereview.chromium.org/8912002/

Given the latter CL, I don't know why we don't always send a
ViewHostMsg_UpdateRect from RW::didCompleteSwapBuffers. I'd poke piman@, nduca@,
or enne@ (in roughly that order).

[Nikita (slow), 2012-06-01 19:04:49]

I'm abandoning this CL, there's a proper one
http://codereview.chromium.org/10476002/