Merge lp:~smspillaz/unity/4.0.fix_881190_918360 into lp:unity/4.0

[Sam Spilsbury]

    Fix LP #881190 and a condition that could happen after: #918360 due to
    a incorrect refcounting.

    The transient for reader would report every window as a "transient" if it
    matched the w->transientFor () definition on the window - at destruction time
    this will be zero, so it will match every single window. That would cause
    all windows to be unminimized.

    However, it makes no sense to do the unminimize/minimize normally dance when
    the window has been destroyed.

    Additionaly, there was a cyclic reference in CompizMinimizedWindowHandler,
    which would cause those objects to never be destroyed. A condition could
    happen where CompizMinimizedWindowHandler could loop its own list and dereference
    a CompWindow that was gone.

[Sam Spilsbury (smspillaz)]

review: Disapprove

[Sam Spilsbury (smspillaz)]

Dropped for 4.0 while the fix is reworked again

Unmerged revisions

1733. By Sam Spilsbury on 2012-01-19

    Fix LP #881190 and a condition that could happen after: #918360 due to
    a incorrect refcounting.

    The transient for reader would report every window as a "transient" if it
    matched the w->transientFor () definition on the window - at destruction time
    this will be zero, so it will match every single window. That would cause
    all windows to be unminimized.

    However, it makes no sense to do the unminimize/minimize normally dance when
    the window has been destroyed.

    Additionaly, there was a cyclic reference in CompizMinimizedWindowHandler,
    which would cause those objects to never be destroyed. A condition could
    happen where CompizMinimizedWindowHandler could loop its own list and dereference
    a CompWindow that was gone.

=== modified file 'plugins/unityshell/src/compizminimizedwindowhandler.h'
--- plugins/unityshell/src/compizminimizedwindowhandler.h	2011-10-05 12:20:19 +0000
+++ plugins/unityshell/src/compizminimizedwindowhandler.h	2012-01-19 03:38:23 +0000
@@ -45,9 +45,12 @@
 {
 public:
 
-  CompizMinimizedWindowHandler (CompWindow *w);
   ~CompizMinimizedWindowHandler ();
 
+  typedef CompizMinimizedWindowHandler<Screen, Window> CompizMinimizedWindowHandler_complete;
+  typedef boost::shared_ptr<CompizMinimizedWindowHandler_complete> Ptr;
+  typedef std::list <CompizMinimizedWindowHandler *> List;
+
   void setVisibility (bool visible);
   unsigned int getPaintMask ();
 
@@ -58,23 +61,23 @@
 
   void windowNotify (CompWindowNotify n);
 
+  static MinimizedWindowHandler::Ptr create (CompWindow *w);
+
   static void setFunctions (bool keepMinimized);
   static void handleCompizEvent (const char *, const char *, CompOption::Vector &);
   static void handleEvent (XEvent *event);
   static std::list<CompWindow *> minimizingWindows;
-
-  typedef CompizMinimizedWindowHandler<Screen, Window> CompizMinimizedWindowHandler_complete;
-  typedef boost::shared_ptr<CompizMinimizedWindowHandler_complete> Ptr;
-  typedef std::list <Ptr> List;
 protected:
 
-  virtual std::vector<unsigned int> getTransients ();
+  std::vector<unsigned int> getTransients ();
 
 private:
 
+  CompizMinimizedWindowHandler (CompWindow *w);
+
   PrivateCompizMinimizedWindowHandler *priv;
   static bool handleEvents;
-  static std::list<Ptr> minimizedWindows;
+  static std::list<CompizMinimizedWindowHandler *> minimizedWindows;
 };
 }
 
@@ -88,6 +91,14 @@
 bool compiz::CompizMinimizedWindowHandler<Screen, Window>::handleEvents = true;
 
 template <typename Screen, typename Window>
