[GTK] Report isMinimized and correctly restore app windows.

chrome/browser/ui/gtk/extensions/native_app_window_gtk.cc

Index: chrome/browser/ui/gtk/extensions/native_app_window_gtk.cc
diff --git a/chrome/browser/ui/gtk/extensions/native_app_window_gtk.cc b/chrome/browser/ui/gtk/extensions/native_app_window_gtk.cc
index e96e322bcb66b627003f07e7302ff2aa96be3260..1537cc89f123620adf6e9459ab0eaf7f2f906e26 100644
--- a/chrome/browser/ui/gtk/extensions/native_app_window_gtk.cc
+++ b/chrome/browser/ui/gtk/extensions/native_app_window_gtk.cc
@@ -4,6 +4,10 @@
 
 #include "chrome/browser/ui/gtk/extensions/native_app_window_gtk.h"
 
+#include <gdk/gdkx.h>
+#include <vector>
+
+#include "base/message_loop/message_pump_gtk.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/gtk/extensions/extension_keybinding_registry_gtk.h"
@@ -28,6 +32,12 @@ namespace {
 // gtk_window_get_position() after the last GTK configure-event signal.
 const int kDebounceTimeoutMilliseconds = 100;
 
+const char* kAtomsToCache[] = {
+  "_NET_WM_STATE",
+  "_NET_WM_STATE_HIDDEN",
+  NULL
+};
+
 } // namespace
 
 NativeAppWindowGtk::NativeAppWindowGtk(ShellWindow* shell_window,
@@ -38,7 +48,8 @@ NativeAppWindowGtk::NativeAppWindowGtk(ShellWindow* shell_window,
       is_active_(false),
       content_thinks_its_fullscreen_(false),
       frameless_(params.frame == ShellWindow::FRAME_NONE),
-      frame_cursor_(NULL) {
+      frame_cursor_(NULL),
+      atom_cache_(base::MessagePumpGtk::GetDefaultXDisplay(), kAtomsToCache) {
   window_ = GTK_WINDOW(gtk_window_new(GTK_WINDOW_TOPLEVEL));
 
   gfx::NativeView native_view =
@@ -127,6 +138,10 @@ NativeAppWindowGtk::NativeAppWindowGtk(ShellWindow* shell_window,
     g_signal_connect(window_, "motion-notify-event",
                      G_CALLBACK(OnMouseMoveEventThunk), this);
   }
+  GdkWindow* window = gtk_widget_get_window(GTK_WIDGET(window_));
+  gdk_window_add_filter(window,
+                        &NativeAppWindowGtk::OnXEventThunk,
+                        this);
 
   // Add the keybinding registry.
   extension_keybinding_registry_.reset(new ExtensionKeybindingRegistryGtk(
@@ -140,6 +155,9 @@ NativeAppWindowGtk::NativeAppWindowGtk(ShellWindow* shell_window,
 
 NativeAppWindowGtk::~NativeAppWindowGtk() {
   ui::ActiveWindowWatcherX::RemoveObserver(this);
+  gdk_window_remove_filter(NULL,
+                           &NativeAppWindowGtk::OnXEventThunk,
+                           this);
 }
 
 bool NativeAppWindowGtk::IsActive() const {
@@ -224,6 +242,10 @@ void NativeAppWindowGtk::Deactivate() {
 }
 
 void NativeAppWindowGtk::Maximize() {
+  // Represent the window first in order to keep the maximization behavior
+  // consistency with Windows platform. Otherwise the window will be hidden if
+  // it has been minimized.
+  gtk_window_present(window_);
   gtk_window_maximize(window_);
 }
 
@@ -236,6 +258,12 @@ void NativeAppWindowGtk::Restore() {
     gtk_window_unmaximize(window_);
   else if (IsMinimized())
     gtk_window_deiconify(window_);
+
+  // Represent the window to keep restoration behavior consistency with Windows
+  // platform.
+  // TODO(zhchbin): verify whether we need this until http://crbug.com/261013 is
+  // fixed.
+  gtk_window_present(window_);

[Daniel Erat, 2013/07/18 20:18:39]

i don't understand this call. http://crbug.com/261013 is about unhiding a
window. i think that the "Restore" in this method's name refers to unmaximizing
it -- see ui::BaseWindow::Restore(). i guess that this method does also include
code to un-minimize a window, but can't this gtk_window_present() call be moved
to the IfMinimized() case, then? it doesn't seem like it should be needed after
you unmaximize a window.

[zhchbin, 2013/07/19 02:12:29]

From our API docs, "Restore" means: "Restore the window, exiting a maximized,
minimized, or fullscreen state.", not only unmaximizing. Yes, the
gtk_window_present() method include un-minimization of window. However, this
code is added to keep restoration behavior consistency with Windows platform as
my comments here. On Windows, no matter what state the window stays, calling
restore will show the window. Though @ scheib said it's a bug if the window  is
hidden, he said that we can keep this call temporarily. So I left a TODO here.
Another case, minimize -> maximize -> restore, without this call, the window
will not show up.

Test on GNOME Shell 3.8.3. 

 }
 
 void NativeAppWindowGtk::SetBounds(const gfx::Rect& bounds) {
@@ -257,6 +285,36 @@ void NativeAppWindowGtk::SetBounds(const gfx::Rect& bounds) {
   }
 }
 
+GdkFilterReturn NativeAppWindowGtk::OnXEvent(GdkXEvent* gdk_x_event,
+                                             GdkEvent* gdk_event) {
+  // Work around for gtk+ not reporting minimize state changes, by listening for
+  // property notify events for state and checking to see if the state includes
+  // hidden. Then from that setting or clearing the iconified bit.

[Daniel Erat, 2013/07/18 20:18:39]

nit: reword this as:

  Work around GTK+ not reporting minimization state changes. Listen
  for _NET_WM_STATE property changes and use _NET_WM_STATE_HIDDEN's
  presence to set or clear the iconified bit.

[zhchbin, 2013/07/19 02:12:29]

Done.

+  // http://crbug.com/162794.
+  XEvent* x_event = static_cast<XEvent*>(gdk_x_event);
+  std::vector< ::Atom> atom_list;
+
+  if (x_event->type == PropertyNotify &&
+      x_event->xproperty.atom == atom_cache_.GetAtom("_NET_WM_STATE") &&
+      ui::GetAtomArrayProperty(GDK_WINDOW_XWINDOW(GTK_WIDGET(window_)->window),
+                               "_NET_WM_STATE",
+                               &atom_list)) {
+    std::vector< ::Atom>::iterator it =
+        std::find(atom_list.begin(),
+                  atom_list.end(),
+                  atom_cache_.GetAtom("_NET_WM_STATE_HIDDEN"));
+    if (it != atom_list.end())

[Evan Stade, 2013/07/18 18:08:26]

I prefer use of the ternary operator here. If no ternary, you need curlies
because the else has 2 lines.

[Daniel Erat, 2013/07/18 20:18:39]

agreed

[zhchbin, 2013/07/19 02:12:29]

Done.

+      state_ = GDK_WINDOW_STATE_ICONIFIED;
+    else
+      state_ =
+          static_cast<GdkWindowState>(state_ & ~GDK_WINDOW_STATE_ICONIFIED);
+
+    shell_window_->OnNativeWindowChanged();
+  }
+
+  return GDK_FILTER_CONTINUE;
+}
+
 void NativeAppWindowGtk::FlashFrame(bool flash) {
   gtk_window_set_urgency_hint(window_, flash);
 }