Wait showing html dialog until renderer finish painting after page is loaded.

re-landing after clang fix. clang bot is red due to bus error in linker.

Wait showing html dialog until renderre finish painting after page is loaded.
 This change keeps track of state transition to make sure we only show the window on the paint after page load.

minor change; use gdk's debug paint. Views no longer manage the damaged rect by itself so we can simply use gdk's debug paint.


Note to mazda. Please consider adding fade-in animation. I believe it will make it much nicer.

BUG=chromium-os:15809
TEST=open keyboard overlay on device. no white flicker should be observed.

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

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

[oshima, 2011-06-07 01:19:29]

I kept if/def minimum because we probably want to do something similar for
windows as well at some point. Please let me know if you think otherwise.

[mazda, 2011-06-07 03:26:58]

lgtm with nits

In description: renderre -> renderer

http://codereview.chromium.org/7024032/diff/4001/chrome/browser/ui/views/keyb...
File chrome/browser/ui/views/keyboard_overlay_dialog_view.cc (right):

http://codereview.chromium.org/7024032/diff/4001/chrome/browser/ui/views/keyb...
chrome/browser/ui/views/keyboard_overlay_dialog_view.cc:15: #include
"content/common/notification_source.h"
Is this necessary?

[sky, 2011-06-07 15:41:16]

LGTM

http://codereview.chromium.org/7024032/diff/4001/chrome/browser/ui/views/html...
File chrome/browser/ui/views/html_dialog_view.cc (right):

http://codereview.chromium.org/7024032/diff/4001/chrome/browser/ui/views/html...
chrome/browser/ui/views/html_dialog_view.cc:80: #if defined(OS_CHROMEOS)
Should the ifdefs be OS_LINUX?

http://codereview.chromium.org/7024032/diff/4001/chrome/browser/ui/views/html...
chrome/browser/ui/views/html_dialog_view.cc:260: if (type ==
NotificationType::RENDER_VIEW_HOST_CREATED_FOR_TAB) {
nit: a switch would be more readable here.

http://codereview.chromium.org/7024032/diff/4001/chrome/browser/ui/views/html...
File chrome/browser/ui/views/html_dialog_view.h (right):

http://codereview.chromium.org/7024032/diff/4001/chrome/browser/ui/views/html...
chrome/browser/ui/views/html_dialog_view.h:86: const NotificationDetails&
details);
OVERRIDE

http://codereview.chromium.org/7024032/diff/4001/chrome/browser/ui/views/html...
chrome/browser/ui/views/html_dialog_view.h:104: DialogState state_;
newline between 103 and 104

http://codereview.chromium.org/7024032/diff/4001/views/widget/native_widget_g...
File views/widget/native_widget_gtk.h (right):

http://codereview.chromium.org/7024032/diff/4001/views/widget/native_widget_g...
views/widget/native_widget_gtk.h:121: // widget is painted.  This is used if the
painting the gtk widget
'if the painting' -> 'if painting'

[oshima, 2011-06-07 19:08:58]

Uploaded new patch and added browser_test. Scott, can you take another look?

http://codereview.chromium.org/7024032/diff/4001/chrome/browser/ui/views/html...
File chrome/browser/ui/views/html_dialog_view.cc (right):

http://codereview.chromium.org/7024032/diff/4001/chrome/browser/ui/views/html...
chrome/browser/ui/views/html_dialog_view.cc:80: #if defined(OS_CHROMEOS)
On 2011/06/07 15:41:17, sky wrote:
> Should the ifdefs be OS_LINUX?

This make sense only with chromeos wm. Note that this requires different
mechanism when we move to NativeWidgetViews.

http://codereview.chromium.org/7024032/diff/4001/chrome/browser/ui/views/html...
chrome/browser/ui/views/html_dialog_view.cc:260: if (type ==
NotificationType::RENDER_VIEW_HOST_CREATED_FOR_TAB) {
On 2011/06/07 15:41:17, sky wrote:
> nit: a switch would be more readable here.

Done.

http://codereview.chromium.org/7024032/diff/4001/chrome/browser/ui/views/keyb...
File chrome/browser/ui/views/keyboard_overlay_dialog_view.cc (right):

http://codereview.chromium.org/7024032/diff/4001/chrome/browser/ui/views/keyb...
chrome/browser/ui/views/keyboard_overlay_dialog_view.cc:15: #include
"content/common/notification_source.h"
On 2011/06/07 03:26:58, mazda wrote:
> Is this necessary?

Done.

http://codereview.chromium.org/7024032/diff/4001/views/widget/native_widget_g...
File views/widget/native_widget_gtk.h (right):

http://codereview.chromium.org/7024032/diff/4001/views/widget/native_widget_g...
views/widget/native_widget_gtk.h:121: // widget is painted.  This is used if the
painting the gtk widget
On 2011/06/07 15:41:17, sky wrote:
> 'if the painting' -> 'if painting'

Done.

[oshima, 2011-06-07 19:10:09]

http://codereview.chromium.org/7024032/diff/14001/chrome/browser/ui/views/htm...
File chrome/browser/ui/views/html_dialog_view_browsertest.cc (right):

http://codereview.chromium.org/7024032/diff/14001/chrome/browser/ui/views/htm...
chrome/browser/ui/views/html_dialog_view_browsertest.cc:45: return
GURL(chrome::kAboutAboutURL);
this has to be some real page so that it gets painted after load.

[sky, 2011-06-07 20:07:52]

LGTM

[oshima, 2011-06-09 23:28:36]

On 2011/06/07 20:07:52, sky wrote:
> LGTM

Dave, requesting merge approval to 782