yukijoou/kwin
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Use smart pointers to protect access to TabBoxClient

Browse source 
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.

CCBUG: 290482
CCBUG: 285747
CCBUG: 237345
REVIEW: 105000
This commit is contained in:
Martin Gräßlin 2012-05-20 15:52:24 +02:00
parent f24103481e
commit d75e7a6d60
8 changed files with 109 additions and 66 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
client.cpp
View file

@ -187,7 +187,7 @@ Client::Client(Workspace* ws)
#ifdef KWIN_BUILD_TABBOX
// TabBoxClient
m_tabBoxClient = new TabBox::TabBoxClientImpl(this);
m_tabBoxClient = QSharedPointer<TabBox::TabBoxClientImpl>(new TabBox::TabBoxClientImpl(this));
#endif
geom = QRect(0, 0, 100, 100); // So that decorations don't start with size being (0,0)
@ -221,9 +221,6 @@ Client::~Client()
assert(block_geometry_updates == 0);
assert(!check_active_modal);
delete bridge;
#ifdef KWIN_BUILD_TABBOX
delete m_tabBoxClient;
#endif
}
// Use destroyClient() or releaseWindow(), Client instances cannot be deleted directly

6
client.h
View file

@ -621,8 +621,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();
}
bool isFirstInTabBox() const {
return m_firstInTabBox;
@ -945,7 +945,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 m_firstInTabBox;
bool electricMaximizing;

68
tabbox/clientmodel.cpp
View file

@ -63,22 +63,26 @@ 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 WIdRole:
return qulonglong(m_clientList[ clientIndex ]->window());
return qulonglong(client->window());
case MinimizedRole:
return m_clientList[ clientIndex ]->isMinimized();
return client->isMinimized();
case CloseableRole:
//clients that claim to be first are not closeable
return m_clientList[ clientIndex ]->isCloseable() && !m_clientList[ clientIndex ]->isFirstInTabBox();
return client->isCloseable() && !client->isFirstInTabBox();
default:
return QVariant();
}
@ -87,7 +91,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();
}
@ -148,7 +153,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();
@ -165,31 +170,35 @@ 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();
QList<TabBoxClient*> stickyClients;
QList< QWeakPointer< TabBoxClient > > stickyClients;
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);
if (add != NULL) {
if (start == add) {
QWeakPointer<TabBoxClient> add = tabBox->clientToAddToList(c, desktop);
if (!add.isNull()) {
if (start == add.data()) {
m_clientList.removeAll(add);
m_clientList.prepend(add);
} else
m_clientList += add;
if (add->isFirstInTabBox()) {
if (add.data()->isFirstInTabBox()) {
stickyClients << add;
}
}
c = tabBox->nextClientFocusChain(c);
c = tabBox->nextClientFocusChain(c).data();
if (c == stop)
break;
@ -199,25 +208,25 @@ 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);
if (add != NULL) {
if (start == add) {
QWeakPointer<TabBoxClient> add = tabBox->clientToAddToList(c, desktop);
if (!add.isNull()) {
if (start == add.data()) {
m_clientList.removeAll(add);
m_clientList.prepend(add);
} else
m_clientList += add;
if (add->isFirstInTabBox()) {
if (add.data()->isFirstInTabBox()) {
stickyClients << add;
}
}
if (index >= stacking.size() - 1) {
c = NULL;
} else {
c = stacking[++index];
c = stacking[++index].data();
}
if (c == stop)
@ -226,13 +235,13 @@ void ClientModel::createClientList(int desktop, bool partialReset)
break;
}
}
foreach (TabBoxClient *c, stickyClients) {
foreach (QWeakPointer< TabBoxClient > c, stickyClients) {
m_clientList.removeAll(c);
m_clientList.prepend(c);
}
if (tabBox->config().showDesktopMode() == TabBoxConfig::ShowDesktopClient || m_clientList.isEmpty()) {
TabBoxClient* desktopClient = tabBox->desktopClient();
if (desktopClient)
QWeakPointer<TabBoxClient> desktopClient = tabBox->desktopClient();
if (!desktopClient.isNull())
m_clientList.append(desktopClient);
}
reset();
@ -244,7 +253,10 @@ void ClientModel::close(int i)
if (!ind.isValid()) {
return;
}
m_clientList.at(i)->close();
QSharedPointer<TabBoxClient> client = m_clientList.at(i).toStrongRef();
if (client) {
client->close();
}
}
void ClientModel::activate(int i)

2
tabbox/clientmodel.h
View file

@ -71,7 +71,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.

40
tabbox/tabbox.cpp
View file

@ -93,14 +93,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
@ -113,12 +113,12 @@ 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>();
}
bool TabBoxHandlerImpl::checkDesktop(TabBoxClient* client, int desktop) const
@ -153,26 +153,35 @@ bool TabBoxHandlerImpl::checkApplications(TabBoxClient* client) const
{
Client* current = (static_cast< TabBoxClientImpl* >(client))->client();
TabBoxClientImpl* c;
QListIterator< TabBoxClient* > i(clientList());
QListIterator< QWeakPointer<TabBoxClient> > i(clientList());
switch (config().clientApplicationsMode()) {
case TabBoxConfig::OneWindowPerApplication:
// check if the list already contains an entry of this application
while (i.hasNext()) {
if ((c = dynamic_cast< TabBoxClientImpl* >(i.next()))) {
QSharedPointer<TabBoxClient> client = i.next().toStrongRef();
if (!client) {
continue;
}
if ((c = dynamic_cast< TabBoxClientImpl* >(client.data()))) {
if (c->client()->resourceClass() == current->resourceClass()) {
return false;
}
}
}
return true;
case TabBoxConfig::AllWindowsCurrentApplication:
if ((c = dynamic_cast< TabBoxClientImpl* >(tabBox->activeClient()))) {
case TabBoxConfig::AllWindowsCurrentApplication: {
QSharedPointer<TabBoxClient> pointer = tabBox->activeClient().toStrongRef();
if (!pointer) {
return false;
}
if ((c = dynamic_cast< TabBoxClientImpl* >(pointer.data()))) {
if (c->client()->resourceClass() == current->resourceClass()) {
return true;
}
}
return false;
}
default: // TabBoxConfig::AllWindowsAllApplications
return true;
}
@ -205,7 +214,7 @@ bool TabBoxHandlerImpl::checkMultiScreen(TabBoxClient* client) const
}
}
TabBoxClient* TabBoxHandlerImpl::clientToAddToList(TabBoxClient* client, int desktop) const
QWeakPointer<TabBoxClient> TabBoxHandlerImpl::clientToAddToList(TabBoxClient* client, int desktop) const
{
Client* ret = NULL;
Client* current = (static_cast< TabBoxClientImpl* >(client))->client();
@ -230,7 +239,7 @@ TabBoxClient* TabBoxHandlerImpl::clientToAddToList(TabBoxClient* client, int des
if (ret)
return ret->tabBoxClient();
else
return NULL;
return QWeakPointer<TabBoxClient>();
}
TabBoxClientList TabBoxHandlerImpl::stackingOrder() const
@ -268,7 +277,7 @@ void TabBoxHandlerImpl::elevateClient(TabBoxClient *c, WId tabbox, bool b) const
}
TabBoxClient* TabBoxHandlerImpl::desktopClient() const
QWeakPointer<TabBoxClient> TabBoxHandlerImpl::desktopClient() const
{
foreach (Toplevel *toplevel, Workspace::self()->stackingOrder()) {
Client *client = qobject_cast<Client*>(toplevel);
@ -276,7 +285,7 @@ TabBoxClient* TabBoxHandlerImpl::desktopClient() const
return client->tabBoxClient();
}
}
return NULL;
return QWeakPointer<TabBoxClient>();
}
void TabBoxHandlerImpl::showOutline(const QRect &outline)
@ -575,8 +584,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;

8
tabbox/tabbox.h
View file

@ -46,19 +46,19 @@ 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 elevateClient(TabBoxClient* c, WId tabbox, bool elevate) const;
virtual void raiseClient(TabBoxClient *client) const;
virtual void restack(TabBoxClient *c, TabBoxClient *under);
virtual TabBoxClient* clientToAddToList(TabBoxClient* client, int desktop) const;
virtual TabBoxClient* desktopClient() const;
virtual QWeakPointer< TabBoxClient > clientToAddToList(KWin::TabBox::TabBoxClient* client, int desktop) const;
virtual QWeakPointer< TabBoxClient > desktopClient() const;
virtual void hideOutline();
virtual void showOutline(const QRect &outline);
virtual QVector< Window > outlineWindowIds() const;

34
tabbox/tabboxhandler.cpp
View file

@ -158,8 +158,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);
}
}
@ -415,7 +421,7 @@ bool TabBoxHandler::containsPos(const QPoint& pos) const
return w->geometry().contains(pos);
}
QModelIndex TabBoxHandler::index(KWin::TabBox::TabBoxClient* client) const
QModelIndex TabBoxHandler::index(QWeakPointer<KWin::TabBox::TabBoxClient> client) const
{
return d->clientModel()->index(client);
}
@ -440,13 +446,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;

12
tabbox/tabboxhandler.h
View file

@ -81,7 +81,7 @@ class ClientModel;
class TabBoxConfig;
class TabBoxClient;
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
@ -105,12 +105,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
@ -172,11 +172,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) const = 0;
virtual QWeakPointer<TabBoxClient> clientToAddToList(TabBoxClient* client, int desktop) const = 0;
/**
* @return The first desktop window in the stacking order.
*/
virtual TabBoxClient* desktopClient() const = 0;
virtual QWeakPointer<TabBoxClient> desktopClient() const = 0;
/**
* Activates the currently selected client and closes the TabBox.
**/
@ -278,7 +278,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