[GTK] Report isMinimized and correctly restore app windows.

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
 #include "chrome/browser/ui/gtk/gtk_util.h"
 #include "chrome/browser/ui/gtk/gtk_window_util.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/extensions/extension.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"
 #include "ui/base/x/active_window_watcher_x.h"
 #include "ui/gfx/gtk_util.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/rect.h"
 
 using apps::ShellWindow;
 
 namespace {
 
 // The timeout in milliseconds before we'll get the true window position with
 // 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,
                                        const ShellWindow::CreateParams& params)
     : shell_window_(shell_window),
       window_(NULL),
       state_(GDK_WINDOW_STATE_WITHDRAWN),
       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 =
       web_contents()->GetView()->GetNativeView();
   gtk_container_add(GTK_CONTAINER(window_), native_view);
 
   if (params.bounds.x() != INT_MIN && params.bounds.y() != INT_MIN)
     gtk_window_move(window_, params.bounds.x(), params.bounds.y());
 
   // This is done to avoid a WM "feature" where setting the window size to
@@ skipping 68 matching lines @@
   g_signal_connect(window_, "configure-event",
                    G_CALLBACK(OnConfigureThunk), this);
   g_signal_connect(window_, "window-state-event",
                    G_CALLBACK(OnWindowStateThunk), this);
   if (frameless_) {
     g_signal_connect(window_, "button-press-event",
                      G_CALLBACK(OnButtonPressThunk), this);
     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(
       shell_window_->profile(),
       window_,
       extensions::ExtensionKeybindingRegistry::PLATFORM_APPS_ONLY,
       shell_window_));
 
   ui::ActiveWindowWatcherX::AddObserver(this);
 }
 
 NativeAppWindowGtk::~NativeAppWindowGtk() {
   ui::ActiveWindowWatcherX::RemoveObserver(this);
+  gdk_window_remove_filter(NULL,
+                           &NativeAppWindowGtk::OnXEventThunk,
+                           this);
 }
 
 bool NativeAppWindowGtk::IsActive() const {
   if (ui::ActiveWindowWatcherX::WMSupportsActivation())
     return is_active_;
 
   // This still works even though we don't get the activation notification.
   return gtk_window_is_active(window_);
 }
 
@@ skipping 64 matching lines @@
 
 void NativeAppWindowGtk::Activate() {
   gtk_window_present(window_);
 }
 
 void NativeAppWindowGtk::Deactivate() {
   gdk_window_lower(gtk_widget_get_window(GTK_WIDGET(window_)));
 }
 
 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_);
 }
 
 void NativeAppWindowGtk::Minimize() {
   gtk_window_iconify(window_);
 }
 
 void NativeAppWindowGtk::Restore() {
   if (IsMaximized())
     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) {
   gfx::Rect content_bounds = bounds;
   content_bounds.Inset(GetFrameInsets());
   gtk_window_move(window_, content_bounds.x(), content_bounds.y());
   if (!resizable_) {
     if (frameless_ &&
         gtk_window_util::BoundsMatchMonitorSize(window_, content_bounds)) {
       content_bounds.set_height(content_bounds.height() - 1);
     }
     // TODO(jeremya): set_size_request doesn't honor min/max size, so the
     // bounds should be constrained manually.
     gtk_widget_set_size_request(GTK_WIDGET(window_),
         content_bounds.width(), content_bounds.height());
   } else {
     gtk_window_util::SetWindowSize(window_,
         gfx::Size(bounds.width(), bounds.height()));
   }
 }
 
+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);
 }
 
 bool NativeAppWindowGtk::IsAlwaysOnTop() const {
   return false;
 }
 
 void NativeAppWindowGtk::RenderViewHostChanged() {
   web_contents()->GetView()->Focus();
@@ skipping 277 matching lines @@
 
   draggable_region_.reset(ShellWindow::RawDraggableRegionsToSkRegion(regions));
 }
 
 // static
 NativeAppWindow* NativeAppWindow::Create(
     ShellWindow* shell_window,
     const ShellWindow::CreateParams& params) {
   return new NativeAppWindowGtk(shell_window, params);
 }