Merge lp:~smspillaz/unity/unity.maybe_fix_861143 into lp:unity

[Sam Spilsbury]

Maybe fix LP#861143 (I can't reproduce it, but some debugging with Cimi showed what may have been going on)

Instead of explicitly undecorating windows when they are maximized, listen
for changes in decoration state on PropertyNotify events on the windows
themselves and respond to them, because we might get the maximization
event when the window is mapped before compiz has had a chance to decorate
the window.

Also cover the case where a window is withdrawn and then mapped maximized
again, where we wouldn't redecorate those windows.

In order to avoid cycling on our own undecoration requests, we set the
high bit on _MOTIF_WM_HINTS decoration field, if that high bit is
present and the window is otherwise undecorated, we should treat it
like it is still a decorated window and redecorate it when
it is restored. (Applications that explicitly undecorate the window
while this high bit is set are likely to clear the high bit and overwrite the
whole property, so we will be notified when this happens and can respond accordingly)

[Andrea Cimitan (cimi)]

Here fixes the bug

[Sam Spilsbury (smspillaz)]

Awesome

On Sat, Oct 1, 2011 at 10:43 PM, Andrea Cimitan
<email address hidden> wrote:
> Here fixes the bug
> --
> https://code.launchpad.net/~smspillaz/unity/unity.maybe_fix_861143/+merge/77786
> You are the owner of lp:~smspillaz/unity/unity.maybe_fix_861143.
>

--
Sam Spilsbury

[Andrea Cimitan (cimi)]

I can test the branch on another PC affected by the bug... Not now though
:-)

[Andrea Cimitan (cimi)]

Tested on a different pc that was exposing the bug... fixes the issue

=== modified file 'plugins/unityshell/src/PanelMenuView.cpp'
--- plugins/unityshell/src/PanelMenuView.cpp	2011-10-01 05:28:01 +0000
+++ plugins/unityshell/src/PanelMenuView.cpp	2011-10-01 06:24:23 +0000
@@ -135,8 +135,11 @@
   win_manager->window_maximized.connect(sigc::mem_fun(this, &PanelMenuView::OnWindowMaximized));
   win_manager->window_restored.connect(sigc::mem_fun(this, &PanelMenuView::OnWindowRestored));
   win_manager->window_unmapped.connect(sigc::mem_fun(this, &PanelMenuView::OnWindowUnmapped));
+  win_manager->window_mapped.connect(sigc::mem_fun(this, &PanelMenuView::OnWindowMapped));
   win_manager->window_moved.connect(sigc::mem_fun(this, &PanelMenuView::OnWindowMoved));
   win_manager->window_resized.connect(sigc::mem_fun(this, &PanelMenuView::OnWindowMoved));
+  win_manager->window_decorated.connect(sigc::mem_fun(this, &PanelMenuView::OnWindowDecorated));
+  win_manager->window_undecorated.connect(sigc::mem_fun(this, &PanelMenuView::OnWindowUndecorated));
 
   PanelStyle::GetDefault()->changed.connect(sigc::mem_fun(this, &PanelMenuView::Refresh));
 
@@ -984,7 +987,7 @@
       // if we've just started tracking this window and it is maximized, let's
       // make sure it's undecorated just in case it slipped by us earlier
       // (I'm looking at you, Chromium!)
