> On Feb. 9, 2015, 5:34 p.m., Hugo Pereira Da Costa wrote: > > Although I have no objection against the change, I must admit I don't > > understand what's wrong with the current code, nor the actual description > > of the patch. > > "registerWidget may take an existing widget as a parameter. If so, we > > don't want to delete it" > > if I understand my own code right, _widget is not the argument passed as > > the registerWidget method. It is an internal member, created parentless, at > > first accepted call to registerWidget. So as such it is not 'explicitly' > > owned by anyone, and implicitly owned by us. > > And then, what is wrong with deleting it in our destructor ? > > is it because, though parentless it might get deleted elsewhere ? > > or because of a thread issue ? > > What do I miss ? > > (PS: the reason behind this interal _widget member, is that you can not > > track palettechanged events on a widget, via event filter, once you set it > > your own 'altered' palette: it won't recive these events anymore. > > So: eventFilter must be installed on either qApp (which is then getting the > > event for _every_ widget, for which we did not alter the palette, which is > > quite a lot), or on a widget for which we are sure the pallette is not > > altered. Hence: our own).
oh, you're absolutely right... I just had my valgrind traces and in my haste didn't see the difference between _widget and widget. The crash was in QApplication trying to update the palette on _widget after you change styles in the style KCM. https://paste.kde.org/pvhhfielh According to valgrind, the widget it was trying to update was very much the one deleted in the PaletteHelper destructor. I'll try replacing just delete _widget with _widget->deleteLater() and see if that crash still happens; it might be the more relevant part of the fix. It's generally a bad idea to directly delete a QObject in anything that might be called from a slot. Thanks. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122500/#review75715 ----------------------------------------------------------- On Feb. 9, 2015, 2:33 p.m., David Edmundson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122500/ > ----------------------------------------------------------- > > (Updated Feb. 9, 2015, 2:33 p.m.) > > > Review request for Plasma and Hugo Pereira Da Costa. > > > Repository: breeze > > > Description > ------- > > Don't delete widgets we don't own when changing styles > > registerWidget may take an existing widget as a parameter. If so, we > don't want to delete it when our paletteHelper is deleted for example if > we change style. > > > Diffs > ----- > > kstyle/breezepalettehelper.cpp 31c32c3 > > Diff: https://git.reviewboard.kde.org/r/122500/diff/ > > > Testing > ------- > > > Thanks, > > David Edmundson > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
