Review Request 113066: Refactor Now playing plugin

[Xuetian Weng]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113066/
-----------------------------------------------------------

(Updated Oct. 2, 2013, 9:43 p.m.)


Review request for Telepathy.


Changes
-------

fixes issues in comment.

Actually I prefer to change the behavior since it's too easy to disable this in config in old version (any presence change in old contact list will disable it) so I guess actually no one have this option enabled.

And even only "Enable" is not that bad/confusing :P.


Bugs: 307582 and 313126
    http://bugs.kde.org/show_bug.cgi?id=307582
    http://bugs.kde.org/show_bug.cgi?id=313126


Repository: ktp-kded-module


Description
-------

This patch contains following changes:
1. disconnect for PropertiesChanges when knownPlayer is reset, the disappered service will not automatically disconnect from dbus.
2. change the "Enable" option to "Enable On Login" (bug 307582) and hence we can simplifies the logic (no more presenceActivated)
3. Changes now playing status at contact list will be remembered only for current login.
4. Fix the wrong behavior of handling PropertiesChanges (invalidate properties is not considered).
5. use requestedPresence to evaluate the current status (part of bug 313126)
6. since it changes to "Enable On Login", always enable the nowplaying text lineedit in kcm.

Current logic:
1. If the enabled option in config changes, set the plugin enable state (this must be caused by user change in kcm).
2. receive onActivate / onDeactivate from contact list, set the plugin enable state.
3. when enable state change to true, start detecting player. (activatePlugin(bool))
4. when enable state change to false, unwatch all player, reset all other state. (activatePlugin(bool))
5. when new player appears, request playbackStatus and metadata by GetAll, and then watch on the PropertiesChange for this player.

Some improvement:
1. not always listening on expensive serviceOwnerChanged.
2. be able to enable nowplaying on login.
3. fix leak connection of dbus signal.
4. code should now be clearer instead of having too much boolean check.


Diffs (updated)
-----

  config/telepathy-kded-config.cpp 953a637 
  telepathy-mpris.h e3378d4 
  telepathy-mpris.cpp 6cd3e20 

Diff: http://git.reviewboard.kde.org/r/113066/diff/


Testing
-------

Everything seems to work as expected.


Thanks,

Xuetian Weng

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20131002/660d6741/attachment.html>