Fix the white flicker of the keyboard overlay when it first appearing.

Fix the white flicker of the keyboard overlay when first appearing.

BUG=129582
TEST=Manual

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

[mazda, 2012-05-25 21:55:12]

Oshima-san,

Could you review this CL since you made a similar fix in GTK era?

Thanks,

[oshima, 2012-05-25 22:33:20]

LGTM

http://codereview.chromium.org/10442044/diff/2001/chrome/browser/ui/views/key...
File chrome/browser/ui/views/keyboard_overlay_delegate.cc (right):

http://codereview.chromium.org/10442044/diff/2001/chrome/browser/ui/views/key...
chrome/browser/ui/views/keyboard_overlay_delegate.cc:55: gfx::Rect rect =
gfx::Screen::GetMonitorNearestWindow(
const gfx::Rect&

http://codereview.chromium.org/10442044/diff/2001/chrome/browser/ui/views/key...
chrome/browser/ui/views/keyboard_overlay_delegate.cc:143: case
content::NOTIFICATION_RENDER_WIDGET_HOST_DID_PAINT:
I wonder if we can rely on this notification when we switch webui to use
accelerated composition. You probably should contact backer about this.

[mazda, 2012-05-25 23:30:37]

Jonathan,
I have a question about content::NOTIFICATION_RENDER_WIDGET_HOST_DID_PAINT.
Is it OK to rely on this notification when we switch webui to use accelerated
composition?
What I'd like to do is holding off the display of Widget until the web contents
is rendered using this notification.

http://codereview.chromium.org/10442044/diff/2001/chrome/browser/ui/views/key...
File chrome/browser/ui/views/keyboard_overlay_delegate.cc (right):

http://codereview.chromium.org/10442044/diff/2001/chrome/browser/ui/views/key...
chrome/browser/ui/views/keyboard_overlay_delegate.cc:55: gfx::Rect rect =
gfx::Screen::GetMonitorNearestWindow(
On 2012/05/25 22:33:20, oshima wrote:
> const gfx::Rect&

Done.

http://codereview.chromium.org/10442044/diff/2001/chrome/browser/ui/views/key...
chrome/browser/ui/views/keyboard_overlay_delegate.cc:143: case
content::NOTIFICATION_RENDER_WIDGET_HOST_DID_PAINT:
On 2012/05/25 22:33:20, oshima wrote:
> I wonder if we can rely on this notification when we switch webui to use
> accelerated composition. You probably should contact backer about this.

Thanks. I'll ask backer.

[mazda, 2012-05-30 16:28:09]

Hi Jonathan,
Is there any advice on the question below?

> I have a question about content::NOTIFICATION_RENDER_WIDGET_HOST_DID_PAINT.
> Is it OK to rely on this notification when we switch webui to use accelerated
composition?
> What I'd like to do is holding off the display of Widget until the web
contents is rendered using this notification.

[mazda, 2012-05-30 20:32:53]

Ben,
Could you do an OWNERS review for chrome/browser/ui?

[Ben Goodger (Google), 2012-05-30 20:49:12]

http://codereview.chromium.org/10442044/diff/5001/chrome/browser/ui/views/key...
File chrome/browser/ui/views/keyboard_overlay_delegate.cc (right):

http://codereview.chromium.org/10442044/diff/5001/chrome/browser/ui/views/key...
chrome/browser/ui/views/keyboard_overlay_delegate.cc:66: widget->SetOpacity(0);
Can we just get away with not calling Show() here instead of messing with the
opacity?

[mazda, 2012-05-31 00:22:17]

Thank you for the comment.
Please take another look.

http://codereview.chromium.org/10442044/diff/5001/chrome/browser/ui/views/key...
File chrome/browser/ui/views/keyboard_overlay_delegate.cc (right):

http://codereview.chromium.org/10442044/diff/5001/chrome/browser/ui/views/key...
chrome/browser/ui/views/keyboard_overlay_delegate.cc:66: widget->SetOpacity(0);
On 2012/05/30 20:49:12, Ben Goodger (Google) wrote:
> Can we just get away with not calling Show() here instead of messing with the
> opacity?

I could remove SetOpacity by delaying calling Show().
I didn't know the web content is loaded without calling Show().
Thanks!

[jonathan.backer, 2012-05-31 15:14:05]

On 2012/05/25 23:30:37, mazda wrote:
> Jonathan,
> I have a question about content::NOTIFICATION_RENDER_WIDGET_HOST_DID_PAINT.
> Is it OK to rely on this notification when we switch webui to use accelerated
> composition?

No. With accelerated compositing and --enable-threaded-compositing (CrOS
default) this message not get sent. nkostylev@ started an e-mail thread with
nduca@ about this. I will forward it to you.

[mazda, 2012-06-01 17:13:43]

Jonathan,
Thank you for the comment and the pointer.

Oshima-san,
Could you take another look at the change?
I changed the implementation with nduca's suggestion that uses
requestAnimationFrame, the callback of which is called regardless of
GPU-composited mode or software mode.

Thanks,

[mazda, 2012-06-05 21:46:04]

Oshima-san: ping?

[oshima, 2012-06-05 21:50:11]

LGTM

[mazda, 2012-06-06 01:13:59]

Ben,
Could you do an OWNERS review for chrome/browser/ui again?

[Ben Goodger (Google), 2012-06-12 18:22:44]

lgtm

[commit-bot: I haz the power, 2012-06-12 18:37:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mazda@chromium.org/10442044/22001

[commit-bot: I haz the power, 2012-06-12 18:37:07]

Presubmit check for 10442044-22001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/browser/resources/chromeos

Presubmit checks took 1.5s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[mazda, 2012-06-12 18:38:45]

Xiyuan,
Could you do an owners review for chrome/browser/resources/chromeos?

[xiyuan, 2012-06-12 19:18:03]

lgtm

[commit-bot: I haz the power, 2012-06-12 19:27:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mazda@chromium.org/10442044/22001

[commit-bot: I haz the power, 2012-06-12 21:06:27]

Change committed as 141737