-      if (_is_maximized)
+      if (_is_maximized && WindowManager::Default ()->IsWindowDecorated(xid))
       {
         WindowManager::Default()->Undecorate(xid);
         _maximized_set.insert(xid);
@@ -1051,8 +1054,38 @@
 void
 PanelMenuView::OnWindowUnmapped(guint32 xid)
 {
-  _decor_map.erase(xid);
-  _maximized_set.erase(xid);
+  if (WindowManager::Default()->IsWindowMaximized(xid))
+  {
+    WindowManager::Default()->Decorate(xid);
+    _maximized_set.erase(xid);
+  }
+}
+
+void
+PanelMenuView::OnWindowMapped(guint32 xid)
+{
+  if (WindowManager::Default()->IsWindowMaximized(xid))
+  {
+    WindowManager::Default()->Undecorate(xid);
+    _maximized_set.insert(xid);
+  }
+}
+
+void
+PanelMenuView::OnWindowDecorated(guint32 xid)
+{
+  _decor_map[xid] = true;
+
+  if (_maximized_set.find(xid) != _maximized_set.end ())
+  {
+    WindowManager::Default ()->Undecorate(xid);
+  }
+}
+
+void
+PanelMenuView::OnWindowUndecorated(guint32 xid)
+{
+  _decor_map[xid] = false;
 }
 
 void

=== modified file 'plugins/unityshell/src/PanelMenuView.h'
--- plugins/unityshell/src/PanelMenuView.h	2011-09-28 16:24:09 +0000
+++ plugins/unityshell/src/PanelMenuView.h	2011-10-01 06:24:23 +0000
@@ -71,9 +71,12 @@
   void OnWindowMinimized(guint32 xid);
   void OnWindowUnminimized(guint32 xid);
   void OnWindowUnmapped(guint32 xid);
+  void OnWindowMapped(guint32 xid);
   void OnWindowMaximized(guint32 xid);
   void OnWindowRestored(guint32 xid);
   void OnWindowMoved(guint32 xid);
+  void OnWindowDecorated(guint32 xid);
+  void OnWindowUndecorated(guint32 xid);
 
   guint32 GetMaximizedWindow();
 

=== modified file 'plugins/unityshell/src/PluginAdapter.cpp'
--- plugins/unityshell/src/PluginAdapter.cpp	2011-09-29 07:49:23 +0000
+++ plugins/unityshell/src/PluginAdapter.cpp	2011-10-01 06:24:23 +0000
@@ -36,6 +36,7 @@
 
 #define MWM_HINTS_FUNCTIONS     (1L << 0)
 #define MWM_HINTS_DECORATIONS   (1L << 1)
+#define MWM_HINTS_UNDECORATED_UNITY 0x80
 #define _XA_MOTIF_WM_HINTS    "_MOTIF_WM_HINTS"
 
 /* static */
@@ -141,6 +142,26 @@
 }
 
 void
+PluginAdapter::NotifyNewDecorationState(guint32 xid)
+{
+  bool wasTracked = (_window_decoration_state.find (xid) != _window_decoration_state.end ());
+  bool wasDecorated = false;
+
+  if (wasTracked)
+    wasDecorated = _window_decoration_state[xid];
+
+  bool decorated = IsWindowDecorated (xid);
+
+  if (decorated == wasDecorated)
+    return;
+
+  if (decorated && (!wasDecorated || !wasTracked))
+    WindowManager::window_decorated.emit(xid);
+  else if (wasDecorated || !wasTracked)
+    WindowManager::window_undecorated.emit(xid);
+}
+
+void
 PluginAdapter::Notify(CompWindow* window, CompWindowNotify notify)
 {
   switch (notify)
@@ -402,27 +423,20 @@
   if (!hints)
     return ret;
 
-  if (type == hints_atom && format != 0 && hints->flags & MWM_HINTS_DECORATIONS)
+  /* Check for the presence of the high bit
+   * if present, it means that we undecorated
+   * this window, so don't mark it as undecorated */
+  if (type == hints_atom && format != 0 &&
+      hints->flags & MWM_HINTS_DECORATIONS)
   {
-    ret = hints->decorations & (MwmDecorAll | MwmDecorTitle);
+    /* Must have both bits set */
+    _window_decoration_state[xid] = ret =
+          (hints->decorations & (MwmDecorAll | MwmDecorTitle))  ||
+          (hints->decorations & MWM_HINTS_UNDECORATED_UNITY);
   }
 
   XFree(hints);
   return ret;
-
-/* FIXME compiz is too slow to update this value, and this could lead to
- * issues like the bug #838923, since the read value isn't valid anymore
-  CompWindow* window;
-
-  window = m_Screen->findWindow(win);
-  if (window)
-  {
-    unsigned int decor = window->mwmDecor();
-
-    return decor & (MwmDecorAll | MwmDecorTitle);
-  }
-  return true;
-*/
 }
 
 bool
@@ -797,7 +811,7 @@
   MotifWmHints hints = { 0 };
 
   hints.flags = MWM_HINTS_DECORATIONS;
-  hints.decorations = GDK_DECOR_ALL;
+  hints.decorations = GDK_DECOR_ALL & ~(MWM_HINTS_UNDECORATED_UNITY);
 
   SetMwmWindowHints(xid, &hints);
 }
