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

Proposed by Sam Spilsbury
Status: Rejected
Rejected by: Sam Spilsbury
Proposed branch: lp:~smspillaz/unity/4.0.fix_881190_918360
Merge into: lp:unity/4.0
Diff against target: 208 lines (+41/-38)
3 files modified
plugins/unityshell/src/compizminimizedwindowhandler.h (+35/-23)
plugins/unityshell/src/minimizedwindowhandler.h (+1/-0)
plugins/unityshell/src/unityshell.cpp (+5/-15)
To merge this branch: bzr merge lp:~smspillaz/unity/4.0.fix_881190_918360
Reviewer Review Type Date Requested Status
Sam Spilsbury (community) Disapprove
Review via email: mp+89183@code.launchpad.net

Description of the change

    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.

To post a comment you must log in.
Revision history for this message
Sam Spilsbury (smspillaz) :
review: Disapprove
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Dropped for 4.0 while the fix is reworked again

Unmerged revisions

1733. By 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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
45{45{
46public:46public:
4747
48 CompizMinimizedWindowHandler (CompWindow *w);
49 ~CompizMinimizedWindowHandler ();48 ~CompizMinimizedWindowHandler ();
5049
50 typedef CompizMinimizedWindowHandler<Screen, Window> CompizMinimizedWindowHandler_complete;
51 typedef boost::shared_ptr<CompizMinimizedWindowHandler_complete> Ptr;
52 typedef std::list <CompizMinimizedWindowHandler *> List;
53
51 void setVisibility (bool visible);54 void setVisibility (bool visible);
52 unsigned int getPaintMask ();55 unsigned int getPaintMask ();
5356
@@ -58,23 +61,23 @@
5861
59 void windowNotify (CompWindowNotify n);62 void windowNotify (CompWindowNotify n);
6063
64 static MinimizedWindowHandler::Ptr create (CompWindow *w);
65
61 static void setFunctions (bool keepMinimized);66 static void setFunctions (bool keepMinimized);
62 static void handleCompizEvent (const char *, const char *, CompOption::Vector &);67 static void handleCompizEvent (const char *, const char *, CompOption::Vector &);
63 static void handleEvent (XEvent *event);68 static void handleEvent (XEvent *event);
64 static std::list<CompWindow *> minimizingWindows;69 static std::list<CompWindow *> minimizingWindows;
65
66 typedef CompizMinimizedWindowHandler<Screen, Window> CompizMinimizedWindowHandler_complete;
67 typedef boost::shared_ptr<CompizMinimizedWindowHandler_complete> Ptr;
68 typedef std::list <Ptr> List;
69protected:70protected:
7071
71 virtual std::vector<unsigned int> getTransients ();72 std::vector<unsigned int> getTransients ();
7273
73private:74private:
7475
76 CompizMinimizedWindowHandler (CompWindow *w);
77
75 PrivateCompizMinimizedWindowHandler *priv;78 PrivateCompizMinimizedWindowHandler *priv;
76 static bool handleEvents;79 static bool handleEvents;
77 static std::list<Ptr> minimizedWindows;80 static std::list<CompizMinimizedWindowHandler *> minimizedWindows;
78};81};
79}82}
8083
@@ -88,6 +91,14 @@
88bool compiz::CompizMinimizedWindowHandler<Screen, Window>::handleEvents = true;91bool compiz::CompizMinimizedWindowHandler<Screen, Window>::handleEvents = true;
8992
90template <typename Screen, typename Window>93template <typename Screen, typename Window>
94compiz::MinimizedWindowHandler::Ptr
95compiz::CompizMinimizedWindowHandler<Screen, Window>::create(CompWindow *w)
96{
97 Ptr p (new compiz::CompizMinimizedWindowHandler<Screen, Window> (w));
98 return boost::static_pointer_cast <MinimizedWindowHandler> (p);
99}
100
101template <typename Screen, typename Window>
91compiz::CompizMinimizedWindowHandler<Screen, Window>::CompizMinimizedWindowHandler(CompWindow *w) :102compiz::CompizMinimizedWindowHandler<Screen, Window>::CompizMinimizedWindowHandler(CompWindow *w) :
92 MinimizedWindowHandler (screen->dpy (), w->id ())103 MinimizedWindowHandler (screen->dpy (), w->id ())
93{104{
@@ -100,12 +111,19 @@
100template <typename Screen, typename Window>111template <typename Screen, typename Window>
101compiz::CompizMinimizedWindowHandler<Screen, Window>::~CompizMinimizedWindowHandler ()112compiz::CompizMinimizedWindowHandler<Screen, Window>::~CompizMinimizedWindowHandler ()
102{113{
103 typedef compiz::CompizMinimizedWindowHandler<Screen, Window> minimized_window_handler_full;114 if (!priv->mWindow->destroyed ())
104115 {
105 compiz::CompizMinimizedWindowHandler<Screen, Window>::Ptr compizMinimizeHandler =116 priv->mWindow->unminimize ();
106 boost::dynamic_pointer_cast <minimized_window_handler_full> (Window::get (priv->mWindow)->mMinimizeHandler);117 priv->mWindow->focusSetEnabled (Window::get (priv->mWindow), false);
107118 priv->mWindow->minimizeSetEnabled (Window::get (priv->mWindow), false);
108 minimizedWindows.remove (compizMinimizeHandler);119 priv->mWindow->unminimizeSetEnabled (Window::get (priv->mWindow), false);
120 priv->mWindow->minimizedSetEnabled (Window::get (priv->mWindow), false);
121 priv->mWindow->minimize ();
122 }
123 else
124 {
125 minimizedWindows.remove (this);
126 }
109}127}
110128
111template <typename Screen, typename Window>129template <typename Screen, typename Window>
@@ -152,10 +170,7 @@
152 priv->mWindow->windowNotify (CompWindowNotifyMinimize);170 priv->mWindow->windowNotify (CompWindowNotifyMinimize);
153 priv->mWindow->changeState (priv->mWindow->state () | CompWindowStateHiddenMask);171 priv->mWindow->changeState (priv->mWindow->state () | CompWindowStateHiddenMask);
154172
155 compiz::CompizMinimizedWindowHandler<Screen, Window>::Ptr compizMinimizeHandler =173 minimizedWindows.push_back (this);
156 boost::dynamic_pointer_cast <minimized_window_handler_full> (Window::get (priv->mWindow)->mMinimizeHandler);
157
158 minimizedWindows.push_back (compizMinimizeHandler);
159174
160 for (unsigned int &w : transients)175 for (unsigned int &w : transients)
161 {176 {
@@ -163,7 +178,7 @@
163178
164 if (win)179 if (win)
165 {180 {
166 Window::get (win)->mMinimizeHandler = MinimizedWindowHandler::Ptr (new CompizMinimizedWindowHandler (win));181 Window::get (win)->mMinimizeHandler = CompizMinimizedWindowHandler::create (win);
167 Window::get (win)->mMinimizeHandler->minimize ();182 Window::get (win)->mMinimizeHandler->minimize ();
168 }183 }
169 }184 }
@@ -230,10 +245,7 @@
230245
231 std::vector<unsigned int> transients = getTransients ();246 std::vector<unsigned int> transients = getTransients ();
232247
233 compiz::CompizMinimizedWindowHandler<Screen, Window>::Ptr compizMinimizeHandler =248 minimizedWindows.remove (this);
234 boost::dynamic_pointer_cast <minimized_window_handler_full> (Window::get (priv->mWindow)->mMinimizeHandler);
235
236 minimizedWindows.remove (compizMinimizeHandler);
237249
238 priv->mWindow->focusSetEnabled (Window::get (priv->mWindow), true);250 priv->mWindow->focusSetEnabled (Window::get (priv->mWindow), true);
239251
@@ -333,7 +345,7 @@
333 typedef compiz::CompizMinimizedWindowHandler<Screen, Window> plugin_handler;345 typedef compiz::CompizMinimizedWindowHandler<Screen, Window> plugin_handler;
334 Window *pw = Window::get (w);346 Window *pw = Window::get (w);
335 plugin_handler::Ptr compizMinimizeHandler =347 plugin_handler::Ptr compizMinimizeHandler =
336 boost::dynamic_pointer_cast <plugin_handler> (pw->mMinimizeHandler);348 boost::static_pointer_cast <plugin_handler> (pw->mMinimizeHandler);
337 /* Restore and re-save input shape and remove */349 /* Restore and re-save input shape and remove */
338 if (compizMinimizeHandler)350 if (compizMinimizeHandler)
339 {351 {
340352
=== 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 @@
27#include "transientfor.h"27#include "transientfor.h"
28#include "inputremover.h"28#include "inputremover.h"
29#include <boost/shared_ptr.hpp>29#include <boost/shared_ptr.hpp>
30#include <boost/weak_ptr.hpp>
30#include <boost/bind.hpp>31#include <boost/bind.hpp>
3132
32// Will be merged back into compiz33// Will be merged back into compiz
3334
=== 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 @@
1640 typedef compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow> minimized_window_handler_unity;1640 typedef compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow> minimized_window_handler_unity;
16411641
1642 compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow>::Ptr compizMinimizeHandler =1642 compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow>::Ptr compizMinimizeHandler =
1643 boost::dynamic_pointer_cast <minimized_window_handler_unity> (mMinimizeHandler);1643 boost::static_pointer_cast <minimized_window_handler_unity> (mMinimizeHandler);
1644 mask |= compizMinimizeHandler->getPaintMask ();1644 mask |= compizMinimizeHandler->getPaintMask ();
1645 }1645 }
1646 else if (mShowdesktopHandler)1646 else if (mShowdesktopHandler)
@@ -1710,7 +1710,7 @@
17101710
1711 if (!mMinimizeHandler)1711 if (!mMinimizeHandler)
1712 {1712 {
1713 mMinimizeHandler = compiz::MinimizedWindowHandler::Ptr (new compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow> (window));1713 mMinimizeHandler = compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow>::create (window);
1714 mMinimizeHandler->minimize ();1714 mMinimizeHandler->minimize ();
1715 }1715 }
1716}1716}
@@ -1831,7 +1831,7 @@
1831 typedef compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow> minimized_window_handler_unity;1831 typedef compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow> minimized_window_handler_unity;
18321832
1833 compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow>::Ptr compizMinimizeHandler =1833 compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow>::Ptr compizMinimizeHandler =
1834 boost::dynamic_pointer_cast <minimized_window_handler_unity> (mMinimizeHandler);1834 boost::static_pointer_cast <minimized_window_handler_unity> (mMinimizeHandler);
1835 compizMinimizeHandler->windowNotify (n);1835 compizMinimizeHandler->windowNotify (n);
1836 }1836 }
1837 else if (mShowdesktopHandler)1837 else if (mShowdesktopHandler)
@@ -1874,7 +1874,7 @@
1874 typedef compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow> minimized_window_handler_unity;1874 typedef compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow> minimized_window_handler_unity;
18751875
1876 compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow>::Ptr compizMinimizeHandler =1876 compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow>::Ptr compizMinimizeHandler =
1877 boost::dynamic_pointer_cast <minimized_window_handler_unity> (mMinimizeHandler);1877 boost::static_pointer_cast <minimized_window_handler_unity> (mMinimizeHandler);
18781878
1879 if (compizMinimizeHandler)1879 if (compizMinimizeHandler)
1880 compizMinimizeHandler->updateFrameRegion (region);1880 compizMinimizeHandler->updateFrameRegion (region);
@@ -2307,7 +2307,7 @@
23072307
2308/* Window init */2308/* Window init */
2309UnityWindow::UnityWindow(CompWindow* window)2309UnityWindow::UnityWindow(CompWindow* window)
2310 : BaseSwitchWindow (dynamic_cast<BaseSwitchScreen *> (UnityScreen::get (screen)), window)2310 : BaseSwitchWindow (static_cast<BaseSwitchScreen *> (UnityScreen::get (screen)), window)
2311 , PluginClassHandler<UnityWindow, CompWindow>(window)2311 , PluginClassHandler<UnityWindow, CompWindow>(window)
2312 , window(window)2312 , window(window)
2313 , gWindow(GLWindow::get(window))2313 , gWindow(GLWindow::get(window))
@@ -2368,16 +2368,6 @@
23682368
2369 UnityShowdesktopHandler::animating_windows.remove (window);2369 UnityShowdesktopHandler::animating_windows.remove (window);
23702370
2371 if (mMinimizeHandler && !window->destroyed ())
2372 {
2373 unminimize ();
2374 window->focusSetEnabled (this, false);
2375 window->minimizeSetEnabled (this, false);
2376 window->unminimizeSetEnabled (this, false);
2377 window->minimizedSetEnabled (this, false);
2378 window->minimize ();
2379 }
2380
2381 mMinimizeHandler.reset ();2371 mMinimizeHandler.reset ();
23822372
2383 if (mShowdesktopHandler)2373 if (mShowdesktopHandler)

Subscribers

People subscribed via source and target branches

to all changes: