aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMartin Gräßlin <mgraesslin@kde.org>2012-05-20 13:52:24 (GMT)
committerMartin Gräßlin <mgraesslin@kde.org>2012-05-29 05:53:38 (GMT)
commit19c0fa5abd90a46de2ef6949a15de31111f930f4 (patch)
tree2e86f61ab0610272f1ecdbfdd6a6eefc38ccad23
parent01565511497b32b35c69228616abdf07f035c34c (diff)
Use smart pointers to protect access to TabBoxClient
Client holds a SharedPointer to the TabBoxClient and only provides access to a WeakPointer which is passed to TabBox. ClientModel is adjusted to hold a list of WeakPointers instead of the direct pointers. This fixes the following reproducable crash: 1. Configure both primary and secondary TabBox with different layouts 2. Use primary TabBox 3. Close a window, best the one which used to be active 4. Use secondary TabBox -> Crash The reason is that the ClientModel still contains the pointer to the deleted TabBoxClient in step 3 and while creating the layout access to the TabBoxClient is needed to get the Client's icon. By using the weak pointer it can be ensured that we don't try to dereference the deleted pointer and prevent the crash. Cherry-Picked from 05a3420175c88c7a106a245071d4bb3a75694e00 BUG: 290482 BUG: 285747 CCBUG: 237345 FIXED-IN: 4.8.4 REVIEW: 105000 REVIEW: 105069
-rw-r--r--kwin/client.cpp5
-rw-r--r--kwin/client.h6
-rw-r--r--kwin/tabbox/clientmodel.cpp53
-rw-r--r--kwin/tabbox/clientmodel.h2
-rw-r--r--kwin/tabbox/desktopitemdelegate.cpp6
-rw-r--r--kwin/tabbox/tabbox.cpp31
-rw-r--r--kwin/tabbox/tabbox.h8
-rw-r--r--kwin/tabbox/tabboxhandler.cpp34
-rw-r--r--kwin/tabbox/tabboxhandler.h12
9 files changed, 97 insertions, 60 deletions
diff --git a/kwin/client.cpp b/kwin/client.cpp
index 6176b90..c50ea9c 100644
--- a/kwin/client.cpp
+++ b/kwin/client.cpp
@@ -197,7 +197,7 @@ Client::Client(Workspace* ws)
#ifdef KWIN_BUILD_TABBOX
// TabBoxClient
- m_tabBoxClient = new TabBox::TabBoxClientImpl();
+ m_tabBoxClient = QSharedPointer<TabBox::TabBoxClientImpl>(new TabBox::TabBoxClientImpl());
m_tabBoxClient->setClient(this);
#endif
@@ -230,9 +230,6 @@ Client::~Client()
assert(block_geometry_updates == 0);
assert(!check_active_modal);
delete bridge;
-#ifdef KWIN_BUILD_TABBOX
- delete m_tabBoxClient;
-#endif
#ifdef KWIN_BUILD_SCRIPTING
delete scriptCache;
#endif
diff --git a/kwin/client.h b/kwin/client.h
index bdebe44..cbfc833 100644
--- a/kwin/client.h
+++ b/kwin/client.h
@@ -423,8 +423,8 @@ public:
};
void layoutDecorationRects(QRect &left, QRect &top, QRect &right, QRect &bottom, CoordinateMode mode) const;
- TabBox::TabBoxClientImpl* tabBoxClient() const {
- return m_tabBoxClient;
+ QWeakPointer<TabBox::TabBoxClientImpl> tabBoxClient() const {
+ return m_tabBoxClient.toWeakRef();
}
//sets whether the client should be treated as a SessionInteract window
@@ -718,7 +718,7 @@ private:
// we (instead of Qt) initialize the Pixmaps, and have to free them
bool m_responsibleForDecoPixmap;
PaintRedirector* paintRedirector;
- TabBox::TabBoxClientImpl* m_tabBoxClient;
+ QSharedPointer<TabBox::TabBoxClientImpl> m_tabBoxClient;
bool electricMaximizing;
QuickTileMode electricMode;
diff --git a/kwin/tabbox/clientmodel.cpp b/kwin/tabbox/clientmodel.cpp
index eea8c91..e3bc058 100644
--- a/kwin/tabbox/clientmodel.cpp
+++ b/kwin/tabbox/clientmodel.cpp
@@ -65,21 +65,25 @@ QVariant ClientModel::data(const QModelIndex& index, int role) const
int clientIndex = index.row() * columnCount() + index.column();
if (clientIndex >= m_clientList.count())
return QVariant();
+ QSharedPointer<TabBoxClient> client = m_clientList[ clientIndex ].toStrongRef();
+ if (!client) {
+ return QVariant();
+ }
switch(role) {
case Qt::DisplayRole:
case CaptionRole:
- return m_clientList[ clientIndex ]->caption();
+ return client->caption();
case ClientRole:
- return qVariantFromValue((void*)m_clientList[ clientIndex ]);
+ return qVariantFromValue((void*)client.data());
case DesktopNameRole: {
- return tabBox->desktopName(m_clientList[ clientIndex ]);
+ return tabBox->desktopName(client.data());
}
case EmptyRole:
return false;
case WIdRole:
- return qulonglong(m_clientList[ clientIndex ]->window());
+ return qulonglong(client->window());
case MinimizedRole:
- return m_clientList[ clientIndex ]->isMinimized();
+ return client->isMinimized();
default:
return QVariant();
}
@@ -88,7 +92,8 @@ QVariant ClientModel::data(const QModelIndex& index, int role) const
QString ClientModel::longestCaption() const
{
QString caption;
- foreach (TabBoxClient *client, m_clientList) {
+ foreach (QWeakPointer<TabBoxClient> clientPointer, m_clientList) {
+ QSharedPointer<TabBoxClient> client = clientPointer.toStrongRef();
if (client->caption().size() > caption.size()) {
caption = client->caption();
}
@@ -149,7 +154,7 @@ QModelIndex ClientModel::index(int row, int column, const QModelIndex& parent) c
return createIndex(row, column);
}
-QModelIndex ClientModel::index(TabBoxClient* client) const
+QModelIndex ClientModel::index(QWeakPointer<TabBoxClient> client) const
{
if (!m_clientList.contains(client))
return QModelIndex();
@@ -166,29 +171,33 @@ void ClientModel::createClientList(bool partialReset)
void ClientModel::createClientList(int desktop, bool partialReset)
{
- TabBoxClient* start = tabBox->activeClient();
+ TabBoxClient* start = tabBox->activeClient().toStrongRef().data();
// TODO: new clients are not added at correct position
- if (partialReset && !m_clientList.isEmpty())
- start = m_clientList.first();
+ if (partialReset && !m_clientList.isEmpty()) {
+ QSharedPointer<TabBoxClient> firstClient = m_clientList.first().toStrongRef();
+ if (firstClient) {
+ start = firstClient.data();
+ }
+ }
m_clientList.clear();
switch(tabBox->config().clientSwitchingMode()) {
case TabBoxConfig::FocusChainSwitching: {
- TabBoxClient* c = tabBox->nextClientFocusChain(start);
+ TabBoxClient* c = tabBox->nextClientFocusChain(start).data();
TabBoxClient* stop = c;
while (c) {
- TabBoxClient* add = tabBox->clientToAddToList(c, desktop,
+ QWeakPointer<TabBoxClient> add = tabBox->clientToAddToList(c, desktop,
tabBox->config().clientListMode() == TabBoxConfig::AllDesktopsClientList ||
tabBox->config().clientListMode() == TabBoxConfig::AllDesktopsApplicationList);
- if (add != NULL) {
- if (start == add) {
+ if (!add.isNull()) {
+ if (start == add.data()) {
m_clientList.removeAll(add);
m_clientList.prepend(add);
} else
m_clientList += add;
}
- c = tabBox->nextClientFocusChain(c);
+ c = tabBox->nextClientFocusChain(c).data();
if (c == stop)
break;
@@ -198,15 +207,15 @@ void ClientModel::createClientList(int desktop, bool partialReset)
case TabBoxConfig::StackingOrderSwitching: {
// TODO: needs improvement
TabBoxClientList stacking = tabBox->stackingOrder();
- TabBoxClient* c = stacking.first();
+ TabBoxClient* c = stacking.first().data();
TabBoxClient* stop = c;
int index = 0;
while (c) {
- TabBoxClient* add = tabBox->clientToAddToList(c, desktop,
+ QWeakPointer<TabBoxClient> add = tabBox->clientToAddToList(c, desktop,
tabBox->config().clientListMode() == TabBoxConfig::AllDesktopsClientList ||
tabBox->config().clientListMode() == TabBoxConfig::AllDesktopsApplicationList);
- if (add != NULL) {
- if (start == add) {
+ if (!add.isNull()) {
+ if (start == add.data()) {
m_clientList.removeAll(add);
m_clientList.prepend(add);
} else
@@ -215,7 +224,7 @@ void ClientModel::createClientList(int desktop, bool partialReset)
if (index >= stacking.size() - 1) {
c = NULL;
} else {
- c = stacking[++index];
+ c = stacking[++index].data();
}
if (c == stop)
@@ -225,8 +234,8 @@ void ClientModel::createClientList(int desktop, bool partialReset)
}
}
if (tabBox->config().isShowDesktop()) {
- TabBoxClient* desktopClient = tabBox->desktopClient();
- if (desktopClient)
+ QWeakPointer<TabBoxClient> desktopClient = tabBox->desktopClient();
+ if (!desktopClient.isNull())
m_clientList.append(desktopClient);
}
reset();
diff --git a/kwin/tabbox/clientmodel.h b/kwin/tabbox/clientmodel.h
index acf81dd..53a12fe 100644
--- a/kwin/tabbox/clientmodel.h
+++ b/kwin/tabbox/clientmodel.h
@@ -70,7 +70,7 @@ public:
* @return Returns the ModelIndex of given TabBoxClient or an invalid ModelIndex
* if the model does not contain the given TabBoxClient.
*/
- QModelIndex index(TabBoxClient* client) const;
+ QModelIndex index(QWeakPointer<TabBoxClient> client) const;
/**
* Generates a new list of TabBoxClients based on the current config.
diff --git a/kwin/tabbox/desktopitemdelegate.cpp b/kwin/tabbox/desktopitemdelegate.cpp
index 8c2a087..4b98bd3 100644
--- a/kwin/tabbox/desktopitemdelegate.cpp
+++ b/kwin/tabbox/desktopitemdelegate.cpp
@@ -154,7 +154,8 @@ void DesktopItemDelegate::paint(QPainter* painter, const QStyleOptionViewItem& o
qreal itemWidth = element.width();
if (element.isStretch())
itemWidth = option.rect.x() + option.rect.width() - x;
- foreach (TabBoxClient * client, clients) {
+ foreach (QWeakPointer<TabBoxClient> clientPointer, clients) {
+ QSharedPointer<TabBoxClient> client = clientPointer.toStrongRef();
if (!client)
continue;
QModelIndex clientIndex = clientModel->index(client);
@@ -255,7 +256,8 @@ QSizeF DesktopItemDelegate::rowSize(const QModelIndex& index, int row) const
TabBoxClientList clients = clientModel->clientList();
qreal elementWidth = 0.0;
qreal elementHeight = 0.0;
- foreach (TabBoxClient * client, clients) {
+ foreach (QWeakPointer<TabBoxClient> clientPointer, clients) {
+ QSharedPointer<TabBoxClient> client = clientPointer.toStrongRef();
if (!client)
continue;
QModelIndex clientIndex = clientModel->index(client);
diff --git a/kwin/tabbox/tabbox.cpp b/kwin/tabbox/tabbox.cpp
index 9cc7935..22b98f7 100644
--- a/kwin/tabbox/tabbox.cpp
+++ b/kwin/tabbox/tabbox.cpp
@@ -92,14 +92,14 @@ QString TabBoxHandlerImpl::desktopName(int desktop) const
return Workspace::self()->desktopName(desktop);
}
-TabBoxClient* TabBoxHandlerImpl::nextClientFocusChain(TabBoxClient* client) const
+QWeakPointer<TabBoxClient> TabBoxHandlerImpl::nextClientFocusChain(TabBoxClient* client) const
{
if (TabBoxClientImpl* c = static_cast< TabBoxClientImpl* >(client)) {
Client* next = Workspace::self()->tabBox()->nextClientFocusChain(c->client());
if (next)
return next->tabBoxClient();
}
- return NULL;
+ return QWeakPointer<TabBoxClient>();
}
int TabBoxHandlerImpl::nextDesktopFocusChain(int desktop) const
@@ -112,15 +112,15 @@ int TabBoxHandlerImpl::numberOfDesktops() const
return Workspace::self()->numberOfDesktops();
}
-TabBoxClient* TabBoxHandlerImpl::activeClient() const
+QWeakPointer<TabBoxClient> TabBoxHandlerImpl::activeClient() const
{
if (Workspace::self()->activeClient())
return Workspace::self()->activeClient()->tabBoxClient();
else
- return NULL;
+ return QWeakPointer<TabBoxClient>();
}
-TabBoxClient* TabBoxHandlerImpl::clientToAddToList(TabBoxClient* client, int desktop, bool allDesktops) const
+QWeakPointer<TabBoxClient> TabBoxHandlerImpl::clientToAddToList(TabBoxClient* client, int desktop, bool allDesktops) const
{
Workspace* workspace = Workspace::self();
Client* ret = NULL;
@@ -146,8 +146,12 @@ TabBoxClient* TabBoxHandlerImpl::clientToAddToList(TabBoxClient* client, int des
}
if (ret && applications) {
// check if the list already contains an entry of this application
- foreach (TabBoxClient * tabBoxClient, clientList()) {
- if (TabBoxClientImpl* c = dynamic_cast< TabBoxClientImpl* >(tabBoxClient)) {
+ foreach (QWeakPointer<TabBoxClient> tabBoxClientPointer, clientList()) {
+ QSharedPointer<TabBoxClient> tabBoxClient = tabBoxClientPointer.toStrongRef();
+ if (!tabBoxClient) {
+ continue;
+ }
+ if (TabBoxClientImpl* c = dynamic_cast< TabBoxClientImpl* >(tabBoxClient.data())) {
if (c->client()->resourceClass() == ret->resourceClass()) {
ret = NULL;
break;
@@ -163,7 +167,7 @@ TabBoxClient* TabBoxHandlerImpl::clientToAddToList(TabBoxClient* client, int des
if (ret)
return ret->tabBoxClient();
else
- return NULL;
+ return QWeakPointer<TabBoxClient>();
}
TabBoxClientList TabBoxHandlerImpl::stackingOrder() const
@@ -188,14 +192,14 @@ void TabBoxHandlerImpl::restack(TabBoxClient *c, TabBoxClient *under)
}
-TabBoxClient* TabBoxHandlerImpl::desktopClient() const
+QWeakPointer<TabBoxClient> TabBoxHandlerImpl::desktopClient() const
{
foreach (const Client * client, Workspace::self()->stackingOrder()) {
if (client->isDesktop() && client->isOnCurrentDesktop() && client->screen() == Workspace::self()->activeScreen()) {
return client->tabBoxClient();
}
}
- return NULL;
+ return QWeakPointer<TabBoxClient>();
}
void TabBoxHandlerImpl::showOutline(const QRect &outline)
@@ -442,8 +446,11 @@ ClientList TabBox::currentClientList()
{
TabBoxClientList list = m_tabBox->clientList();
ClientList ret;
- foreach (const TabBoxClient * client, list) {
- if (const TabBoxClientImpl* c = static_cast< const TabBoxClientImpl* >(client))
+ foreach (QWeakPointer<TabBoxClient> clientPointer, list) {
+ QSharedPointer<TabBoxClient> client = clientPointer.toStrongRef();
+ if (!client)
+ continue;
+ if (const TabBoxClientImpl* c = static_cast< const TabBoxClientImpl* >(client.data()))
ret.append(c->client());
}
return ret;
diff --git a/kwin/tabbox/tabbox.h b/kwin/tabbox/tabbox.h
index 6ab07d6..3e3d21b 100644
--- a/kwin/tabbox/tabbox.h
+++ b/kwin/tabbox/tabbox.h
@@ -46,18 +46,18 @@ public:
virtual ~TabBoxHandlerImpl();
virtual int activeScreen() const;
- virtual TabBoxClient* activeClient() const;
+ virtual QWeakPointer< TabBoxClient > activeClient() const;
virtual int currentDesktop() const;
virtual QString desktopName(TabBoxClient* client) const;
virtual QString desktopName(int desktop) const;
- virtual TabBoxClient* nextClientFocusChain(TabBoxClient* client) const;
+ virtual QWeakPointer< TabBoxClient > nextClientFocusChain(TabBoxClient* client) const;
virtual int nextDesktopFocusChain(int desktop) const;
virtual int numberOfDesktops() const;
virtual TabBoxClientList stackingOrder() const;
virtual void raiseClient(TabBoxClient *client) const;
virtual void restack(TabBoxClient *c, TabBoxClient *under);
- virtual TabBoxClient* clientToAddToList(TabBoxClient* client, int desktop, bool allDesktops) const;
- virtual TabBoxClient* desktopClient() const;
+ virtual QWeakPointer< TabBoxClient > clientToAddToList(TabBoxClient* client, int desktop, bool allDesktops) const;
+ virtual QWeakPointer< TabBoxClient > desktopClient() const;
virtual void hideOutline();
virtual void showOutline(const QRect &outline);
virtual QVector< Window > outlineWindowIds() const;
diff --git a/kwin/tabbox/tabboxhandler.cpp b/kwin/tabbox/tabboxhandler.cpp
index 8a1f50f..0dfe177 100644
--- a/kwin/tabbox/tabboxhandler.cpp
+++ b/kwin/tabbox/tabboxhandler.cpp
@@ -179,8 +179,14 @@ void TabBoxHandlerPrivate::updateHighlightWindows()
// TODO if ( (lastRaisedClientWasMinimized = lastRaisedClient->isMinimized()) )
// lastRaisedClient->setMinimized( false );
TabBoxClientList order = q->stackingOrder();
- int succIdx = order.indexOf(lastRaisedClient) + 1; // this is likely related to the index parameter?!
- lastRaisedClientSucc = (succIdx < order.count()) ? order.at(succIdx) : 0;
+ int succIdx = order.count() + 1;
+ for (int i=0; i<order.count(); ++i) {
+ if (order.at(i).data() == lastRaisedClient) {
+ succIdx = i + 1;
+ break;
+ }
+ }
+ lastRaisedClientSucc = (succIdx < order.count()) ? order.at(succIdx).data() : 0;
q->raiseClient(lastRaisedClient);
}
}
@@ -633,7 +639,7 @@ QModelIndex TabBoxHandler::indexAt(const QPoint& pos) const
return QModelIndex();
}
-QModelIndex TabBoxHandler::index(KWin::TabBox::TabBoxClient* client) const
+QModelIndex TabBoxHandler::index(QWeakPointer<KWin::TabBox::TabBoxClient> client) const
{
return d->clientModel()->index(client);
}
@@ -659,13 +665,29 @@ TabBoxClient* TabBoxHandler::client(const QModelIndex& index) const
void TabBoxHandler::createModel(bool partialReset)
{
switch(d->config.tabBoxMode()) {
- case TabBoxConfig::ClientTabBox:
+ case TabBoxConfig::ClientTabBox: {
d->clientModel()->createClientList(partialReset);
- if (d->lastRaisedClient && !stackingOrder().contains(d->lastRaisedClient))
+ // TODO: C++11 use lambda function
+ bool lastRaised = false;
+ bool lastRaisedSucc = false;
+ foreach (QWeakPointer<TabBoxClient> clientPointer, stackingOrder()) {
+ QSharedPointer<TabBoxClient> client = clientPointer.toStrongRef();
+ if (!client) {
+ continue;
+ }
+ if (client.data() == d->lastRaisedClient) {
+ lastRaised = true;
+ }
+ if (client.data() == d->lastRaisedClientSucc) {
+ lastRaisedSucc = true;
+ }
+ }
+ if (d->lastRaisedClient && !lastRaised)
d->lastRaisedClient = 0;
- if (d->lastRaisedClientSucc && !stackingOrder().contains(d->lastRaisedClientSucc))
+ if (d->lastRaisedClientSucc && !lastRaisedSucc)
d->lastRaisedClientSucc = 0;
break;
+ }
case TabBoxConfig::DesktopTabBox:
d->desktopModel()->createDesktopList();
break;
diff --git a/kwin/tabbox/tabboxhandler.h b/kwin/tabbox/tabboxhandler.h
index bf9fecd..080edc4 100644
--- a/kwin/tabbox/tabboxhandler.h
+++ b/kwin/tabbox/tabboxhandler.h
@@ -82,7 +82,7 @@ class TabBoxConfig;
class TabBoxClient;
class TabBoxView;
class TabBoxHandlerPrivate;
-typedef QList< TabBoxClient* > TabBoxClientList;
+typedef QList< QWeakPointer< TabBoxClient > > TabBoxClientList;
/**
* This class is a wrapper around KWin Workspace. It is used for accessing the
@@ -106,12 +106,12 @@ public:
* @return The current active TabBoxClient or NULL
* if there is no active client.
*/
- virtual TabBoxClient* activeClient() const = 0;
+ virtual QWeakPointer<TabBoxClient> activeClient() const = 0;
/**
* @param client The client which is starting point to find the next client
* @return The next TabBoxClient in focus chain
*/
- virtual TabBoxClient* nextClientFocusChain(TabBoxClient* client) const = 0;
+ virtual QWeakPointer<TabBoxClient> nextClientFocusChain(TabBoxClient* client) const = 0;
/**
* @param client The client whose desktop name should be retrieved
* @return The desktop name of the given TabBoxClient. If the client is
@@ -168,11 +168,11 @@ public:
* @param allDesktops Add clients from all desktops or only from current
* @return The client to be included in the list or NULL if it isn't to be included
*/
- virtual TabBoxClient* clientToAddToList(TabBoxClient* client, int desktop, bool allDesktops) const = 0;
+ virtual QWeakPointer<TabBoxClient> clientToAddToList(TabBoxClient* client, int desktop, bool allDesktops) const = 0;
/**
* @return The first desktop window in the stacking order.
*/
- virtual TabBoxClient* desktopClient() const = 0;
+ virtual QWeakPointer<TabBoxClient> desktopClient() const = 0;
/**
* @return The currently used TabBoxConfig
@@ -289,7 +289,7 @@ public:
* if the model does not contain the given TabBoxClient.
* @see ClientModel::index
*/
- QModelIndex index(TabBoxClient* client) const;
+ QModelIndex index(QWeakPointer<TabBoxClient> client) const;
/**
* @return Returns the current list of TabBoxClients.
* If TabBoxMode is not TabBoxConfig::ClientTabBox an empty list will