Port KAnimatedSystemTray into KSystemTray

[Ingo Klöcker]

On Saturday 13 September 2008, Aaron J. Seigo wrote:
> suggestion: reparent the movie to the tray icon
>
> rationale: instead of deleting the movie member directly, reparent it
> in setMovie: movie->setParent(this); not only does this make the
> memory management a bit easier (less code), this will prevent crashes
> where people do something like:
>
> new KSystemTrayIcon(new QMovie(pathToMovie, someObject), this);
>
> and then later delete someObject, causing the movie to be deleted as
> well which would then lead to a crash in KSystemTrayIcon

Wouldn't it make sense to use QPointer<QMovie> instead of QMovie* for 
the member variable? This would prevent a crash in KSystemTrayIcon if 
somebody does

  QMovie *movie = new QMovie(pathToMovie, someObject);
  new KSystemTrayIcon(movie, this);

and later

  delete movie;

Or do you think the note "Memory management for the movie will be 
handled by KAnimatedSystemTrayIcon." in the API docs of the c'tor is 
sufficient?

BTW, the API docs of setMovie() lack the note "Memory management for the 
movie will be handled by KAnimatedSystemTrayIcon."

Is copying QMovie expensive or is it implicitely shared? If it isn't 
expensive to copy a QMovie then I suggest to pass a const QMovie& to 
the c'tor and to setMovie() instead of a QMovie*. Apart from avoiding 
problems with memory management this would also make the signature of 
the QMovie-taking c'tor and of setMovie() equivalent to the signature 
of the c'tor taking a QIcon and of setIcon() (given this setter exists, 
that is).


Regards,
Ingo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 194 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080913/9526f801/attachment.sig>