@@ -807,8 +821,11 @@
 {
   MotifWmHints hints = { 0 };
 
+  /* Set the high bit to indicate that we undecorated this
+   * window, when an application attempts to "undecorate"
+   * the window again, this bit will be cleared */
   hints.flags = MWM_HINTS_DECORATIONS;
-  hints.decorations = 0;
+  hints.decorations = MWM_HINTS_UNDECORATED_UNITY;
 
   SetMwmWindowHints(xid, &hints);
 }
@@ -831,7 +848,7 @@
   Status       status;
   std::string  win_wmclass;
   int          num_monitor;
-  CompOutput   screen;
+
   int          screen_width;
   int          screen_height;
   float        covering_part;
@@ -839,6 +856,9 @@
   if (!window)
     return;
 
+  if ((window->state() & MAXIMIZE_STATE) == MAXIMIZE_STATE)
+    return;
+
   if (window->type() != CompWindowTypeNormalMask
       || (window->actions() & MAXIMIZABLE) != MAXIMIZABLE)
     return;
@@ -856,17 +876,10 @@
     return;
 
   num_monitor = window->outputDevice();
-  screen = m_Screen->outputDevs().at(num_monitor);
-
-  screen_height = screen.workArea().height();
-  screen_width = screen.workArea().width();
-
-  if ((window->state() & MAXIMIZE_STATE) == MAXIMIZE_STATE)
-  {
-    LOG_DEBUG(logger) << "window mapped and already maximized, just undecorate";
-    Undecorate(window->id());
-    return;
-  }
+  CompOutput &o = m_Screen->outputDevs().at(num_monitor);
+
+  screen_height = o.workArea().height();
+  screen_width = o.workArea().width();
 
   // use server<parameter> because the window won't show the real parameter as
   // not mapped yet
@@ -879,7 +892,6 @@
     return;
   }
 
-  Undecorate(window->id());
   window->maximize(MAXIMIZE_STATE);
 }
 

=== modified file 'plugins/unityshell/src/PluginAdapter.h'
--- plugins/unityshell/src/PluginAdapter.h	2011-09-27 03:41:15 +0000
+++ plugins/unityshell/src/PluginAdapter.h	2011-10-01 06:24:23 +0000
@@ -106,6 +106,7 @@
   void NotifyResized(CompWindow* window, int x, int y, int w, int h);
   void NotifyStateChange(CompWindow* window, unsigned int state, unsigned int last_state);
   void NotifyCompizEvent(const char* plugin, const char* event, CompOption::Vector& option);
+  void NotifyNewDecorationState(guint32 xid);
 
   void Decorate(guint32 xid);
   void Undecorate(guint32 xid);
@@ -175,6 +176,8 @@
   bool _in_show_desktop;
   CompWindow* _last_focused_window;
 
+  std::map<guint32, unsigned int> _window_decoration_state;
+
   static PluginAdapter* _default;
 };
 

=== modified file 'plugins/unityshell/src/WindowManager.h'
--- plugins/unityshell/src/WindowManager.h	2011-09-27 03:41:15 +0000
+++ plugins/unityshell/src/WindowManager.h	2011-10-01 06:24:23 +0000
@@ -107,6 +107,8 @@
   sigc::signal<void, guint32> window_resized;
   sigc::signal<void, guint32> window_moved;
   sigc::signal<void, guint32> window_focus_changed;
+  sigc::signal<void, guint32> window_decorated;
+  sigc::signal<void, guint32> window_undecorated;
 
   sigc::signal<void> initiate_spread;
   sigc::signal<void> terminate_spread;

=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2011-09-30 15:55:29 +0000
+++ plugins/unityshell/src/unityshell.cpp	2011-10-01 06:24:23 +0000
@@ -1021,6 +1021,14 @@
   if (!skip_other_plugins)
     screen->handleEvent(event);
 
+  if (event->type == PropertyNotify)
+  {
+    if (event->xproperty.atom == Atoms::mwmHints)
+    {
+      PluginAdapter::Default ()->NotifyNewDecorationState(event->xproperty.window);
+    }
+  }
+
   if (!skip_other_plugins &&
       screen->otherGrabExist("deco", "move", "switcher", "resize", NULL) &&
       !switcherController->Visible())