+compiz::MinimizedWindowHandler::Ptr
+compiz::CompizMinimizedWindowHandler<Screen, Window>::create(CompWindow *w)
+{
+  Ptr p (new compiz::CompizMinimizedWindowHandler<Screen, Window> (w));
+  return boost::static_pointer_cast <MinimizedWindowHandler> (p);
+}
+
+template <typename Screen, typename Window>
 compiz::CompizMinimizedWindowHandler<Screen, Window>::CompizMinimizedWindowHandler(CompWindow *w) :
   MinimizedWindowHandler (screen->dpy (), w->id ())
 {
@@ -100,12 +111,19 @@
 template <typename Screen, typename Window>
 compiz::CompizMinimizedWindowHandler<Screen, Window>::~CompizMinimizedWindowHandler ()
 {
-  typedef compiz::CompizMinimizedWindowHandler<Screen, Window> minimized_window_handler_full;
-
-  compiz::CompizMinimizedWindowHandler<Screen, Window>::Ptr compizMinimizeHandler =
-        boost::dynamic_pointer_cast <minimized_window_handler_full> (Window::get (priv->mWindow)->mMinimizeHandler);
-
-  minimizedWindows.remove (compizMinimizeHandler);
+  if (!priv->mWindow->destroyed ())
+  {
+    priv->mWindow->unminimize ();
+    priv->mWindow->focusSetEnabled (Window::get (priv->mWindow), false);
+    priv->mWindow->minimizeSetEnabled (Window::get (priv->mWindow), false);
+    priv->mWindow->unminimizeSetEnabled (Window::get (priv->mWindow), false);
+    priv->mWindow->minimizedSetEnabled (Window::get (priv->mWindow), false);
+    priv->mWindow->minimize ();
+  }
+  else
+  {
+    minimizedWindows.remove (this);
+  }
 }
 
 template <typename Screen, typename Window>
@@ -152,10 +170,7 @@
   priv->mWindow->windowNotify (CompWindowNotifyMinimize);
   priv->mWindow->changeState (priv->mWindow->state () | CompWindowStateHiddenMask);
 
-  compiz::CompizMinimizedWindowHandler<Screen, Window>::Ptr compizMinimizeHandler =
-        boost::dynamic_pointer_cast <minimized_window_handler_full> (Window::get (priv->mWindow)->mMinimizeHandler);
-
-  minimizedWindows.push_back (compizMinimizeHandler);
+  minimizedWindows.push_back (this);
 
   for (unsigned int &w : transients)
   {
@@ -163,7 +178,7 @@
 
     if (win)
     {
-      Window::get (win)->mMinimizeHandler = MinimizedWindowHandler::Ptr (new CompizMinimizedWindowHandler (win));
+      Window::get (win)->mMinimizeHandler = CompizMinimizedWindowHandler::create (win);
       Window::get (win)->mMinimizeHandler->minimize ();
     }
   }
@@ -230,10 +245,7 @@
 
   std::vector<unsigned int> transients = getTransients ();
 
-  compiz::CompizMinimizedWindowHandler<Screen, Window>::Ptr compizMinimizeHandler =
-        boost::dynamic_pointer_cast <minimized_window_handler_full> (Window::get (priv->mWindow)->mMinimizeHandler);
-
-  minimizedWindows.remove (compizMinimizeHandler);
+  minimizedWindows.remove (this);
 
   priv->mWindow->focusSetEnabled (Window::get (priv->mWindow), true);
 
@@ -333,7 +345,7 @@
       typedef compiz::CompizMinimizedWindowHandler<Screen, Window> plugin_handler;
       Window *pw = Window::get (w);
       plugin_handler::Ptr compizMinimizeHandler =
-        boost::dynamic_pointer_cast <plugin_handler> (pw->mMinimizeHandler);
+        boost::static_pointer_cast <plugin_handler> (pw->mMinimizeHandler);
       /* Restore and re-save input shape and remove */
       if (compizMinimizeHandler)
       {

=== modified file 'plugins/unityshell/src/minimizedwindowhandler.h'
--- plugins/unityshell/src/minimizedwindowhandler.h	2011-10-05 12:20:19 +0000
+++ plugins/unityshell/src/minimizedwindowhandler.h	2012-01-19 03:38:23 +0000
@@ -27,6 +27,7 @@
 #include "transientfor.h"
 #include "inputremover.h"
 #include <boost/shared_ptr.hpp>
+#include <boost/weak_ptr.hpp>
 #include <boost/bind.hpp>
 
 // Will be merged back into compiz

=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2012-01-05 11:36:23 +0000
+++ plugins/unityshell/src/unityshell.cpp	2012-01-19 03:38:23 +0000
@@ -1640,7 +1640,7 @@
     typedef compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow> minimized_window_handler_unity;
 
     compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow>::Ptr compizMinimizeHandler =
-        boost::dynamic_pointer_cast <minimized_window_handler_unity> (mMinimizeHandler);
+        boost::static_pointer_cast <minimized_window_handler_unity> (mMinimizeHandler);
     mask |= compizMinimizeHandler->getPaintMask ();
   }
   else if (mShowdesktopHandler)
@@ -1710,7 +1710,7 @@
 
   if (!mMinimizeHandler)
   {
-    mMinimizeHandler = compiz::MinimizedWindowHandler::Ptr (new compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow> (window));
+    mMinimizeHandler = compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow>::create (window);
     mMinimizeHandler->minimize ();
   }
 }
@@ -1831,7 +1831,7 @@
     typedef compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow> minimized_window_handler_unity;
 
     compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow>::Ptr compizMinimizeHandler =
-        boost::dynamic_pointer_cast <minimized_window_handler_unity> (mMinimizeHandler);
+        boost::static_pointer_cast <minimized_window_handler_unity> (mMinimizeHandler);
     compizMinimizeHandler->windowNotify (n);
   }
   else if (mShowdesktopHandler)
@@ -1874,7 +1874,7 @@
   typedef compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow> minimized_window_handler_unity;
 
   compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow>::Ptr compizMinimizeHandler =
-      boost::dynamic_pointer_cast <minimized_window_handler_unity> (mMinimizeHandler);
+      boost::static_pointer_cast <minimized_window_handler_unity> (mMinimizeHandler);
 
   if (compizMinimizeHandler)
     compizMinimizeHandler->updateFrameRegion (region);
@@ -2307,7 +2307,7 @@
 
 /* Window init */
 UnityWindow::UnityWindow(CompWindow* window)
-  : BaseSwitchWindow (dynamic_cast<BaseSwitchScreen *> (UnityScreen::get (screen)), window)
+  : BaseSwitchWindow (static_cast<BaseSwitchScreen *> (UnityScreen::get (screen)), window)
   , PluginClassHandler<UnityWindow, CompWindow>(window)
   , window(window)
   , gWindow(GLWindow::get(window))
@@ -2368,16 +2368,6 @@
 
   UnityShowdesktopHandler::animating_windows.remove (window);
 
-  if (mMinimizeHandler && !window->destroyed ())
-  {
-    unminimize ();
-    window->focusSetEnabled (this, false);
-    window->minimizeSetEnabled (this, false);
-    window->unminimizeSetEnabled (this, false);
-    window->minimizedSetEnabled (this, false);
-    window->minimize ();
-  }
-
   mMinimizeHandler.reset ();
 
   if (mShowdesktopHandler)