Review Request: Change the way dbus and fdo tasks are destroyed
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/3505/ --- Review request for Plasma, Aaron Seigo and Marco Martin. Summary --- Try to make dbus and fdo tasks have the same life cycle. Now the Manager class is responsible for calling deleteLater. Make DBus tasks emits only once the destroyed signal. Diffs - /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/core/manager.cpp 1110082 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/core/task.cpp 1110082 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/dbussystemtray/dbussystemtrayprotocol.cpp 1110298 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/fdo/fdoselectionmanager.cpp 1110082 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/fdo/fdotask.h 1110082 Diff: http://reviewboard.kde.org/r/3505/diff Testing --- Tested with dbus and fdo tasks inside a kde 4.4.2 session and Qt 4.6.2. Nothing crash. Thanks, Matthieu ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Change the way dbus and fdo tasks are destroyed
---
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3505/#review4897
---
/trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/dbussystemtray/dbussystemtrayprotocol.cpp
http://reviewboard.kde.org/r/3505/#comment4377
afaict from reading the code, this is the line that is incorrect. this
patch just works around that fact.
my guess is that this was done this way because deleteLater() schedules the
deletion for the next trip through the event loop.
what's probably needed here is a new method in Task that does this:
void Task::destroy()
{
emit destroyed(this);
deleteLater();
}
then make the destructor of Task private to ensure that all deletions must
be replaced with cals to destroy().
- Aaron
On 2010-04-06 16:49:20, Matthieu Gallien wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3505/
---
(Updated 2010-04-06 16:49:20)
Review request for Plasma, Aaron Seigo and Marco Martin.
Summary
---
Try to make dbus and fdo tasks have the same life cycle. Now the Manager
class is responsible for calling deleteLater. Make DBus tasks emits only once
the destroyed signal.
Diffs
-
/trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/core/manager.cpp
1110082
/trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/core/task.cpp
1110082
/trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/dbussystemtray/dbussystemtrayprotocol.cpp
1110298
/trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/fdo/fdoselectionmanager.cpp
1110082
/trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/fdo/fdotask.h
1110082
Diff: http://reviewboard.kde.org/r/3505/diff
Testing
---
Tested with dbus and fdo tasks inside a kde 4.4.2 session and Qt 4.6.2.
Nothing crash.
Thanks,
Matthieu
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Change the way dbus and fdo tasks are destroyed
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/3505/ --- (Updated 2010-04-06 19:45:35.640069) Review request for Plasma, Aaron Seigo and Marco Martin. Changes --- Implement the suggested change by Aaron. I had to make the destructor of Task protected in order to be able to call destructors of the derived classes. Summary --- Try to make dbus and fdo tasks have the same life cycle. Now the Manager class is responsible for calling deleteLater. Make DBus tasks emits only once the destroyed signal. Diffs (updated) - /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/core/task.cpp 824 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/dbussystemtray/dbussystemtrayprotocol.cpp 824 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/fdo/fdoselectionmanager.cpp 824 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/fdo/fdotask.h 824 Diff: http://reviewboard.kde.org/r/3505/diff Testing --- Tested with dbus and fdo tasks inside a kde 4.4.2 session and Qt 4.6.2. Nothing crash. Thanks, Matthieu ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Change the way dbus and fdo tasks are destroyed
On 2010-04-06 17:34:33, Aaron Seigo wrote:
/trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/dbussystemtray/dbussystemtrayprotocol.cpp,
line 92
http://reviewboard.kde.org/r/3505/diff/2/?file=22631#file22631line92
afaict from reading the code, this is the line that is incorrect. this
patch just works around that fact.
my guess is that this was done this way because deleteLater() schedules
the deletion for the next trip through the event loop.
what's probably needed here is a new method in Task that does this:
void Task::destroy()
{
emit destroyed(this);
deleteLater();
}
then make the destructor of Task private to ensure that all deletions
must be replaced with cals to destroy().
I did the changes you suggested but I do not really understand what you mean.
What was causing me troubles is that if I was waiting for the signal emission
fro the Task destructor, then I get crashes in the Manager::removeTask method.
Does your suggestion is referring to this ?
- Matthieu
---
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3505/#review4897
---
On 2010-04-06 19:45:35, Matthieu Gallien wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3505/
---
(Updated 2010-04-06 19:45:35)
Review request for Plasma, Aaron Seigo and Marco Martin.
Summary
---
Try to make dbus and fdo tasks have the same life cycle. Now the Manager
class is responsible for calling deleteLater. Make DBus tasks emits only once
the destroyed signal.
Diffs
-
/trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/core/task.cpp
824
/trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/dbussystemtray/dbussystemtrayprotocol.cpp
824
/trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/fdo/fdoselectionmanager.cpp
824
/trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/fdo/fdotask.h
824
Diff: http://reviewboard.kde.org/r/3505/diff
Testing
---
Tested with dbus and fdo tasks inside a kde 4.4.2 session and Qt 4.6.2.
Nothing crash.
Thanks,
Matthieu
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Change the way dbus and fdo tasks are destroyed
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/3505/ --- (Updated 2010-04-06 20:13:24.362369) Review request for Plasma, Aaron Seigo and Marco Martin. Changes --- More complete patch than the previous one. Change all the code including plasmoid protocol to use the new slot destroy in Task class. I tested the addition and removal of fdo visible and hidden tasks. I tested the addition and removal of dbus visible and hidden tasks. My testing does not trigger any problem. Summary --- Try to make dbus and fdo tasks have the same life cycle. Now the Manager class is responsible for calling deleteLater. Make DBus tasks emits only once the destroyed signal. Diffs (updated) - /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/core/task.h 824 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/core/task.cpp 824 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/core/task.cpp 824 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/dbussystemtray/dbussystemtrayprotocol.cpp 824 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/dbussystemtray/dbussystemtraytask.h 824 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/fdo/fdographicswidget.cpp 824 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/fdo/fdoselectionmanager.cpp 824 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/fdo/fdoselectionmanager.cpp 824 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/fdo/fdotask.h 824 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/fdo/fdotask.h 824 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/fdo/fdotask.cpp 824 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/plasmoid/plasmoidtask.h 824 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/plasmoid/plasmoidtask.cpp 824 /trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/protocols/plasmoid/plasmoidtaskprotocol.cpp 824 Diff: http://reviewboard.kde.org/r/3505/diff Testing --- Tested with dbus and fdo tasks inside a kde 4.4.2 session and Qt 4.6.2. Nothing crash. Thanks, Matthieu ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
