From 974a2b5071b4f0c12a97525d18c8cf7c5c93f548 Mon Sep 17 00:00:00 2001 From: David Rosca Date: Sat, 12 Mar 2016 13:02:34 +0100 Subject: [PATCH] FrameSvg: Fix dangling pointers in sharedFrames when theme changes Store theme pointer in FrameData and set it when adding to sharedFrames. REVIEW: 127344 --- autotests/framesvgtest.cpp | 21 +++++++++++++++++++++ autotests/framesvgtest.h | 1 + src/plasma/framesvg.cpp | 20 ++++++++++++-------- src/plasma/private/framesvg_p.h | 7 +++++-- 4 files changed, 39 insertions(+), 10 deletions(-) diff --git a/autotests/framesvgtest.cpp b/autotests/framesvgtest.cpp index e3c1f33b4..8db99ad04 100644 --- a/autotests/framesvgtest.cpp +++ b/autotests/framesvgtest.cpp @@ -54,4 +54,25 @@ void FrameSvgTest::contentsRect() QCOMPARE(m_frameSvg->contentsRect(), QRectF(26, 26, 48, 48)); } +void FrameSvgTest::setTheme() +{ + // Should not crash + + Plasma::FrameSvg *frameSvg = new Plasma::FrameSvg; + frameSvg->setImagePath("widgets/background"); + frameSvg->setTheme(new Plasma::Theme("breeze-light", this)); + frameSvg->framePixmap(); + frameSvg->setTheme(new Plasma::Theme("breeze-dark", this)); + frameSvg->framePixmap(); + delete frameSvg; + + frameSvg = new Plasma::FrameSvg; + frameSvg->setImagePath("widgets/background"); + frameSvg->setTheme(new Plasma::Theme("breeze-light", this)); + frameSvg->framePixmap(); + frameSvg->setTheme(new Plasma::Theme("breeze-dark", this)); + frameSvg->framePixmap(); + delete frameSvg; +} + QTEST_MAIN(FrameSvgTest) diff --git a/autotests/framesvgtest.h b/autotests/framesvgtest.h index 4d31e1026..e201becf6 100644 --- a/autotests/framesvgtest.h +++ b/autotests/framesvgtest.h @@ -37,6 +37,7 @@ public Q_SLOTS: private Q_SLOTS: void margins(); void contentsRect(); + void setTheme(); private: Plasma::FrameSvg *m_frameSvg; diff --git a/src/plasma/framesvg.cpp b/src/plasma/framesvg.cpp index 96b3156d2..a50a829d3 100644 --- a/src/plasma/framesvg.cpp +++ b/src/plasma/framesvg.cpp @@ -81,7 +81,7 @@ void FrameSvg::setImagePath(const QString &path) // we're the only user of it, let's remove it from the shared keys // we don't want to deref it, however, as we'll still be using it const QString oldKey = d->cacheId(fd, d->prefix); - FrameSvgPrivate::s_sharedFrames[theme()->d].remove(oldKey); + FrameSvgPrivate::s_sharedFrames[fd->theme].remove(oldKey); } else { // others are using this frame, so deref it for ourselves fd->deref(this); @@ -116,6 +116,7 @@ void FrameSvg::setImagePath(const QString &path) // ensure our frame is in the cache const QString key = d->cacheId(fd, d->prefix); FrameSvgPrivate::s_sharedFrames[theme()->d].insert(key, fd); + fd->theme = theme()->d; // this will emit repaintNeeded() as well when it is done d->updateAndSignalSizes(); @@ -150,7 +151,7 @@ void FrameSvg::setEnabledBorders(const EnabledBorders borders) if (fd->deref(this)) { //const QString oldKey = d->cacheId(fd, d->prefix); //qCDebug(LOG_PLASMA) << "1. Removing it" << oldKey << fd->refcount; - FrameSvgPrivate::s_sharedFrames[theme()->d].remove(oldKey); + FrameSvgPrivate::s_sharedFrames[fd->theme].remove(oldKey); delete fd; } @@ -160,7 +161,7 @@ void FrameSvg::setEnabledBorders(const EnabledBorders borders) if (fd->refcount() == 1) { // we're the only user of it, let's remove it from the shared keys // we don't want to deref it, however, as we'll still be using it - FrameSvgPrivate::s_sharedFrames[theme()->d].remove(oldKey); + FrameSvgPrivate::s_sharedFrames[fd->theme].remove(oldKey); } else { // others are using it, but we wish to change its size. so deref it, // then create a copy of it (we're automatically ref'd via the ctor), @@ -260,6 +261,7 @@ void FrameSvg::setElementPrefix(const QString &prefix) //qCDebug(LOG_PLASMA) << this << " 1. inserting as" << key; FrameSvgPrivate::s_sharedFrames[theme()->d].insert(key, newFd); + newFd->theme = theme()->d; } } else { // couldn't find anything useful, so we just create something here @@ -276,7 +278,7 @@ void FrameSvg::setElementPrefix(const QString &prefix) if (oldFrameData) { if (oldFrameData->deref(this)) { const QString oldKey = d->cacheId(oldFrameData, oldPrefix); - FrameSvgPrivate::s_sharedFrames[theme()->d].remove(oldKey); + FrameSvgPrivate::s_sharedFrames[oldFrameData->theme].remove(oldKey); delete oldFrameData; } } @@ -358,7 +360,7 @@ void FrameSvg::resizeFrame(const QSizeF &size) if (fd->deref(this)) { //const QString oldKey = d->cacheId(fd, d->prefix); //qCDebug(LOG_PLASMA) << "1. Removing it" << oldKey << fd->refcount; - FrameSvgPrivate::s_sharedFrames[theme()->d].remove(oldKey); + FrameSvgPrivate::s_sharedFrames[fd->theme].remove(oldKey); delete fd; } @@ -368,7 +370,7 @@ void FrameSvg::resizeFrame(const QSizeF &size) if (fd->refcount() == 1) { // we're the only user of it, let's remove it from the shared keys // we don't want to deref it, however, as we'll still be using it - FrameSvgPrivate::s_sharedFrames[theme()->d].remove(oldKey); + FrameSvgPrivate::s_sharedFrames[fd->theme].remove(oldKey); } else { // others are using it, but we wish to change its size. so deref it, // then create a copy of it (we're automatically ref'd via the ctor), @@ -382,6 +384,7 @@ void FrameSvg::resizeFrame(const QSizeF &size) fd->frameSize = size.toSize(); // we know it isn't in s_sharedFrames due to the check above, so insert it now FrameSvgPrivate::s_sharedFrames[theme()->d].insert(newKey, fd); + fd->theme = theme()->d; } QSizeF FrameSvg::frameSize() const @@ -539,7 +542,7 @@ void FrameSvg::clearCache() //TODO: should we clear from the Theme pixmap cache as well? if (p->deref(this)) { const QString key = d->cacheId(p, it.key()); - FrameSvgPrivate::s_sharedFrames[theme()->d].remove(key); + FrameSvgPrivate::s_sharedFrames[p->theme].remove(key); p->cachedBackground = QPixmap(); } @@ -606,7 +609,7 @@ FrameSvgPrivate::~FrameSvgPrivate() // qCDebug(LOG_PLASMA) << "2. Removing it" << key << it.value() << it.value()->refcount() << s_sharedFrames[theme()->d].contains(key); #endif #endif - s_sharedFrames[q->theme()->d].remove(key); + s_sharedFrames[it.value()->theme].remove(key); delete it.value(); } #ifdef DEBUG_FRAMESVG_CACHE @@ -690,6 +693,7 @@ QPixmap FrameSvgPrivate::alphaMask() } else { maskFrame = new FrameData(*frame, q); s_sharedFrames[q->theme()->d].insert(key, maskFrame); + maskFrame->theme = q->theme()->d; } maskFrame->enabledBorders = frame->enabledBorders; diff --git a/src/plasma/private/framesvg_p.h b/src/plasma/private/framesvg_p.h index beee956d4..e48bc56fa 100644 --- a/src/plasma/private/framesvg_p.h +++ b/src/plasma/private/framesvg_p.h @@ -50,7 +50,8 @@ public: noBorderPadding(false), stretchBorders(false), tileCenter(false), - composeOverBorder(false) + composeOverBorder(false), + theme(0) { ref(svg); } @@ -72,7 +73,8 @@ public: noBorderPadding(false), stretchBorders(false), tileCenter(false), - composeOverBorder(false) + composeOverBorder(false), + theme(0) { ref(svg); } @@ -127,6 +129,7 @@ public: bool composeOverBorder : 1; QHash references; + Plasma::ThemePrivate *theme; }; class FrameSvgPrivate