Fix for paint issue on initial screen.

 It turns out that the sync counter that we used to rely on  to synchronize window painting doesn't work for the initial screen as gtk may ignore and simply update the count without painting if the configure notify doesn't change the window size. This change uses FREEZE_UPDATES to notify WM to  guarantee that WM will show only after the window is painted.
 I also removed gdk_window_process_updates from login code because this fixes crosbug.com/12566.
 I confirmed that gdk_widnow_process_update in map handler is necessary for touch. This is due to a bug in touch code but since this path will be removed in near future, i'll keep this workaround as is for touch.

BUG=chromium-os:11514
TEST=see bug description.

Review URL: http://codereview.chromium.org/6698063

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@80056 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 108 insertions, 20 deletions

diff --git a/chrome/browser/chromeos/login/user_controller.cc b/chrome/browser/chromeos/login/user_controller.cc
index 118f197..6189d6f 100644
--- a/chrome/browser/chromeos/login/user_controller.cc
+++ b/chrome/browser/chromeos/login/user_controller.cc
@@ -66,11 +66,22 @@ class ControlsWindow : public WidgetGtk {
 
  private:
   // WidgetGtk overrides:
-  virtual void SetInitialFocus() {
+  virtual void SetInitialFocus() OVERRIDE {
     if (initial_focus_view_)
       initial_focus_view_->RequestFocus();
   }
 
+  virtual void OnMap(GtkWidget* widget) OVERRIDE {
+    // For some reason, Controls window never gets first expose event,
+    // which makes WM believe that the login screen is not ready.
+    // This is a workaround to let WM show the login screen. While
+    // this may allow WM to show unpainted window, we haven't seen any
+    // issue (yet). We will not investigate this further because we're
+    // migrating to different implemention (WebUI).
+    UpdateFreezeUpdatesProperty(GTK_WINDOW(GetNativeView()),
+                                false /* remove */);
+  }
+
   views::View* initial_focus_view_;
 
   DISALLOW_COPY_AND_ASSIGN(ControlsWindow);
@@ -190,15 +201,6 @@ void UserController::Init(int index,
   label_window_ = CreateLabelWindow(index, WM_IPC_WINDOW_LOGIN_LABEL);
   unselected_label_window_ =
       CreateLabelWindow(index, WM_IPC_WINDOW_LOGIN_UNSELECTED_LABEL);
-
-  // Flush updates to all the windows so their appearance will be synchronized
-  // when being displayed.
-  gdk_window_process_updates(controls_window_->GetNativeView()->window, false);
-  gdk_window_process_updates(image_window_->GetNativeView()->window, false);
-  gdk_window_process_updates(border_window_->GetNativeView()->window, false);
-  gdk_window_process_updates(label_window_->GetNativeView()->window, false);
-  gdk_window_process_updates(
-      unselected_label_window_->GetNativeView()->window, false);
 }
 
 void UserController::ClearAndEnableFields() {
diff --git a/views/controls/menu/native_menu_gtk.cc b/views/controls/menu/native_menu_gtk.cc
index bb5a531..7acc3a2 100644
--- a/views/controls/menu/native_menu_gtk.cc
+++ b/views/controls/menu/native_menu_gtk.cc
@@ -22,6 +22,7 @@
 #include "views/controls/menu/menu_2.h"
 #include "views/controls/menu/nested_dispatcher_gtk.h"
 #include "views/views_delegate.h"
+#include "views/widget/widget_gtk.h"
 
 namespace {
 
@@ -71,6 +72,8 @@ NativeMenuGtk::NativeMenuGtk(Menu2* menu)
       activated_index_(-1),
       activate_factory_(this),
       host_menu_(menu),
+      destroy_handler_id_(0),
+      expose_handler_id_(0),
       menu_action_(MENU_ACTION_NONE),
       nested_dispatcher_(NULL),
       ignore_button_release_(true) {
@@ -83,6 +86,7 @@ NativeMenuGtk::~NativeMenuGtk() {
     nested_dispatcher_->CreatorDestroyed();
   }
   if (menu_) {
+    DCHECK(destroy_handler_id_);
     // Don't call MenuDestroyed because menu2 has already been destroyed.
     g_signal_handler_disconnect(menu_, destroy_handler_id_);
     gtk_widget_destroy(menu_);
@@ -100,6 +104,15 @@ void NativeMenuGtk::RunMenuAt(const gfx::Point& point, int alignment) {
   ignore_button_release_ = true;
 
   UpdateStates();
+  // Set the FREEZE UPDATE property to the menu's window so that WM maps
+  // the menu after the menu painted itself.
+  GtkWidget* popup_window = gtk_widget_get_ancestor(menu_, GTK_TYPE_WINDOW);
+  CHECK(popup_window);
+  WidgetGtk::UpdateFreezeUpdatesProperty(GTK_WINDOW(popup_window),
+                                         true /* add */);
+  expose_handler_id_ = g_signal_connect_after(G_OBJECT(menu_), "expose_event",
+                                              G_CALLBACK(&OnExposeThunk), this);
+
   Position position = { point, static_cast<Menu2::Alignment>(alignment) };
   // TODO(beng): value of '1' will not work for context menus!
   gtk_menu_popup(GTK_MENU(menu_), NULL, NULL, MenuPositionFunc, &position, 1,
@@ -320,6 +333,17 @@ void NativeMenuGtk::AfterMenuMoveCurrent(GtkWidget* menu_widget,
   SendAccessibilityEvent();
 }
 
+gboolean NativeMenuGtk::OnExpose(GtkWidget* widget, GdkEventExpose* event) {
+  GtkWidget* popup_window = gtk_widget_get_ancestor(menu_, GTK_TYPE_WINDOW);
+  CHECK(popup_window);
+  CHECK(expose_handler_id_);
+  WidgetGtk::UpdateFreezeUpdatesProperty(GTK_WINDOW(popup_window),
+                                         false /* remove */);
+  g_signal_handler_disconnect(menu_, expose_handler_id_);
+  expose_handler_id_ = 0;
+  return false;
+}
+
 void NativeMenuGtk::AddSeparatorAt(int index) {
   GtkWidget* separator = gtk_separator_menu_item_new();
   gtk_widget_show(separator);
diff --git a/views/controls/menu/native_menu_gtk.h b/views/controls/menu/native_menu_gtk.h
index 4c1d21b..fce1f48 100644
--- a/views/controls/menu/native_menu_gtk.h
+++ b/views/controls/menu/native_menu_gtk.h
@@ -57,6 +57,7 @@ class NativeMenuGtk : public MenuWrapper,
                        GtkMenuDirectionType);
   CHROMEGTK_CALLBACK_1(NativeMenuGtk, void, AfterMenuMoveCurrent,
                        GtkMenuDirectionType);
+  CHROMEGTK_CALLBACK_1(NativeMenuGtk, gboolean, OnExpose, GdkEventExpose*);
 
   void AddSeparatorAt(int index);
   GtkWidget* AddMenuItemAt(int index, GtkRadioMenuItem* radio_group,
@@ -134,6 +135,7 @@ class NativeMenuGtk : public MenuWrapper,
   // used to delete the menu2 when its native menu gtk is destroyed first.
   Menu2* host_menu_;
   gulong destroy_handler_id_;
+  gulong expose_handler_id_;
 
   // The action that took place during the call to RunMenuAt.
   MenuAction menu_action_;
diff --git a/views/widget/widget_gtk.cc b/views/widget/widget_gtk.cc
index 8972ad5..3014b5c 100644
--- a/views/widget/widget_gtk.cc
+++ b/views/widget/widget_gtk.cc
@@ -7,6 +7,8 @@
 #include <gdk/gdk.h>
 #include <gdk/gdkx.h>
 #include <X11/extensions/shape.h>
+#include <X11/Xatom.h>
+#include <X11/Xlib.h>
 
 #include <set>
 #include <vector>
@@ -281,7 +283,8 @@ WidgetGtk::WidgetGtk(Type type)
       always_on_top_(false),
       is_double_buffered_(false),
       should_handle_menu_key_release_(false),
-      dragged_view_(NULL) {
+      dragged_view_(NULL),
+      painted_(false) {
   set_native_widget(this);
   static bool installed_message_loop_observer = false;
   if (!installed_message_loop_observer) {
@@ -724,6 +727,32 @@ void WidgetGtk::EnableDebugPaint() {
   debug_paint_enabled_ = true;
 }
 
+// static
+void WidgetGtk::UpdateFreezeUpdatesProperty(GtkWindow* window, bool enable) {
+  if (!GTK_WIDGET_REALIZED(GTK_WIDGET(window)))
+    gtk_widget_realize(GTK_WIDGET(window));
+  GdkWindow* gdk_window = GTK_WIDGET(window)->window;
+
+  static GdkAtom freeze_atom_ =
+      gdk_atom_intern("_CHROME_FREEZE_UPDATES", FALSE);
+  if (enable) {
+    VLOG(1) << "setting FREEZE UPDATES property. xid=" <<
+        GDK_WINDOW_XID(gdk_window);
+    int32 val = 1;
+    gdk_property_change(gdk_window,
+                        freeze_atom_,
+                        freeze_atom_,
+                        32,
+                        GDK_PROP_MODE_REPLACE,
+                        reinterpret_cast<const guchar*>(&val),
+                        1);
+  } else {
+    VLOG(1) << "deleting FREEZE UPDATES property. xid=" <<
+        GDK_WINDOW_XID(gdk_window);
+    gdk_property_delete(gdk_window, freeze_atom_);
+  }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // WidgetGtk, NativeWidget implementation:
 
@@ -915,7 +944,13 @@ void WidgetGtk::RunShellDrag(View* view,
 }
 
 void WidgetGtk::SchedulePaintInRect(const gfx::Rect& rect) {
-  if (widget_ && GTK_WIDGET_DRAWABLE(widget_)) {
+  // No need to schedule paint if
+  // 1) widget_ is NULL. This may happen because this instance may
+  // be deleted after the gtk widget has been destroyed (See OnDestroy()).
+  // 2) widget_ is not drawable (mapped and visible)
+  // 3) If it's never painted before. The first expose event will
+  // paint the area that has to be painted.
+  if (widget_ && GTK_WIDGET_DRAWABLE(widget_) && painted_) {
     gtk_widget_queue_draw_area(widget_, rect.x(), rect.y(), rect.width(),
                                rect.height());
   }
@@ -991,6 +1026,12 @@ gboolean WidgetGtk::OnPaint(GtkWidget* widget, GdkEventExpose* event) {
     canvas.set_composite_alpha(is_transparent());
     delegate_->OnNativeWidgetPaint(&canvas);
   }
+
+  if (!painted_) {
+    painted_ = true;
+    if (type_ != TYPE_CHILD)
+      UpdateFreezeUpdatesProperty(GTK_WINDOW(widget_), false /* remove */);
+  }
   return false;  // False indicates other widgets should get the event as well.
 }
 
@@ -1269,12 +1310,16 @@ void WidgetGtk::OnShow(GtkWidget* widget) {
 }
 
 void WidgetGtk::OnMap(GtkWidget* widget) {
-  // Force an expose event to trigger OnPaint. This is necessary because earlier
-  // SchedulePaintInRect calls for the widget will have happened before the
-  // widget was drawable. This means that gtk_widget_queue_draw_area wasn't
-  // called, and so the widget will not get any expose events. Consequently, the
-  // widget won't paint itself until something else triggers a paint call.
+#if defined(TOUCH_UI)
+  // Force an expose event to trigger OnPaint for touch. This is
+  // a workaround for a bug that X Expose event does not trigger
+  // Gdk's expose signal. This happens when you try to open views menu
+  // while a virtual keyboard gets kicked in or out. This seems to be
+  // a bug in message_pump_glib_x.cc as we do get X Expose event but
+  // it doesn't trigger gtk's expose signal. We're not going to fix this
+  // as we're removing gtk and migrating to new compositor.
   gdk_window_process_updates(widget_->window, true);
+#endif
 }
 
 void WidgetGtk::OnHide(GtkWidget* widget) {
@@ -1487,9 +1532,9 @@ void WidgetGtk::CreateGtkWidget(GtkWidget* parent, const gfx::Rect& bounds) {
       ConfigureWidgetForIgnoreEvents();
 
     SetAlwaysOnTop(always_on_top_);
-    // The widget needs to be realized before handlers like size-allocate can
-    // function properly.
-    gtk_widget_realize(widget_);
+    // UpdateFreezeUpdatesProperty will realize the widget and handlers like
+    // size-allocate will function properly.
+    UpdateFreezeUpdatesProperty(GTK_WINDOW(widget_), true /* add */);
   }
   SetNativeWindowProperty(kNativeWidgetKey, this);
 }
diff --git a/views/widget/widget_gtk.h b/views/widget/widget_gtk.h
index 963076f..1bb4adc 100644
--- a/views/widget/widget_gtk.h
+++ b/views/widget/widget_gtk.h
@@ -179,6 +179,17 @@ class WidgetGtk : public Widget,
   // Enables debug painting. See |debug_paint_enabled_| for details.
   static void EnableDebugPaint();
 
+  // Sets and deletes FREEZE_UPDATES property on given |window|.
+  // It adds the property when |enable| is true and remove if false.
+  // Calling this method will realize the window if it's not realized yet.
+  // This property is used to help WindowManager know when the window
+  // is fully painted so that WM can map the fully painted window.
+  // The property is based on Owen Taylor's proposal at
+  // http://mail.gnome.org/archives/wm-spec-list/2009-June/msg00002.html.
+  // This is just a hint to WM, and won't change the behavior for WM
+  // which does not support this property.
+  static void UpdateFreezeUpdatesProperty(GtkWindow* window, bool enable);
+
   // Overridden from NativeWidget:
   virtual Widget* GetWidget() OVERRIDE;
   virtual void SetNativeWindowProperty(const char* name, void* value) OVERRIDE;
@@ -428,6 +439,10 @@ class WidgetGtk : public Widget,
   // view the drag started from.
   View* dragged_view_;
 
+  // If the widget has ever been painted. This is used to guarantee
+  // that window manager shows the window only after the window is painted.
+  bool painted_;
+
   DISALLOW_COPY_AND_ASSIGN(WidgetGtk);
 };