[Differential] [Commented On] D3240: [kstyle] Implement application unpolish to delete ShadowHelper

2016-11-07 Thread Martin Gräßlin
graesslin added a comment. In https://phabricator.kde.org/D3240#61517, @ivan wrote: > I've just had a crash in Cantata (on launch) with this patch. The cleanup() is called which deletes the _shadowHelper which is later used in polish(). @ivan can you run cantata through gdb and br

[Differential] [Commented On] D3240: [kstyle] Implement application unpolish to delete ShadowHelper

2016-11-03 Thread hpereiradacosta (Hugo Pereira Da Costa)
hpereiradacosta added a comment. > I don't like calling virtual methods from a dtor, always a mess. agreed > But we can do a private cleanup method and call it from both dtor and unpolish. Would that be OK to you? yes REPOSITORY rBREEZE Breeze REVISION DETAIL https:/

[Differential] [Commented On] D3240: [kstyle] Implement application unpolish to delete ShadowHelper

2016-11-03 Thread Martin Gräßlin
graesslin added a comment. In https://phabricator.kde.org/D3240#60309, @hpereiradacosta wrote: > > For the general case you are right. Unpolish is only called from QApplication::setStyle when the old style gets destroyed. On Application tear-down it's not called. > > To be honest, I

[Differential] [Commented On] D3240: [kstyle] Implement application unpolish to delete ShadowHelper

2016-11-03 Thread hpereiradacosta (Hugo Pereira Da Costa)
hpereiradacosta added a comment. > For the general case you are right. Unpolish is only called from QApplication::setStyle when the old style gets destroyed. On Application tear-down it's not called. To be honest, I am a bit uneasy about this change. Sounds somewhat like abusing t

[Differential] [Commented On] D3240: [kstyle] Implement application unpolish to delete ShadowHelper

2016-11-03 Thread Martin Gräßlin
graesslin added a comment. In https://phabricator.kde.org/D3240#60265, @hpereiradacosta wrote: > In https://phabricator.kde.org/D3240#60257, @hpereiradacosta wrote: > > > Did you test whether unpolish is actually called ? Last time I tried, it was not, at least not always. (depending

[Differential] [Commented On] D3240: [kstyle] Implement application unpolish to delete ShadowHelper

2016-11-03 Thread hpereiradacosta (Hugo Pereira Da Costa)
hpereiradacosta added a comment. In https://phabricator.kde.org/D3240#60257, @hpereiradacosta wrote: > Did you test whether unpolish is actually called ? Last time I tried, it was not, at least not always. (depending on whether you run a QApplication, KApplication, etc.) > This is wha