From 7b63ce043dfaec5ec83d938b1cea8ee0ead614ff Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Fri, 5 Nov 2010 12:59:18 +0100 Subject: QStyleSheetStyle: Fix crash that occurs with several instance of QStyleSheetStyle The problem is that when a widget is destroyed, it was not properly removed from the cache. We connected the destroyed signal to a slot in the QStyleSheetStyle for the first QStyleSheetStyle that handle a widget. but if this QStyleSheetStyle is destroyed, that connection is lost. Solution: Create a new QStyleSheetStyleCaches that will be responsible to clean the caches. This objects is not destroyed as long as there is a QStyleSheetStyle instance, so the connection is not lost. I took the oportunity to move all the caches to this object. Reveiwed-by: Gabriel Task-number: QTBUG-11658 --- src/gui/styles/qstylesheetstyle.cpp | 139 +++++++++------------ src/gui/styles/qstylesheetstyle_p.h | 20 ++- .../auto/qstylesheetstyle/tst_qstylesheetstyle.cpp | 32 +++++ 3 files changed, 106 insertions(+), 85 deletions(-) diff --git a/src/gui/styles/qstylesheetstyle.cpp b/src/gui/styles/qstylesheetstyle.cpp index e23c246b0b..fb6fc23200 100644 --- a/src/gui/styles/qstylesheetstyle.cpp +++ b/src/gui/styles/qstylesheetstyle.cpp @@ -99,14 +99,7 @@ public: }; -static QHash > *styleRulesCache = 0; -static QHash > *hasStyleRuleCache = 0; -typedef QHash > QRenderRules; -static QHash *renderRulesCache = 0; -static QHash *customPaletteWidgets = 0; // widgets whose palette we tampered -static QHash *styleSheetCache = 0; // parsed style sheets -static QSet *autoFillDisabledWidgets = 0; - +static QStyleSheetStyleCaches *styleSheetCaches = 0; /* RECURSION_GUARD: * the QStyleSheetStyle is a proxy. If used with others proxy style, we may end up with something like: @@ -1525,8 +1518,8 @@ private: QVector QStyleSheetStyle::styleRules(const QWidget *w) const { - QHash >::const_iterator cacheIt = styleRulesCache->constFind(w); - if (cacheIt != styleRulesCache->constEnd()) + QHash >::const_iterator cacheIt = styleSheetCaches->styleRulesCache.constFind(w); + if (cacheIt != styleSheetCaches->styleRulesCache.constEnd()) return cacheIt.value(); if (!initWidget(w)) { @@ -1536,12 +1529,12 @@ QVector QStyleSheetStyle::styleRules(const QWidget *w) const QStyleSheetStyleSelector styleSelector; StyleSheet defaultSs; - QHash::const_iterator defaultCacheIt = styleSheetCache->constFind(baseStyle()); - if (defaultCacheIt == styleSheetCache->constEnd()) { + QHash::const_iterator defaultCacheIt = styleSheetCaches->styleSheetCache.constFind(baseStyle()); + if (defaultCacheIt == styleSheetCaches->styleSheetCache.constEnd()) { defaultSs = getDefaultStyleSheet(); QStyle *bs = baseStyle(); - styleSheetCache->insert(bs, defaultSs); - QObject::connect(bs, SIGNAL(destroyed(QObject*)), this, SLOT(styleDestroyed(QObject*)), Qt::UniqueConnection); + styleSheetCaches->styleSheetCache.insert(bs, defaultSs); + QObject::connect(bs, SIGNAL(destroyed(QObject*)), styleSheetCaches, SLOT(styleDestroyed(QObject*)), Qt::UniqueConnection); } else { defaultSs = defaultCacheIt.value(); } @@ -1549,8 +1542,8 @@ QVector QStyleSheetStyle::styleRules(const QWidget *w) const if (!qApp->styleSheet().isEmpty()) { StyleSheet appSs; - QHash::const_iterator appCacheIt = styleSheetCache->constFind(qApp); - if (appCacheIt == styleSheetCache->constEnd()) { + QHash::const_iterator appCacheIt = styleSheetCaches->styleSheetCache.constFind(qApp); + if (appCacheIt == styleSheetCaches->styleSheetCache.constEnd()) { QString ss = qApp->styleSheet(); if (ss.startsWith(QLatin1String("file:///"))) ss.remove(0, 8); @@ -1559,7 +1552,7 @@ QVector QStyleSheetStyle::styleRules(const QWidget *w) const qWarning("Could not parse application stylesheet"); appSs.origin = StyleSheetOrigin_Inline; appSs.depth = 1; - styleSheetCache->insert(qApp, appSs); + styleSheetCaches->styleSheetCache.insert(qApp, appSs); } else { appSs = appCacheIt.value(); } @@ -1571,8 +1564,8 @@ QVector QStyleSheetStyle::styleRules(const QWidget *w) const if (wid->styleSheet().isEmpty()) continue; StyleSheet ss; - QHash::const_iterator widCacheIt = styleSheetCache->constFind(wid); - if (widCacheIt == styleSheetCache->constEnd()) { + QHash::const_iterator widCacheIt = styleSheetCaches->styleSheetCache.constFind(wid); + if (widCacheIt == styleSheetCaches->styleSheetCache.constEnd()) { parser.init(wid->styleSheet()); if (!parser.parse(&ss)) { parser.init(QLatin1String("* {") + wid->styleSheet() + QLatin1Char('}')); @@ -1580,7 +1573,7 @@ QVector QStyleSheetStyle::styleRules(const QWidget *w) const qWarning("Could not parse stylesheet of widget %p", wid); } ss.origin = StyleSheetOrigin_Inline; - styleSheetCache->insert(wid, ss); + styleSheetCaches->styleSheetCache.insert(wid, ss); } else { ss = widCacheIt.value(); } @@ -1595,7 +1588,7 @@ QVector QStyleSheetStyle::styleRules(const QWidget *w) const StyleSelector::NodePtr n; n.ptr = (void *)w; QVector rules = styleSelector.styleRulesForNode(n); - styleRulesCache->insert(w, rules); + styleSheetCaches->styleRulesCache.insert(w, rules); return rules; } @@ -1724,7 +1717,7 @@ static void qt_check_if_internal_widget(const QWidget **w, int *element) QRenderRule QStyleSheetStyle::renderRule(const QWidget *w, int element, quint64 state) const { qt_check_if_internal_widget(&w, &element); - QHash &cache = (*renderRulesCache)[w][element]; + QHash &cache = styleSheetCaches->renderRulesCache[w][element]; QHash::const_iterator cacheIt = cache.constFind(state); if (cacheIt != cache.constEnd()) return cacheIt.value(); @@ -2035,7 +2028,7 @@ QRenderRule QStyleSheetStyle::renderRule(const QWidget *w, const QStyleOption *o bool QStyleSheetStyle::hasStyleRule(const QWidget *w, int part) const { - QHash &cache = (*hasStyleRuleCache)[w]; + QHash &cache = styleSheetCaches->hasStyleRuleCache[w]; QHash::const_iterator cacheIt = cache.constFind(part); if (cacheIt != cache.constEnd()) return cacheIt.value(); @@ -2565,7 +2558,7 @@ void QStyleSheetStyle::setPalette(QWidget *w) rule.configurePalette(&p, map[i].group, ew, ew != w); } - customPaletteWidgets->insert(w, w->palette()); + styleSheetCaches->customPaletteWidgets.insert(w, w->palette()); w->setPalette(p); if (ew != w) ew->setPalette(p); @@ -2573,32 +2566,32 @@ void QStyleSheetStyle::setPalette(QWidget *w) void QStyleSheetStyle::unsetPalette(QWidget *w) { - if (customPaletteWidgets->contains(w)) { - QPalette p = customPaletteWidgets->value(w); + if (styleSheetCaches->customPaletteWidgets.contains(w)) { + QPalette p = styleSheetCaches->customPaletteWidgets.value(w); w->setPalette(p); QWidget *ew = embeddedWidget(w); if (ew != w) ew->setPalette(p); - customPaletteWidgets->remove(w); + styleSheetCaches->customPaletteWidgets.remove(w); } QVariant oldFont = w->property("_q_styleSheetWidgetFont"); if (oldFont.isValid()) { w->setFont(qvariant_cast(oldFont)); } - if (autoFillDisabledWidgets->contains(w)) { + if (styleSheetCaches->autoFillDisabledWidgets.contains(w)) { embeddedWidget(w)->setAutoFillBackground(true); - autoFillDisabledWidgets->remove(w); + styleSheetCaches->autoFillDisabledWidgets.remove(w); } } static void updateWidgets(const QList& widgets) { - if (!styleRulesCache->isEmpty() || !hasStyleRuleCache->isEmpty() || !renderRulesCache->isEmpty()) { + if (!styleSheetCaches->styleRulesCache.isEmpty() || !styleSheetCaches->hasStyleRuleCache.isEmpty() || !styleSheetCaches->renderRulesCache.isEmpty()) { for (int i = 0; i < widgets.size(); ++i) { const QWidget *widget = widgets.at(i); - styleRulesCache->remove(widget); - hasStyleRuleCache->remove(widget); - renderRulesCache->remove(widget); + styleSheetCaches->styleRulesCache.remove(widget); + styleSheetCaches->hasStyleRuleCache.remove(widget); + styleSheetCaches->renderRulesCache.remove(widget); } } for (int i = 0; i < widgets.size(); ++i) { @@ -2622,12 +2615,7 @@ QStyleSheetStyle::QStyleSheetStyle(QStyle *base) { ++numinstances; if (numinstances == 1) { - styleRulesCache = new QHash >; - hasStyleRuleCache = new QHash >; - renderRulesCache = new QHash; - customPaletteWidgets = new QHash; - styleSheetCache = new QHash; - autoFillDisabledWidgets = new QSet; + styleSheetCaches = new QStyleSheetStyleCaches; } } @@ -2635,18 +2623,7 @@ QStyleSheetStyle::~QStyleSheetStyle() { --numinstances; if (numinstances == 0) { - delete styleRulesCache; - styleRulesCache = 0; - delete hasStyleRuleCache; - hasStyleRuleCache = 0; - delete renderRulesCache; - renderRulesCache = 0; - delete customPaletteWidgets; - customPaletteWidgets = 0; - delete styleSheetCache; - styleSheetCache = 0; - delete autoFillDisabledWidgets; - autoFillDisabledWidgets = 0; + delete styleSheetCaches; } } QStyle *QStyleSheetStyle::baseStyle() const @@ -2658,19 +2635,19 @@ QStyle *QStyleSheetStyle::baseStyle() const return QApplication::style(); } -void QStyleSheetStyle::widgetDestroyed(QObject *o) +void QStyleSheetStyleCaches::widgetDestroyed(QObject *o) { - styleRulesCache->remove((const QWidget *)o); - hasStyleRuleCache->remove((const QWidget *)o); - renderRulesCache->remove((const QWidget *)o); - customPaletteWidgets->remove((const QWidget *)o); - styleSheetCache->remove((const QWidget *)o); - autoFillDisabledWidgets->remove((const QWidget *)o); + styleRulesCache.remove((const QWidget *)o); + hasStyleRuleCache.remove((const QWidget *)o); + renderRulesCache.remove((const QWidget *)o); + customPaletteWidgets.remove((const QWidget *)o); + styleSheetCache.remove((const QWidget *)o); + autoFillDisabledWidgets.remove((const QWidget *)o); } -void QStyleSheetStyle::styleDestroyed(QObject *o) +void QStyleSheetStyleCaches::styleDestroyed(QObject *o) { - styleSheetCache->remove(o); + styleSheetCache.remove(o); } /*! @@ -2688,7 +2665,7 @@ bool QStyleSheetStyle::initWidget(const QWidget *w) const return false; const_cast(w)->setAttribute(Qt::WA_StyleSheet, true); - QObject::connect(w, SIGNAL(destroyed(QObject*)), this, SLOT(widgetDestroyed(QObject*))); + QObject::connect(w, SIGNAL(destroyed(QObject*)), styleSheetCaches, SLOT(widgetDestroyed(QObject*)), Qt::UniqueConnection); return true; } @@ -2700,12 +2677,12 @@ void QStyleSheetStyle::polish(QWidget *w) if (!initWidget(w)) return; - if (styleRulesCache->contains(w)) { + if (styleSheetCaches->styleRulesCache.contains(w)) { // the widget accessed its style pointer before polish (or repolish) // (exemple: the QAbstractSpinBox constructor ask for the stylehint) - styleRulesCache->remove(w); - hasStyleRuleCache->remove(w); - renderRulesCache->remove(w); + styleSheetCaches->styleRulesCache.remove(w); + styleSheetCaches->hasStyleRuleCache.remove(w); + styleSheetCaches->renderRulesCache.remove(w); } setGeometry(w); setProperties(w); @@ -2771,7 +2748,7 @@ void QStyleSheetStyle::polish(QWidget *w) QWidget *ew = embeddedWidget(w); if (ew->autoFillBackground()) { ew->setAutoFillBackground(false); - autoFillDisabledWidgets->insert(w); + styleSheetCaches->autoFillDisabledWidgets.insert(w); if (ew != w) { //eg. viewport of a scrollarea //(in order to draw the background anyway in case we don't.) ew->setAttribute(Qt::WA_StyledBackground, true); @@ -2797,18 +2774,18 @@ void QStyleSheetStyle::repolish(QWidget *w) { QList children = w->findChildren(QString()); children.append(w); - styleSheetCache->remove(w); + styleSheetCaches->styleSheetCache.remove(w); updateWidgets(children); } void QStyleSheetStyle::repolish(QApplication *app) { Q_UNUSED(app); - const QList allWidgets = styleRulesCache->keys(); - styleSheetCache->remove(qApp); - styleRulesCache->clear(); - hasStyleRuleCache->clear(); - renderRulesCache->clear(); + const QList allWidgets = styleSheetCaches->styleRulesCache.keys(); + styleSheetCaches->styleSheetCache.remove(qApp); + styleSheetCaches->styleRulesCache.clear(); + styleSheetCaches->hasStyleRuleCache.clear(); + styleSheetCaches->renderRulesCache.clear(); updateWidgets(allWidgets); } @@ -2819,10 +2796,10 @@ void QStyleSheetStyle::unpolish(QWidget *w) return; } - styleRulesCache->remove(w); - hasStyleRuleCache->remove(w); - renderRulesCache->remove(w); - styleSheetCache->remove(w); + styleSheetCaches->styleRulesCache.remove(w); + styleSheetCaches->hasStyleRuleCache.remove(w); + styleSheetCaches->renderRulesCache.remove(w); + styleSheetCaches->styleSheetCache.remove(w); unsetPalette(w); w->setProperty("_q_stylesheet_minw", QVariant()); w->setProperty("_q_stylesheet_minh", QVariant()); @@ -2849,10 +2826,10 @@ void QStyleSheetStyle::unpolish(QApplication *app) { baseStyle()->unpolish(app); RECURSION_GUARD(return) - styleRulesCache->clear(); - hasStyleRuleCache->clear(); - renderRulesCache->clear(); - styleSheetCache->remove(qApp); + styleSheetCaches->styleRulesCache.clear(); + styleSheetCaches->hasStyleRuleCache.clear(); + styleSheetCaches->renderRulesCache.clear(); + styleSheetCaches->styleSheetCache.remove(qApp); } #ifndef QT_NO_TABBAR @@ -4261,7 +4238,7 @@ void QStyleSheetStyle::drawPrimitive(PrimitiveElement pe, const QStyleOption *op case PE_Widget: if (!rule.hasDrawable()) { QWidget *container = containerWidget(w); - if (autoFillDisabledWidgets->contains(container) + if (styleSheetCaches->autoFillDisabledWidgets.contains(container) && (container == w || !renderRule(container, opt).hasBackground())) { //we do not have a background, but we disabled the autofillbackground anyway. so fill the background now. // (this may happen if we have rules like :focus) diff --git a/src/gui/styles/qstylesheetstyle_p.h b/src/gui/styles/qstylesheetstyle_p.h index fd81437b7a..45649507b7 100644 --- a/src/gui/styles/qstylesheetstyle_p.h +++ b/src/gui/styles/qstylesheetstyle_p.h @@ -145,10 +145,6 @@ protected Q_SLOTS: protected: bool event(QEvent *e); -private Q_SLOTS: - void widgetDestroyed(QObject *); - void styleDestroyed(QObject *); - private: int refcount; @@ -186,6 +182,22 @@ private: Q_DECLARE_PRIVATE(QStyleSheetStyle) }; +class QStyleSheetStyleCaches : public QObject +{ + Q_OBJECT +public Q_SLOTS: + void widgetDestroyed(QObject *); + void styleDestroyed(QObject *); +public: + QHash > styleRulesCache; + QHash > hasStyleRuleCache; + typedef QHash > QRenderRules; + QHash renderRulesCache; + QHash customPaletteWidgets; // widgets whose palette we tampered + QHash styleSheetCache; // parsed style sheets + QSet autoFillDisabledWidgets; +}; + QT_END_NAMESPACE #endif // QT_NO_STYLE_STYLESHEET diff --git a/tests/auto/qstylesheetstyle/tst_qstylesheetstyle.cpp b/tests/auto/qstylesheetstyle/tst_qstylesheetstyle.cpp index 04b1e79f09..9526ad88fc 100644 --- a/tests/auto/qstylesheetstyle/tst_qstylesheetstyle.cpp +++ b/tests/auto/qstylesheetstyle/tst_qstylesheetstyle.cpp @@ -103,6 +103,7 @@ private slots: //at the end because it mess with the style. void widgetStyle(); void appStyle(); + void QTBUG11658_cachecrash(); private: QColor COLOR(const QWidget& w) { w.ensurePolished(); @@ -1622,6 +1623,37 @@ void tst_QStyleSheetStyle::changeStyleInChangeEvent() wid.ensurePolished(); } +void tst_QStyleSheetStyle::QTBUG11658_cachecrash() +{ + //should not crash + class Widget : public QWidget + { + public: + Widget(QWidget *parent = 0) + : QWidget(parent) + { + QVBoxLayout* pLayout = new QVBoxLayout(this); + QCheckBox* pCheckBox = new QCheckBox(this); + pLayout->addWidget(pCheckBox); + setLayout(pLayout); + + QString szStyleSheet = QLatin1String("* { color: red; }"); + qApp->setStyleSheet(szStyleSheet); + qApp->setStyle(QStyleFactory::create(QLatin1String("Windows"))); + } + }; + + Widget *w = new Widget(); + delete w; + w = new Widget(); + w->show(); + + QTest::qWaitForWindowShown(w); + delete w; + qApp->setStyleSheet(QString()); +} + + QTEST_MAIN(tst_QStyleSheetStyle) #include "tst_qstylesheetstyle.moc" -- cgit v1.2.3