Merge into trunk : session-reattach-nonfixed : Code : Media Hub

Status:	Merged
Approved by:	Jim Hodapp on 2015-04-21
Approved revision:	155
Merged at revision:	133
Proposed branch:	lp:~ricmm/media-hub/session-reattach-nonfixed
Merge into:	lp:media-hub
Prerequisite:	lp:~phablet-team/media-hub/tracklist-cli
Diff against target:	1135 lines (+555/-61) 19 files modified CMakeLists.txt (+1/-1) debian/changelog (+6/-0) include/core/media/player.h (+4/-0) include/core/media/service.h (+16/-0) src/core/media/mpris/service.h (+39/-0) src/core/media/player_configuration.h (+1/-1) src/core/media/player_implementation.cpp (+47/-17) src/core/media/player_implementation.h (+4/-0) src/core/media/player_stub.cpp (+22/-3) src/core/media/player_stub.h (+6/-1) src/core/media/server/server.cpp (+1/-1) src/core/media/service_implementation.cpp (+16/-0) src/core/media/service_implementation.h (+3/-0) src/core/media/service_skeleton.cpp (+235/-12) src/core/media/service_skeleton.h (+6/-0) src/core/media/service_stub.cpp (+39/-3) src/core/media/service_stub.h (+3/-0) tests/test-track-list/test_track_list.cpp (+99/-20) tests/test-track-list/test_track_list.h (+7/-2)
To merge this branch:	bzr merge lp:~ricmm/media-hub/session-reattach-nonfixed
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Approve on 2015-04-20
Thomas Voß (community)			Approve on 2015-04-17
Jim Hodapp (community)	code	2015-04-14	Approve on 2015-04-15
Review via email: mp+256101@code.launchpad.net

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-14:

#

FAILED: Continuous integration, rev:139
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~ricmm/media-hub/session-reattach-nonfixed/+merge/256101/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/media-hub-ci/325/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/165/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/165/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/165/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/325/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Jim Hodapp (jhodapp) wrote on 2015-04-14:

#

Looks good, a few things to fix.

review: Needs Fixing (code)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-14:

#

FAILED: Continuous integration, rev:139
http://jenkins.qa.ubuntu.com/job/media-hub-ci/327/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/167/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/167/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/167/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/327/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Jim Hodapp (jhodapp) wrote on 2015-04-15:

#

Looks good, thanks.

review: Approve (code)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-15:

#

FAILED: Continuous integration, rev:140
http://jenkins.qa.ubuntu.com/job/media-hub-ci/329/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/169/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/169/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/169/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/329/rebuild

review: Needs Fixing (continuous-integration)

lp:~ricmm/media-hub/session-reattach-nonfixed updated on 2015-04-15

141. By Ricardo Mendoza on 2015-04-15: Merge parent

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-15:

#

FAILED: Continuous integration, rev:141
http://jenkins.qa.ubuntu.com/job/media-hub-ci/332/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/172/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/172/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/172/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/332/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Thomas Voß (thomas-voss) wrote on 2015-04-16:

#

A few comments inline, one bigger question:

What is the difference between reattach/detach and the existing resume_session functionality?

review: Needs Fixing

Revision history for this message

Jim Hodapp (jhodapp) wrote on 2015-04-16:

#

Some added comments to my review and tvoss' review.

Revision history for this message

Thomas Voß (thomas-voss) wrote on 2015-04-16:

#

Replied inline.

Revision history for this message

Jim Hodapp (jhodapp) on 2015-04-16:

#

lp:~ricmm/media-hub/session-reattach-nonfixed updated on 2015-04-16

142. By Ricardo Mendoza on 2015-04-16: Address review comments and plug through no-impl methods of the Service parts

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-16:

#

PASSED: Continuous integration, rev:142
http://jenkins.qa.ubuntu.com/job/media-hub-ci/334/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/174
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/174
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/174/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/174

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/334/rebuild

review: Approve (continuous-integration)

lp:~ricmm/media-hub/session-reattach-nonfixed updated on 2015-04-16

143. By Ricardo Mendoza on 2015-04-16: Not a reference

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-16:

#

PASSED: Continuous integration, rev:143
http://jenkins.qa.ubuntu.com/job/media-hub-ci/335/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/175
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/175
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/175/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/175

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/335/rebuild

review: Approve (continuous-integration)

lp:~ricmm/media-hub/session-reattach-nonfixed updated on 2015-04-17

144. By Ricardo Mendoza on 2015-04-17: Correctly propagate track list changes from bus to local and viceversa

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-17:

#

PASSED: Continuous integration, rev:144
http://jenkins.qa.ubuntu.com/job/media-hub-ci/336/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/176
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/176
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/176/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/176

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/336/rebuild

review: Approve (continuous-integration)

Revision history for this message

Thomas Voß (thomas-voss) wrote on 2015-04-17:

#

Download full text (33.8 KiB)

On Thu, Apr 16, 2015 at 5:14 PM, Jim Hodapp <email address hidden> wrote:
>
>
> Diff comments:
>
>> === modified file 'CMakeLists.txt'
>> --- CMakeLists.txt 2015-01-23 13:00:36 +0000
>> +++ CMakeLists.txt 2015-04-15 18:29:47 +0000
>> @@ -3,7 +3,7 @@
>> project(ubuntu-media-hub)
>>
>> set(UBUNTU_MEDIA_HUB_VERSION_MAJOR 3)
>> -set(UBUNTU_MEDIA_HUB_VERSION_MINOR 0)
>> +set(UBUNTU_MEDIA_HUB_VERSION_MINOR 1)
>> set(UBUNTU_MEDIA_HUB_VERSION_PATCH 0)
>>
>> set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -fPIC -pthread")
>>
>> === modified file 'debian/changelog'
>> --- debian/changelog 2015-04-02 16:43:42 +0000
>> +++ debian/changelog 2015-04-15 18:29:47 +0000
>> @@ -1,3 +1,9 @@
>> +media-hub (3.1.0) UNRELEASED; urgency=medium
>> +
>> + * Support for signalling player reconnection up the stack.
>> +
>> + -- Ricardo Mendoza <email address hidden> Fri, 10 Apr 2015 18:20:32 +0200
>> +
>> media-hub (3.0.0+15.04.20150402-0ubuntu1) vivid; urgency=medium
>>
>> [ Jim Hodapp ]
>>
>> === modified file 'include/core/media/player.h'
>> --- include/core/media/player.h 2015-04-15 18:29:47 +0000
>> +++ include/core/media/player.h 2015-04-15 18:29:47 +0000
>> @@ -126,6 +126,9 @@
>> Player& operator=(const Player&) = delete;
>> bool operator==(const Player&) const = delete;
>>
>> + virtual std::string uuid() const = 0;
>
> Yeah I would say this isn't hugely important though as the likely-hood of this happening is very slim. Also, we could just change it if that unlikely event occurred.
>

That's somewhat difficult as this is public facing API. Your call.

>> + virtual void reconnect() = 0;
>> +
>> virtual std::shared_ptr<TrackList> track_list() = 0;
>> virtual PlayerKey key() const = 0;
>>
>>
>> === modified file 'include/core/media/service.h'
>> --- include/core/media/service.h 2014-09-30 06:51:15 +0000
>> +++ include/core/media/service.h 2015-04-15 18:29:47 +0000
>> @@ -42,9 +42,25 @@
>> Service& operator=(const Service&) = delete;
>> bool operator==(const Service&) const = delete;
>>
>> + /** @brief Creates a session with the media-hub service. */
>> virtual std::shared_ptr<Player> create_session(const Player::Configuration&) = 0;
>> +
>> + /** @brief Detaches a UUID-identified session for later resuming. */
>> + virtual void detach_session(const std::string& uuid, const Player::Configuration&) = 0;
>> +
>> + /** @brief Reattaches to a UUID-identified session that is in detached state. */
>> + virtual std::shared_ptr<Player> reattach_session(const std::string& uuid, const Player::Configuration&) = 0;
>> +
>> + /** @brief Asks the service to destroy a session. The session is destroyed when the client exits. */
>> + virtual void destroy_session(const std::string& uuid, const Player::Configuration&) = 0;
>> +
>> + /** @brief Creates a fixed-named session with the media-hub service. */
>> virtual std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&) = 0;
>> +
>> + /** @brief Resumes a fixed-name session directly by player key. */
>> virtual std::shared_ptr<Player> resume_session(Player::Play...

On Thu, Apr 16, 2015 at 5:14 PM, Jim Hodapp <jim.hodapp@canonical.com> wrote:
>
>
> Diff comments:
>
>> === modified file 'CMakeLists.txt'
>> --- CMakeLists.txt    2015-01-23 13:00:36 +0000
>> +++ CMakeLists.txt    2015-04-15 18:29:47 +0000
>> @@ -3,7 +3,7 @@
>>  project(ubuntu-media-hub)
>>
>>  set(UBUNTU_MEDIA_HUB_VERSION_MAJOR 3)
>> -set(UBUNTU_MEDIA_HUB_VERSION_MINOR 0)
>> +set(UBUNTU_MEDIA_HUB_VERSION_MINOR 1)
>>  set(UBUNTU_MEDIA_HUB_VERSION_PATCH 0)
>>
>>  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -fPIC -pthread")
>>
>> === modified file 'debian/changelog'
>> --- debian/changelog  2015-04-02 16:43:42 +0000
>> +++ debian/changelog  2015-04-15 18:29:47 +0000
>> @@ -1,3 +1,9 @@
>> +media-hub (3.1.0) UNRELEASED; urgency=medium
>> +
>> +  * Support for signalling player reconnection up the stack.
>> +
>> + -- Ricardo Mendoza <ricardo.mendoza@canonical.com>  Fri, 10 Apr 2015 18:20:32 +0200
>> +
>>  media-hub (3.0.0+15.04.20150402-0ubuntu1) vivid; urgency=medium
>>
>>    [ Jim Hodapp ]
>>
>> === modified file 'include/core/media/player.h'
>> --- include/core/media/player.h       2015-04-15 18:29:47 +0000
>> +++ include/core/media/player.h       2015-04-15 18:29:47 +0000
>> @@ -126,6 +126,9 @@
>>      Player& operator=(const Player&) = delete;
>>      bool operator==(const Player&) const = delete;
>>
>> +    virtual std::string uuid() const = 0;
>
> Yeah I would say this isn't hugely important though as the likely-hood of this happening is very slim. Also, we could just change it if that unlikely event occurred.
>

That's somewhat difficult as this is public facing API. Your call.

>> +    virtual void reconnect() = 0;
>> +
>>      virtual std::shared_ptr<TrackList> track_list() = 0;
>>      virtual PlayerKey key() const = 0;
>>
>>
>> === modified file 'include/core/media/service.h'
>> --- include/core/media/service.h      2014-09-30 06:51:15 +0000
>> +++ include/core/media/service.h      2015-04-15 18:29:47 +0000
>> @@ -42,9 +42,25 @@
>>      Service& operator=(const Service&) = delete;
>>      bool operator==(const Service&) const = delete;
>>
>> +    /** @brief Creates a session with the media-hub service. */
>>      virtual std::shared_ptr<Player> create_session(const Player::Configuration&) = 0;
>> +
>> +    /** @brief Detaches a UUID-identified session for later resuming. */
>> +    virtual void detach_session(const std::string& uuid, const Player::Configuration&) = 0;
>> +
>> +    /** @brief Reattaches to a UUID-identified session that is in detached state. */
>> +    virtual std::shared_ptr<Player> reattach_session(const std::string& uuid, const Player::Configuration&) = 0;
>> +
>> +    /** @brief Asks the service to destroy a session. The session is destroyed when the client exits. */
>> +    virtual void destroy_session(const std::string& uuid, const Player::Configuration&) = 0;
>> +
>> +    /** @brief Creates a fixed-named session with the media-hub service. */
>>      virtual std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&) = 0;
>> +
>> +    /** @brief Resumes a fixed-name session directly by player key. */
>>      virtual std::shared_ptr<Player> resume_session(Player::PlayerKey) = 0;
>> +
>> +    /** @brief Pauses sessions other than the supplied one. */
>>      virtual void pause_other_sessions(Player::PlayerKey) = 0;
>>
>>    protected:
>>
>> === modified file 'src/core/media/mpris/service.h'
>> --- src/core/media/mpris/service.h    2014-09-30 06:51:15 +0000
>> +++ src/core/media/mpris/service.h    2015-04-15 18:29:47 +0000
>> @@ -48,6 +48,42 @@
>>              }
>>          };
>>
>> +        struct DetachingSession
>> +        {
>> +            static const std::string& name()
>> +            {
>> +                static const std::string s
>> +                {
>> +                    "core.ubuntu.media.Service.Error.DetachingSession"
>> +                };
>> +                return s;
>> +            }
>> +        };
>> +
>> +        struct ReattachingSession
>> +        {
>> +            static const std::string& name()
>> +            {
>> +                static const std::string s
>> +                {
>> +                    "core.ubuntu.media.Service.Error.ReattachingSession"
>> +                };
>> +                return s;
>> +            }
>> +        };
>> +
>> +        struct DestroyingSession
>> +        {
>> +            static const std::string& name()
>> +            {
>> +                static const std::string s
>> +                {
>> +                    "core.ubuntu.media.Service.Error.DestroyingSession"
>> +                };
>> +                return s;
>> +            }
>> +        };
>> +
>>          struct CreatingFixedSession
>>          {
>>              static const std::string& name()
>> @@ -74,6 +110,9 @@
>>      };
>>
>>      DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(CreateSession, Service, 1000)
>> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(DetachSession, Service, 1000)
>> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(ReattachSession, Service, 1000)
>> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(DestroySession, Service, 1000)
>>      DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(CreateFixedSession, Service, 1000)
>>      DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(ResumeSession, Service, 1000)
>>      DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(PauseOtherSessions, Service, 1000)
>>
>> === modified file 'src/core/media/player_configuration.h'
>> --- src/core/media/player_configuration.h     2014-11-26 16:23:19 +0000
>> +++ src/core/media/player_configuration.h     2015-04-15 18:29:47 +0000
>> @@ -27,7 +27,7 @@
>>  // to the implementation in a way that is opaque to the client.
>>  struct core::ubuntu::media::Player::Configuration
>>  {
>> -    // Unique key for identifying the session.
>> +    // Unique key for identifying the session path.
>>      core::ubuntu::media::Player::PlayerKey key;
>>      // The bus connection to expose objects on.
>>      std::shared_ptr<core::dbus::Bus> bus;
>>
>> === modified file 'src/core/media/player_implementation.cpp'
>> --- src/core/media/player_implementation.cpp  2015-04-15 18:29:47 +0000
>> +++ src/core/media/player_implementation.cpp  2015-04-15 18:29:47 +0000
>> @@ -512,6 +512,18 @@
>>  }
>>
>>  template<typename Parent>
>> +std::string media::PlayerImplementation<Parent>::uuid() const
>> +{
>> +    // No impl for now, as not needed internally.
>> +}
>> +
>> +template<typename Parent>
>> +void media::PlayerImplementation<Parent>::reconnect()
>> +{
>> +    d->config.client_death_observer->register_for_death_notifications_with_key(d->config.key);
>> +}
>> +
>> +template<typename Parent>
>>  std::shared_ptr<media::TrackList> media::PlayerImplementation<Parent>::track_list()
>>  {
>>      return d->track_list;
>>
>> === modified file 'src/core/media/player_implementation.h'
>> --- src/core/media/player_implementation.h    2015-04-15 18:29:47 +0000
>> +++ src/core/media/player_implementation.h    2015-04-15 18:29:47 +0000
>> @@ -55,6 +55,9 @@
>>      PlayerImplementation(const Configuration& configuration);
>>      ~PlayerImplementation();
>>
>> +    virtual std::string uuid() const;
>> +    virtual void reconnect();
>> +
>>      virtual std::shared_ptr<TrackList> track_list();
>>      virtual Player::PlayerKey key() const;
>>
>>
>> === modified file 'src/core/media/player_stub.cpp'
>> --- src/core/media/player_stub.cpp    2015-04-15 18:29:47 +0000
>> +++ src/core/media/player_stub.cpp    2015-04-15 18:29:47 +0000
>> @@ -43,10 +43,12 @@
>>  struct media::PlayerStub::Private
>>  {
>>      Private(const std::shared_ptr<Service>& parent,
>> -            const std::shared_ptr<core::dbus::Object>& object
>> +            const std::shared_ptr<core::dbus::Object>& object,
>> +            const std::string uuid
>>              ) : parent(parent),
>>                  object(object),
>>                  key(object->invoke_method_synchronously<mpris::Player::Key, media::Player::PlayerKey>().value()),
>> +                uuid(uuid),
>>                  sink_factory(media::video::make_platform_default_sink_factory(key)),
>>                  properties
>>                  {
>> @@ -93,6 +95,7 @@
>>      std::shared_ptr<TrackList> track_list;
>>      dbus::Object::Ptr object;
>>      media::Player::PlayerKey key;
>> +    std::string uuid;
>>      media::video::SinkFactory sink_factory;
>>      struct
>>      {
>> @@ -209,8 +212,9 @@
>>
>>  media::PlayerStub::PlayerStub(
>>      const std::shared_ptr<Service>& parent,
>> -    const std::shared_ptr<core::dbus::Object>& object)
>> -        : d(new Private{parent, object})
>> +    const std::shared_ptr<core::dbus::Object>& object,
>> +    std::string uuid)
>> +        : d(new Private{parent, object, uuid})
>>  {
>>  }
>>
>> @@ -218,6 +222,16 @@
>>  {
>>  }
>>
>> +std::string media::PlayerStub::uuid() const
>> +{
>> +    return d->uuid;
>> +}
>> +
>> +void media::PlayerStub::reconnect()
>> +{
>> +    // No implementation
>> +}
>> +
>>  std::shared_ptr<media::TrackList> media::PlayerStub::track_list()
>>  {
>>      if (!d->track_list)
>>
>> === modified file 'src/core/media/player_stub.h'
>> --- src/core/media/player_stub.h      2015-04-15 18:29:47 +0000
>> +++ src/core/media/player_stub.h      2015-04-15 18:29:47 +0000
>> @@ -39,10 +39,14 @@
>>    public:
>>      explicit PlayerStub(
>>          const std::shared_ptr<Service>& parent,
>> -        const std::shared_ptr<core::dbus::Object>& object);
>> +        const std::shared_ptr<core::dbus::Object>& object,
>> +        std::string uuid = "");
>
> Indeed.
>
>>
>>      ~PlayerStub();
>>
>> +    virtual std::string uuid() const;
>> +    virtual void reconnect();
>> +
>>      virtual std::shared_ptr<TrackList> track_list();
>>      virtual PlayerKey key() const;
>>
>>
>> === modified file 'src/core/media/server/server.cpp'
>> --- src/core/media/server/server.cpp  2014-12-15 14:43:44 +0000
>> +++ src/core/media/server/server.cpp  2015-04-15 18:29:47 +0000
>> @@ -123,7 +123,7 @@
>>      {
>>          impl,
>>          player_store,
>> -
>> +        external_services
>>      });
>>
>>      std::thread service_worker
>>
>> === modified file 'src/core/media/service_skeleton.cpp'
>> --- src/core/media/service_skeleton.cpp       2015-04-15 18:29:47 +0000
>> +++ src/core/media/service_skeleton.cpp       2015-04-15 18:29:47 +0000
>> @@ -35,6 +35,10 @@
>>
>>  #include <core/posix/this_process.h>
>>
>> +#include <boost/uuid/uuid.hpp>
>> +#include <boost/uuid/uuid_generators.hpp>
>> +#include <boost/uuid/uuid_io.hpp>
>> +
>>  #include <map>
>>  #include <regex>
>>  #include <sstream>
>> @@ -50,17 +54,33 @@
>>  struct media::ServiceSkeleton::Private
>>  {
>>      Private(media::ServiceSkeleton* impl, const ServiceSkeleton::Configuration& config)
>> -        : impl(impl),
>> +        : request_context_resolver(media::apparmor::ubuntu::make_platform_default_request_context_resolver(config.external_services)),
>> +          impl(impl),
>>            object(impl->access_service()->add_object_for_path(
>>                       dbus::traits::Service<media::Service>::object_path())),
>> -          exported(impl->access_bus(), config.cover_art_resolver),
>> -          configuration(config)
>> +          configuration(config),
>> +          exported(impl->access_bus(), config.cover_art_resolver)
>>      {
>>          object->install_method_handler<mpris::Service::CreateSession>(
>>                      std::bind(
>>                          &Private::handle_create_session,
>>                          this,
>>                          std::placeholders::_1));
>> +        object->install_method_handler<mpris::Service::DetachSession>(
>> +                    std::bind(
>> +                        &Private::handle_detach_session,
>> +                        this,
>> +                        std::placeholders::_1));
>> +        object->install_method_handler<mpris::Service::ReattachSession>(
>> +                    std::bind(
>> +                        &Private::handle_reattach_session,
>> +                        this,
>> +                        std::placeholders::_1));
>> +        object->install_method_handler<mpris::Service::DestroySession>(
>> +                    std::bind(
>> +                        &Private::handle_destroy_session,
>> +                        this,
>> +                        std::placeholders::_1));
>>          object->install_method_handler<mpris::Service::CreateFixedSession>(
>>                      std::bind(
>>                          &Private::handle_create_fixed_session,
>> @@ -78,24 +98,26 @@
>>                          std::placeholders::_1));
>>      }
>>
>> -    std::pair<std::string, media::Player::PlayerKey> create_session_info()
>> +    std::tuple<std::string, media::Player::PlayerKey, std::string> create_session_info()
>>      {
>>          static unsigned int session_counter = 0;
>>
>>          unsigned int current_session = session_counter++;
>> +        boost::uuids::uuid uuid = gen();
>
> Right, it's not available directly, but uuid() is, which in my opinion would be good enough. And I don't think we need to mock the generator as it's available anywhere we need it and can safely rely on it being safe.
>

Again, your call. It's less a question if the generator is safe, but
more a question if it is actually called.

>>
>>          std::stringstream ss;
>>          ss << "/core/ubuntu/media/Service/sessions/" << current_session;
>>
>> -        return std::make_pair(ss.str(), media::Player::PlayerKey(current_session));
>> +        return std::make_tuple(ss.str(), media::Player::PlayerKey(current_session), to_string(uuid));
>>      }
>>
>>      void handle_create_session(const core::dbus::Message::Ptr& msg)
>>      {
>> -        auto  session_info = create_session_info();
>> +        auto session_info = create_session_info();
>>
>> -        dbus::types::ObjectPath op{session_info.first};
>> -        media::Player::PlayerKey key{session_info.second};
>> +        dbus::types::ObjectPath op{std::get<0>(session_info)};
>> +        media::Player::PlayerKey key{std::get<1>(session_info)};
>> +        std::string uuid{std::get<2>(session_info)};
>>
>>          media::Player::Configuration config
>>          {
>> @@ -104,11 +126,21 @@
>>              impl->access_service()->add_object_for_path(op)
>>          };
>>
>> +        std::cout << "Session created by request of: " << msg->sender() << ", uuid: " << uuid << ", path:" << op << std::endl;
>> +
>>          try
>>          {
>>              configuration.player_store->add_player_for_key(key, impl->create_session(config));
>> +            uuid_player_map.insert(std::make_pair(uuid, key));
>> +
>> +            request_context_resolver->resolve_context_for_dbus_name_async(msg->sender(), [this, key, msg](const media::apparmor::ubuntu::Context& context)
>> +            {
>> +                fprintf(stderr, "%s():%d -- app_name='%s', attached\n", __func__, __LINE__, context.str().c_str());
>> +                player_owner_map.insert(std::make_pair(key, std::make_tuple(context.str(), true, msg->sender())));
>> +            });
>> +
>>              auto reply = dbus::Message::make_method_return(msg);
>> -            reply->writer() << op;
>> +            reply->writer() << std::make_tuple(op, uuid);
>>
>>              impl->access_bus()->send(reply);
>>          } catch(const std::runtime_error& e)
>> @@ -121,6 +153,171 @@
>>          }
>>      }
>>
>> +    void handle_detach_session(const core::dbus::Message::Ptr& msg)
>> +    {
>> +        try
>> +        {
>> +            std::string uuid;
>> +            msg->reader() >> uuid;
>> +
>> +            auto key = uuid_player_map.at(uuid);
>> +
>> +            if (player_owner_map.count(key) != 0) {
>> +                auto info = player_owner_map.at(key);
>> +                // Check if session is attached(1) and that the detachment
>> +                // request comes from the same peer(2) that created the session.
>> +                if (std::get<1>(info) && (std::get<2>(info) == msg->sender())) { // Player is attached
>> +                    std::get<1>(info) = false; // Detached
>> +                    std::get<2>(info).clear(); // Clear registered sender/peer
>> +                    auto player = configuration.player_store->player_for_key(key);
>> +                    player->lifetime().set(media::Player::Lifetime::resumable);
>> +                }
>> +            }
>> +
>> +            auto reply = dbus::Message::make_method_return(msg);
>> +            impl->access_bus()->send(reply);
>> +
>> +        } catch(const std::runtime_error& e)
>> +        {
>> +            auto reply = dbus::Message::make_error(
>> +                        msg,
>> +                        mpris::Service::Errors::DetachingSession::name(),
>> +                        e.what());
>> +            impl->access_bus()->send(reply);
>> +        }
>> +    }
>> +
>> +    void handle_reattach_session(const core::dbus::Message::Ptr& msg)
>> +    {
>> +        try
>> +        {
>> +            std::string uuid;
>> +            msg->reader() >> uuid;
>> +
>> +            if (uuid_player_map.count(uuid) != 0) {
>> +                auto key = uuid_player_map.at(uuid);
>> +                if (not configuration.player_store->has_player_for_key(key)) {
>> +                    auto reply = dbus::Message::make_error(
>> +                                msg,
>> +                                mpris::Service::Errors::ReattachingSession::name(),
>> +                                "Unable to locate player session");
>> +                    impl->access_bus()->send(reply);
>> +                    return;
>> +                }
>> +                std::stringstream ss;
>> +                ss << "/core/ubuntu/media/Service/sessions/" << key;
>> +                dbus::types::ObjectPath op{ss.str()};
>> +
>> +                request_context_resolver->resolve_context_for_dbus_name_async(msg->sender(), [this, msg, key, op](const media::apparmor::ubuntu::Context& context)
>> +                {
>> +                    auto info = player_owner_map.at(key);
>> +                    fprintf(stderr, "%s():%d -- reattach app_name='%s', info='%s', '%s'\n", __func__, __LINE__, context.str().c_str(), std::get<0>(info).c_str(), std::get<2>(info).c_str());
>> +                    if (std::get<0>(info) == context.str()) {
>> +                        std::get<1>(info) = true; // Set to Attached
>> +                        std::get<2>(info) = msg->sender(); // Register new owner
>> +
>> +                        // Signal player reconnection
>> +                        auto player = configuration.player_store->player_for_key(key);
>> +                        player->reconnect();
>> +
>> +                        auto reply = dbus::Message::make_method_return(msg);
>> +                        reply->writer() << op;
>> +
>> +                        impl->access_bus()->send(reply);
>> +                    }
>> +                    else {
>> +                        auto reply = dbus::Message::make_error(
>> +                                    msg,
>> +                                    mpris::Service::Errors::ReattachingSession::name(),
>> +                                    "Invalid permissions for the requested session");
>> +                        impl->access_bus()->send(reply);
>> +                        return;
>> +                    }
>> +                });
>> +            }
>> +            else {
>> +               auto reply = dbus::Message::make_error(
>> +                           msg,
>> +                           mpris::Service::Errors::ReattachingSession::name(),
>> +                           "Invalid session");
>> +               impl->access_bus()->send(reply);
>> +               return;
>> +            }
>> +        } catch(const std::runtime_error& e)
>> +        {
>> +            auto reply = dbus::Message::make_error(
>> +                        msg,
>> +                        mpris::Service::Errors::ReattachingSession::name(),
>> +                        e.what());
>> +            impl->access_bus()->send(reply);
>> +        }
>> +    }
>> +
>> +    void handle_destroy_session(const core::dbus::Message::Ptr& msg)
>> +    {
>> +
>> +        try
>> +        {
>> +            std::string uuid;
>> +            msg->reader() >> uuid;
>> +
>> +            if (uuid_player_map.count(uuid) != 0) {
>> +                auto key = uuid_player_map.at(uuid);
>> +                if (not configuration.player_store->has_player_for_key(key)) {
>> +                    auto reply = dbus::Message::make_error(
>> +                                msg,
>> +                                mpris::Service::Errors::DestroyingSession::name(),
>> +                                "Unable to locate player session");
>> +                    impl->access_bus()->send(reply);
>> +                    return;
>> +                }
>> +
>> +                // Remove control entries from the map, at this point
>> +                // the session is no longer usable.
>> +                uuid_player_map.erase(uuid);
>> +
>> +                request_context_resolver->resolve_context_for_dbus_name_async(msg->sender(), [this, msg, key](const media::apparmor::ubuntu::Context& context)
>> +                {
>> +                    auto info = player_owner_map.at(key);
>> +                    fprintf(stderr, "%s():%d -- Destroying app_name='%s', info='%s', '%s'\n", __func__, __LINE__, context.str().c_str(), std::get<0>(info).c_str(), std::get<2>(info).c_str());
>> +                    if (std::get<0>(info) == context.str()) {
>> +                        player_owner_map.erase(key);
>> +
>> +                        // Reset lifecycle to non-resumable on the now-abandoned session
>> +                        auto player = configuration.player_store->player_for_key(key);
>> +                        player->lifetime().set(media::Player::Lifetime::normal);
>> +
>> +                        auto reply = dbus::Message::make_method_return(msg);
>> +                        impl->access_bus()->send(reply);
>> +                    }
>> +                    else {
>> +                        auto reply = dbus::Message::make_error(
>> +                                    msg,
>> +                                    mpris::Service::Errors::DestroyingSession::name(),
>> +                                    "Invalid permissions for the requested session");
>> +                        impl->access_bus()->send(reply);
>> +                        return;
>> +                    }
>> +                });
>> +            }
>> +            else {
>> +               auto reply = dbus::Message::make_error(
>> +                           msg,
>> +                           mpris::Service::Errors::DestroyingSession::name(),
>> +                           "Invalid session");
>> +               impl->access_bus()->send(reply);
>> +               return;
>> +            }
>> +        } catch(const std::runtime_error& e)
>> +        {
>> +            auto reply = dbus::Message::make_error(
>> +                        msg,
>> +                        mpris::Service::Errors::DestroyingSession::name(),
>> +                        e.what());
>> +            impl->access_bus()->send(reply);
>> +        }
>> +    }
>> +
>>      void handle_create_fixed_session(const core::dbus::Message::Ptr& msg)
>>      {
>>          try
>> @@ -132,8 +329,8 @@
>>                  // Create new session
>>                  auto  session_info = create_session_info();
>>
>> -                dbus::types::ObjectPath op{session_info.first};
>> -                media::Player::PlayerKey key{session_info.second};
>> +                dbus::types::ObjectPath op{std::get<0>(session_info)};
>> +                media::Player::PlayerKey key{std::get<1>(session_info)};
>>
>>                  media::Player::Configuration config
>>                  {
>> @@ -147,7 +344,6 @@
>>
>>                  configuration.player_store->add_player_for_key(key, session);
>>
>> -
>>                  named_player_map.insert(std::make_pair(name, key));
>>
>>                  auto reply = dbus::Message::make_method_return(msg);
>> @@ -231,6 +427,7 @@
>>          impl->access_bus()->send(reply);
>>      }
>>
>> +    media::apparmor::ubuntu::RequestContextResolver::Ptr request_context_resolver;
>>      media::ServiceSkeleton* impl;
>>      dbus::Object::Ptr object;
>>
>> @@ -238,6 +435,14 @@
>>      ServiceSkeleton::Configuration configuration;
>>      // We map named/fixed player instances to their respective keys.
>>      std::map<std::string, media::Player::PlayerKey> named_player_map;
>> +    // We map UUIDs to their respective keys.
>> +    std::map<std::string, media::Player::PlayerKey> uuid_player_map;
>> +    // We keep a list of keys and their respective owners and states.
>> +    // value: (owner context, attached state, attached dbus name)
>> +    std::map<media::Player::PlayerKey, std::tuple<std::string, bool, std::string>> player_owner_map;
>> +
>> +    boost::uuids::random_generator gen;
>> +
>>      // We expose the entire service as an MPRIS player.
>>      struct Exported
>>      {
>>
>> === modified file 'src/core/media/service_skeleton.h'
>> --- src/core/media/service_skeleton.h 2014-12-15 14:43:44 +0000
>> +++ src/core/media/service_skeleton.h 2015-04-15 18:29:47 +0000
>> @@ -19,6 +19,8 @@
>>  #ifndef CORE_UBUNTU_MEDIA_SERVICE_SKELETON_H_
>>  #define CORE_UBUNTU_MEDIA_SERVICE_SKELETON_H_
>>
>> +#include "apparmor/ubuntu.h"
>> +
>>  #include <core/media/service.h>
>>
>>  #include "cover_art_resolver.h"
>> @@ -43,6 +45,7 @@
>>      {
>>          std::shared_ptr<Service> impl;
>>          KeyedPlayerStore::Ptr player_store;
>> +        helper::ExternalServices& external_services;
>>          CoverArtResolver cover_art_resolver;
>>      };
>>
>>
>> === modified file 'src/core/media/service_stub.cpp'
>> --- src/core/media/service_stub.cpp   2014-10-23 14:42:59 +0000
>> +++ src/core/media/service_stub.cpp   2015-04-15 18:29:47 +0000
>> @@ -58,7 +58,7 @@
>>  std::shared_ptr<media::Player> media::ServiceStub::create_session(const media::Player::Configuration&)
>>  {
>>      auto op = d->object->invoke_method_synchronously<mpris::Service::CreateSession,
>> -         dbus::types::ObjectPath>();
>> +         std::tuple<dbus::types::ObjectPath, std::string>>();
>>
>>      if (op.is_error())
>>          throw std::runtime_error("Problem creating session: " + op.error());
>> @@ -66,8 +66,43 @@
>>      return std::shared_ptr<media::Player>(new media::PlayerStub
>>      {
>>          shared_from_this(),
>> -        access_service()->object_for_path(op.value())
>> -    });
>> +        access_service()->object_for_path(std::get<0>(op.value())),
>> +        std::get<1>(op.value())
>> +    });
>> +}
>> +
>> +void media::ServiceStub::detach_session(const std::string& uuid, const media::Player::Configuration&)
>> +{
>> +    auto op = d->object->invoke_method_synchronously<mpris::Service::DetachSession,
>> +         void>(uuid);
>> +
>> +    if (op.is_error())
>> +        throw std::runtime_error("Problem detaching session: " + op.error());
>> +}
>> +
>> +std::shared_ptr<media::Player> media::ServiceStub::reattach_session(const std::string& uuid, const media::Player::Configuration&)
>> +{
>> +    auto op = d->object->invoke_method_synchronously<mpris::Service::ReattachSession,
>> +         dbus::types::ObjectPath>(uuid);
>> +
>> +    if (op.is_error())
>> +        throw std::runtime_error("Problem reattaching session: " + op.error());
>> +
>> +    return std::shared_ptr<media::Player>(new media::PlayerStub
>> +    {
>> +        shared_from_this(),
>> +        access_service()->object_for_path(op.value()),
>> +        uuid
>> +    });
>> +}
>> +
>> +void media::ServiceStub::destroy_session(const std::string& uuid, const media::Player::Configuration&)
>> +{
>> +    auto op = d->object->invoke_method_synchronously<mpris::Service::DestroySession,
>> +         void>(uuid);
>> +
>> +    if (op.is_error())
>> +        throw std::runtime_error("Problem destroying session: " + op.error());
>>  }
>>
>>  std::shared_ptr<media::Player> media::ServiceStub::create_fixed_session(const std::string& name, const media::Player::Configuration&)
>>
>> === modified file 'src/core/media/service_stub.h'
>> --- src/core/media/service_stub.h     2014-10-23 14:42:59 +0000
>> +++ src/core/media/service_stub.h     2015-04-15 18:29:47 +0000
>> @@ -40,6 +40,9 @@
>>      ~ServiceStub();
>>
>>      std::shared_ptr<Player> create_session(const Player::Configuration&);
>> +    void detach_session(const std::string& uuid, const Player::Configuration&);
>> +    std::shared_ptr<Player> reattach_session(const std::string& uuid, const Player::Configuration&);
>> +    void destroy_session(const std::string& uuid, const Player::Configuration&);
>>      std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&);
>>      std::shared_ptr<Player> resume_session(Player::PlayerKey key);
>>      void pause_other_sessions(Player::PlayerKey key);
>>
>> === modified file 'tests/test-track-list/test_track_list.cpp'
>> --- tests/test-track-list/test_track_list.cpp 2015-04-15 18:29:47 +0000
>> +++ tests/test-track-list/test_track_list.cpp 2015-04-15 18:29:47 +0000
>> @@ -44,7 +44,7 @@
>>  {
>>  }
>>
>> -void media::TestTrackList::create_new_player_session()
>> +std::string media::TestTrackList::create_new_player_session()
>>  {
>>      try {
>>          m_hubPlayerSession = m_hubService->create_session(media::Player::Client::default_configuration());
>> @@ -59,9 +59,51 @@
>>      catch (std::runtime_error &e) {
>>          cerr << "FATAL: Failed to retrieve the current player's TrackList: " << e.what() << endl;
>>      }
>> -}
>> -
>> -void media::TestTrackList::destroy_player_session()
>> +
>> +    std::string uuid;
>> +    try {
>> +        uuid.assign(m_hubPlayerSession->uuid());
>> +    }
>> +    catch (std::runtime_error &e) {
>> +        cerr << "FATAL: Failed to retrieve the current player's uuid: " << e.what() << endl;
>> +    }
>> +
>> +    cout << "Connected to session " << uuid << endl;
>> +    return uuid;
>> +}
>> +
>> +void media::TestTrackList::detach_player_session(const std::string &uuid)
>> +{
>> +    try {
>> +        m_hubService->detach_session(uuid, media::Player::Client::default_configuration());
>> +    }
>> +    catch (std::runtime_error &e) {
>> +        cerr << "FATAL: Failed to detach the media-hub player session: " << e.what() << endl;
>> +    }
>> +
>> +    cout << "Detached session " << uuid << endl;
>> +}
>> +
>> +void media::TestTrackList::reattach_player_session(const std::string &uuid)
>> +{
>> +    try {
>> +        m_hubPlayerSession = m_hubService->reattach_session(uuid, media::Player::Client::default_configuration());
>> +    }
>> +    catch (std::runtime_error &e) {
>> +        cerr << "FATAL: Failed to reattach the media-hub player session: " << e.what() << endl;
>> +    }
>> +
>> +    try {
>> +        m_hubTrackList = m_hubPlayerSession->track_list();
>> +    }
>> +    catch (std::runtime_error &e) {
>> +        cerr << "FATAL: Failed to retrieve the current player's TrackList: " << e.what() << endl;
>> +    }
>> +
>> +    cout << "Re-connected to session " << uuid << endl;
>> +}
>> +
>> +void media::TestTrackList::destroy_player_session(const std::string &uuid)
>>  {
>>      // TODO: explicitly add a destroy session to the Service class after ricmm lands his new creation_session
>>      // that returns a session ID. This will allow me to clear the tracklist after each test.
>> @@ -121,8 +163,28 @@
>>      //destroy_player_session();
>>  }
>>
>> +void media::TestTrackList::test_tracklist_resume(const std::string &uri1, const std::string &uri2, const std::string &uuid)
>> +{
>> +    cout << "--> Running test: test_tracklist_resume" << std::endl;
>> +
>> +    add_track(uri1);
>> +    add_track(uri2);
>> +
>> +    int initial_size = m_hubTrackList->tracks().get().size();
>> +    cout << "Tracklist size: " << initial_size << endl;
>> +    detach_player_session(uuid);
>> +    reattach_player_session(uuid);
>> +    cout << "Tracklist size: " << m_hubTrackList->tracks().get().size() << endl;
>> +
>> +    m_hubPlayerSession->play();
>> +
>> +    if (initial_size != m_hubTrackList->tracks().get().size())
>> +        cout << "Tracklist sizes are different, error in resuming" << endl;
>> +}
>> +
>>  void media::TestTrackList::test_ensure_tracklist_is_not_empty(const std::string &uri1, const std::string &uri2)
>>  {
>> +    //create_new_player_session();
>>      cout << "--> Running test: test_ensure_tracklist_is_not_empty" << std::endl;
>>
>>      add_track(uri1);
>> @@ -335,6 +397,11 @@
>>          tracklist->test_shuffle(argv[1], argv[2], argv[3]);
>>          tracklist->test_remove_track(argv[1], argv[2], argv[3]);
>>      }
>> +    else if (argc == 5)
>> +    {
>> +        std::string uuid = tracklist->create_new_player_session();
>> +        tracklist->test_tracklist_resume(argv[1], argv[2], uuid);
>> +    }
>>      else
>>      {
>>          cout << "Can't test TrackList, no track(s) specified." << endl;
>>
>> === modified file 'tests/test-track-list/test_track_list.h'
>> --- tests/test-track-list/test_track_list.h   2015-04-15 18:29:47 +0000
>> +++ tests/test-track-list/test_track_list.h   2015-04-15 18:29:47 +0000
>> @@ -45,14 +45,19 @@
>>      TestTrackList();
>>      ~TestTrackList();
>>
>> -    void create_new_player_session();
>> -    void destroy_player_session();
>> +    std::string create_new_player_session();
>> +    void detach_player_session(const std::string &uuid);
>> +    void reattach_player_session(const std::string &uuid);
>> +    void destroy_player_session(const std::string &uuid);
>>
>>      void add_track(const std::string &uri, bool make_current = false);
>>
>>      // Takes in one or two files for playback, adds it/them to the TrackList, and plays
>>      void test_basic_playback(const std::string &uri1, const std::string &uri2 = std::string{});
>>
>> +    // Takes two uris and confirms that they remain after a detach/reattach
>> +    void test_tracklist_resume(const std::string &uri1, const std::string &uri2, const std::string &uuid);
>> +
>>      void test_ensure_tracklist_is_not_empty(const std::string &uri1, const std::string &uri2 = std::string{});
>>
>>      // Takes in one or two files for playback, adds it/them to the TrackList, plays and makes sure
>>
>
>
> --
> https://code.launchpad.net/~ricmm/media-hub/session-reattach-nonfixed/+merge/256101
> You are reviewing the proposed merge of lp:~ricmm/media-hub/session-reattach-nonfixed into lp:media-hub.

lp:~ricmm/media-hub/session-reattach-nonfixed updated on 2015-04-17

145. By Ricardo Mendoza on 2015-04-17: Remove printfs

Revision history for this message

Thomas Voß (thomas-voss) wrote on 2015-04-17:

#

LGTM.

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-17:

#

PASSED: Continuous integration, rev:145
http://jenkins.qa.ubuntu.com/job/media-hub-ci/338/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/178
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/178
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/178/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/178

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/338/rebuild

review: Approve (continuous-integration)

lp:~ricmm/media-hub/session-reattach-nonfixed updated on 2015-04-17

146. By Ricardo Mendoza on 2015-04-17: Revert last changes
147. By Ricardo Mendoza on 2015-04-17: Delete player instance instead of abandoning
148. By Ricardo Mendoza on 2015-04-17: Update tracklist cli for new api
149. By Ricardo Mendoza on 2015-04-17: Merge parent

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-17:

#

FAILED: Continuous integration, rev:149
http://jenkins.qa.ubuntu.com/job/media-hub-ci/342/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/182/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/182/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/182/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/342/rebuild

review: Needs Fixing (continuous-integration)

lp:~ricmm/media-hub/session-reattach-nonfixed updated on 2015-04-17

150. By Ricardo Mendoza on 2015-04-17: Implement full player destruction
151. By Ricardo Mendoza on 2015-04-17: Merge parent
152. By Ricardo Mendoza on 2015-04-17: Syntax error
153. By Ricardo Mendoza on 2015-04-17: Sync test cli

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-17:

#

PASSED: Continuous integration, rev:152
http://jenkins.qa.ubuntu.com/job/media-hub-ci/344/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/184
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/184
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/184/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/184

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/344/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-17:

#

PASSED: Continuous integration, rev:153
http://jenkins.qa.ubuntu.com/job/media-hub-ci/346/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/186
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/186
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/186/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/186

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/346/rebuild

review: Approve (continuous-integration)

lp:~ricmm/media-hub/session-reattach-nonfixed updated on 2015-04-17

154. By Ricardo Mendoza on 2015-04-17: Merge parent

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-17:

#

PASSED: Continuous integration, rev:154
http://jenkins.qa.ubuntu.com/job/media-hub-ci/347/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/187
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/187
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/187/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/187

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/347/rebuild

review: Approve (continuous-integration)

lp:~ricmm/media-hub/session-reattach-nonfixed updated on 2015-04-20

155. By Ricardo Mendoza on 2015-04-20: Merge parent

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-20:

#

PASSED: Continuous integration, rev:155
http://jenkins.qa.ubuntu.com/job/media-hub-ci/348/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/188
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/188
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/188/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/188

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/348/rebuild

review: Approve (continuous-integration)

Media Hub

Merge lp:~ricmm/media-hub/session-reattach-nonfixed into lp:media-hub

Commit message

Description of the change

Preview Diff

Subscribers

 === modified file 'CMakeLists.txt'
 --- CMakeLists.txt	2015-01-23 13:00:36 +0000
 +++ CMakeLists.txt	2015-04-20 15:56:34 +0000
@@ -3,7 +3,7 @@
  project(ubuntu-media-hub)
  set(UBUNTU_MEDIA_HUB_VERSION_MAJOR 3)
--set(UBUNTU_MEDIA_HUB_VERSION_MINOR 0)
++set(UBUNTU_MEDIA_HUB_VERSION_MINOR 1)
  set(UBUNTU_MEDIA_HUB_VERSION_PATCH 0)
  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -fPIC -pthread")
 === modified file 'debian/changelog'
 --- debian/changelog	2015-04-02 16:43:42 +0000
 +++ debian/changelog	2015-04-20 15:56:34 +0000
@@ -1,3 +1,9 @@
++media-hub (3.1.0) UNRELEASED; urgency=medium
++
++  * Support for signalling player reconnection up the stack.
++
++ -- Ricardo Mendoza <ricardo.mendoza@canonical.com>  Fri, 10 Apr 2015 18:20:32 +0200
++
  media-hub (3.0.0+15.04.20150402-0ubuntu1) vivid; urgency=medium
    [ Jim Hodapp ]
 === modified file 'include/core/media/player.h'
 --- include/core/media/player.h	2015-04-20 15:56:34 +0000
 +++ include/core/media/player.h	2015-04-20 15:56:34 +0000
@@ -126,6 +126,10 @@
      Player& operator=(const Player&) = delete;
      bool operator==(const Player&) const = delete;
++    virtual std::string uuid() const = 0;
++    virtual void reconnect() = 0;
++    virtual void abandon() = 0;
++
      virtual std::shared_ptr<TrackList> track_list() = 0;
      virtual PlayerKey key() const = 0;
 === modified file 'include/core/media/service.h'
 --- include/core/media/service.h	2014-09-30 06:51:15 +0000
 +++ include/core/media/service.h	2015-04-20 15:56:34 +0000
@@ -42,9 +42,25 @@
      Service& operator=(const Service&) = delete;
      bool operator==(const Service&) const = delete;
++    /** @brief Creates a session with the media-hub service. */
      virtual std::shared_ptr<Player> create_session(const Player::Configuration&) = 0;
++
++    /** @brief Detaches a UUID-identified session for later resuming. */
++    virtual void detach_session(const std::string& uuid, const Player::Configuration&) = 0;
++
++    /** @brief Reattaches to a UUID-identified session that is in detached state. */
++    virtual std::shared_ptr<Player> reattach_session(const std::string& uuid, const Player::Configuration&) = 0;
++
++    /** @brief Asks the service to destroy a session. The session is destroyed when the client exits. */
++    virtual void destroy_session(const std::string& uuid, const Player::Configuration&) = 0;
++
++    /** @brief Creates a fixed-named session with the media-hub service. */
      virtual std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&) = 0;
++
++    /** @brief Resumes a fixed-name session directly by player key. */
      virtual std::shared_ptr<Player> resume_session(Player::PlayerKey) = 0;
++
++    /** @brief Pauses sessions other than the supplied one. */
      virtual void pause_other_sessions(Player::PlayerKey) = 0;
    protected:
 === modified file 'src/core/media/mpris/service.h'
 --- src/core/media/mpris/service.h	2014-09-30 06:51:15 +0000
 +++ src/core/media/mpris/service.h	2015-04-20 15:56:34 +0000
@@ -48,6 +48,42 @@
+             }
          };
++        struct DetachingSession
++        {
++            static const std::string& name()
++            {
++                static const std::string s
++                {
++                    "core.ubuntu.media.Service.Error.DetachingSession"
++                };
++                return s;
++            }
++        };
++
++        struct ReattachingSession
++        {
++            static const std::string& name()
++            {
++                static const std::string s
++                {
++                    "core.ubuntu.media.Service.Error.ReattachingSession"
++                };
++                return s;
++            }
++        };
++
++        struct DestroyingSession
++        {
++            static const std::string& name()
++            {
++                static const std::string s
++                {
++                    "core.ubuntu.media.Service.Error.DestroyingSession"
++                };
++                return s;
++            }
++        };
++
          struct CreatingFixedSession
+         {
              static const std::string& name()
@@ -74,6 +110,9 @@
      };
      DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(CreateSession, Service, 1000)
++    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(DetachSession, Service, 1000)
++    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(ReattachSession, Service, 1000)
++    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(DestroySession, Service, 1000)
      DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(CreateFixedSession, Service, 1000)
      DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(ResumeSession, Service, 1000)
      DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(PauseOtherSessions, Service, 1000)
 === modified file 'src/core/media/player_configuration.h'
 --- src/core/media/player_configuration.h	2015-04-20 15:56:34 +0000
 +++ src/core/media/player_configuration.h	2015-04-20 15:56:34 +0000
@@ -27,7 +27,7 @@
  // to the implementation in a way that is opaque to the client.
  struct core::ubuntu::media::Player::Configuration
+ {
--    // Unique key for identifying the session.
++    // Unique key for identifying the session path.
      core::ubuntu::media::Player::PlayerKey key;
      // The bus connection to expose objects on.
      std::shared_ptr<core::dbus::Bus> bus;
 === modified file 'src/core/media/player_implementation.cpp'
 --- src/core/media/player_implementation.cpp	2015-04-20 15:56:34 +0000
 +++ src/core/media/player_implementation.cpp	2015-04-20 15:56:34 +0000
@@ -68,7 +68,7 @@
            previous_state(Engine::State::stopped),
            engine_state_change_connection(engine->state().changed().connect(make_state_change_handler())),
            engine_playback_status_change_connection(engine->playback_status_changed_signal().connect(make_playback_status_change_handler())),
--          doing_go_to_track(false)
++          doing_abandon(false)
+     {
          // Poor man's logging of release/acquire events.
          display_state_lock->acquired().connect([](media::power::DisplayState state)
@@ -301,7 +301,8 @@
      core::Connection engine_state_change_connection;
      core::Connection engine_playback_status_change_connection;
      // Prevent the TrackList from auto advancing to the next track
--    std::atomic<bool> doing_go_to_track;
++    std::mutex doing_go_to_track;
++    std::atomic<bool> doing_abandon;
  };
  template<typename Parent>
@@ -390,23 +391,26 @@
      d->engine->about_to_finish_signal().connect([this]()
+     {
++        if (d->doing_abandon)
++            return;
++
          Parent::about_to_finish()();
--        if (!d->doing_go_to_track)
++        // This lambda needs to be mutually exclusive with the on_go_to_track lambda below
++        d->doing_go_to_track.lock();
++
++        const media::Track::Id prev_track_id = d->track_list->current();
++        // Make sure that the TrackList keeps advancing. The logic for what gets played next,
++        // if anything at all, occurs in TrackListSkeleton::next()
++        const Track::UriType uri = d->track_list->query_uri_for_track(d->track_list->next());
++        if (prev_track_id != d->track_list->current() && !uri.empty())
+         {
--            const media::Track::Id prev_track_id = d->track_list->current();
--            // Make sure that the TrackList keeps advancing. The logic for what gets played next,
--            // if anything at all, occurs in TrackListSkeleton::next()
--            const Track::UriType uri = d->track_list->query_uri_for_track(d->track_list->next());
--            if (prev_track_id != d->track_list->current() && !uri.empty())
--            {
--                std::cout << "Setting next track on playbin: " << uri << std::endl;
--                static const bool do_pipeline_reset = false;
--                d->engine->open_resource_for_uri(uri, do_pipeline_reset);
--            }
++            std::cout << "Setting next track on playbin: " << uri << std::endl;
++            static const bool do_pipeline_reset = false;
++            d->engine->open_resource_for_uri(uri, do_pipeline_reset);
+         }
--        else
--            std::cout << "Not auto-advancing the TrackList since doing_go_to_track is true" << std::endl;
++
++        d->doing_go_to_track.unlock();
      });
      d->engine->client_disconnected_signal().connect([this]()
@@ -443,7 +447,8 @@
+     {
          // This prevents the TrackList from auto advancing in other areas such as the about_to_finish signal
          // handler.
--        d->doing_go_to_track = true;
++        // This lambda needs to be mutually exclusive with the about_to_finish lambda above
++        d->doing_go_to_track.lock();
          const media::Track::Id id = p.first;
          const bool toggle_player_state = p.second;
@@ -463,7 +468,8 @@
          if (toggle_player_state)
              d->engine->play();
--        d->doing_go_to_track = false;
++        d->doing_go_to_track.unlock();
++
      });
      // Everything is setup, we now subscribe to death notifications.
@@ -474,6 +480,9 @@
+     {
          if (auto sp = wp.lock())
+         {
++            if (sp->doing_abandon)
++                return;
++
              if (died != sp->config.key)
                  return;
@@ -519,6 +528,27 @@
+ }
  template<typename Parent>
++std::string media::PlayerImplementation<Parent>::uuid() const
++{
++    // No impl for now, as not needed internally.
++    return std::string{};
++}
++
++template<typename Parent>
++void media::PlayerImplementation<Parent>::reconnect()
++{
++    d->config.client_death_observer->register_for_death_notifications_with_key(d->config.key);
++}
++
++template<typename Parent>
++void media::PlayerImplementation<Parent>::abandon()
++{
++    // Signal client disconnection due to abandonment of player
++    d->doing_abandon = true;
++    d->on_client_died();
++}
++
++template<typename Parent>
  std::shared_ptr<media::TrackList> media::PlayerImplementation<Parent>::track_list()
+ {
      return d->track_list;
 === modified file 'src/core/media/player_implementation.h'
 --- src/core/media/player_implementation.h	2015-04-20 15:56:34 +0000
 +++ src/core/media/player_implementation.h	2015-04-20 15:56:34 +0000
@@ -55,6 +55,10 @@
      PlayerImplementation(const Configuration& configuration);
      ~PlayerImplementation();
++    virtual std::string uuid() const;
++    virtual void reconnect();
++    virtual void abandon();
++
      virtual std::shared_ptr<TrackList> track_list();
      virtual Player::PlayerKey key() const;
 === modified file 'src/core/media/player_stub.cpp'
 --- src/core/media/player_stub.cpp	2015-04-20 15:56:34 +0000
 +++ src/core/media/player_stub.cpp	2015-04-20 15:56:34 +0000
@@ -44,11 +44,13 @@
+ {
      Private(const std::shared_ptr<Service>& parent,
              const std::shared_ptr<core::dbus::Service>& service,
--            const std::shared_ptr<core::dbus::Object>& object
++            const std::shared_ptr<core::dbus::Object>& object,
++            const std::string& uuid
              ) : parent(parent),
                  service(service),
                  object(object),
                  key(object->invoke_method_synchronously<mpris::Player::Key, media::Player::PlayerKey>().value()),
++                uuid(uuid),
                  sink_factory(media::video::make_platform_default_sink_factory(key)),
                  properties
+                 {
@@ -96,6 +98,7 @@
      dbus::Service::Ptr service;
      dbus::Object::Ptr object;
      media::Player::PlayerKey key;
++    std::string uuid;
      media::video::SinkFactory sink_factory;
      struct
+     {
@@ -213,8 +216,9 @@
  media::PlayerStub::PlayerStub(
      const std::shared_ptr<Service>& parent,
      const std::shared_ptr<core::dbus::Service>& service,
--    const std::shared_ptr<core::dbus::Object>& object)
--    : d(new Private{parent, service, object})
++    const std::shared_ptr<core::dbus::Object>& object,
++    const std::string& uuid)
++        : d(new Private{parent, service, object, uuid})
+ {
+ }
@@ -222,6 +226,21 @@
+ {
+ }
++std::string media::PlayerStub::uuid() const
++{
++    return d->uuid;
++}
++
++void media::PlayerStub::reconnect()
++{
++    // No implementation
++}
++
++void media::PlayerStub::abandon()
++{
++    // No implementation
++}
++
  std::shared_ptr<media::TrackList> media::PlayerStub::track_list()
+ {
      if (!d->track_list)
 === modified file 'src/core/media/player_stub.h'
 --- src/core/media/player_stub.h	2015-04-20 15:56:34 +0000
 +++ src/core/media/player_stub.h	2015-04-20 15:56:34 +0000
@@ -40,10 +40,15 @@
      explicit PlayerStub(
          const std::shared_ptr<Service>& parent,
          const std::shared_ptr<core::dbus::Service>& service,
--        const std::shared_ptr<core::dbus::Object>& object);
++        const std::shared_ptr<core::dbus::Object>& object,
++        const std::string& uuid = std::string{});
      ~PlayerStub();
++    virtual std::string uuid() const;
++    virtual void reconnect();
++    virtual void abandon();
++
      virtual std::shared_ptr<TrackList> track_list();
      virtual PlayerKey key() const;
 === modified file 'src/core/media/server/server.cpp'
 --- src/core/media/server/server.cpp	2014-12-15 14:43:44 +0000
 +++ src/core/media/server/server.cpp	2015-04-20 15:56:34 +0000
@@ -123,7 +123,7 @@
+     {
          impl,
          player_store,
--
++        external_services
      });
      std::thread service_worker
 === modified file 'src/core/media/service_implementation.cpp'
 --- src/core/media/service_implementation.cpp	2015-04-20 15:56:34 +0000
 +++ src/core/media/service_implementation.cpp	2015-04-20 15:56:34 +0000
@@ -218,6 +218,22 @@
      return player;
+ }
++void media::ServiceImplementation::detach_session(const std::string&, const media::Player::Configuration&)
++{
++    // no impl
++}
++
++std::shared_ptr<media::Player> media::ServiceImplementation::reattach_session(const std::string&, const media::Player::Configuration&)
++{
++    // no impl
++    return std::shared_ptr<media::Player>();
++}
++
++void media::ServiceImplementation::destroy_session(const std::string&, const media::Player::Configuration&)
++{
++    // no impl
++}
++
  std::shared_ptr<media::Player> media::ServiceImplementation::create_fixed_session(const std::string&, const media::Player::Configuration&)
+ {
    // no impl
 === modified file 'src/core/media/service_implementation.h'
 --- src/core/media/service_implementation.h	2015-04-02 14:46:52 +0000
 +++ src/core/media/service_implementation.h	2015-04-20 15:56:34 +0000
@@ -44,6 +44,9 @@
      ~ServiceImplementation ();
      std::shared_ptr<Player> create_session(const Player::Configuration&);
++    void detach_session(const std::string&, const Player::Configuration&);
++    std::shared_ptr<Player> reattach_session(const std::string&, const Player::Configuration&);
++    void destroy_session(const std::string&, const Player::Configuration&);
      std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&);
      std::shared_ptr<Player> resume_session(Player::PlayerKey key);
      void pause_other_sessions(Player::PlayerKey key);
 === modified file 'src/core/media/service_skeleton.cpp'
 --- src/core/media/service_skeleton.cpp	2015-04-20 15:56:34 +0000
 +++ src/core/media/service_skeleton.cpp	2015-04-20 15:56:34 +0000
@@ -35,6 +35,10 @@
  #include <core/posix/this_process.h>
++#include <boost/uuid/uuid.hpp>
++#include <boost/uuid/uuid_generators.hpp>
++#include <boost/uuid/uuid_io.hpp>
++
  #include <map>
  #include <regex>
  #include <sstream>
@@ -50,17 +54,33 @@
  struct media::ServiceSkeleton::Private
+ {
      Private(media::ServiceSkeleton* impl, const ServiceSkeleton::Configuration& config)
--        : impl(impl),
++        : request_context_resolver(media::apparmor::ubuntu::make_platform_default_request_context_resolver(config.external_services)),
++          impl(impl),
            object(impl->access_service()->add_object_for_path(
                       dbus::traits::Service<media::Service>::object_path())),
--          exported(impl->access_bus(), config.cover_art_resolver),
--          configuration(config)
++          configuration(config),
++          exported(impl->access_bus(), config.cover_art_resolver)
+     {
          object->install_method_handler<mpris::Service::CreateSession>(
                      std::bind(
                          &Private::handle_create_session,
                          this,
                          std::placeholders::_1));
++        object->install_method_handler<mpris::Service::DetachSession>(
++                    std::bind(
++                        &Private::handle_detach_session,
++                        this,
++                        std::placeholders::_1));
++        object->install_method_handler<mpris::Service::ReattachSession>(
++                    std::bind(
++                        &Private::handle_reattach_session,
++                        this,
++                        std::placeholders::_1));
++        object->install_method_handler<mpris::Service::DestroySession>(
++                    std::bind(
++                        &Private::handle_destroy_session,
++                        this,
++                        std::placeholders::_1));
          object->install_method_handler<mpris::Service::CreateFixedSession>(
                      std::bind(
                          &Private::handle_create_fixed_session,
@@ -78,24 +98,26 @@
                          std::placeholders::_1));
+     }
--    std::pair<std::string, media::Player::PlayerKey> create_session_info()
++    std::tuple<std::string, media::Player::PlayerKey, std::string> create_session_info()
+     {
          static unsigned int session_counter = 0;
          unsigned int current_session = session_counter++;
++        boost::uuids::uuid uuid = gen();
          std::stringstream ss;
          ss << "/core/ubuntu/media/Service/sessions/" << current_session;
--        return std::make_pair(ss.str(), media::Player::PlayerKey(current_session));
++        return std::make_tuple(ss.str(), media::Player::PlayerKey(current_session), to_string(uuid));
+     }
      void handle_create_session(const core::dbus::Message::Ptr& msg)
+     {
--        auto  session_info = create_session_info();
++        auto session_info = create_session_info();
--        dbus::types::ObjectPath op{session_info.first};
--        media::Player::PlayerKey key{session_info.second};
++        dbus::types::ObjectPath op{std::get<0>(session_info)};
++        media::Player::PlayerKey key{std::get<1>(session_info)};
++        std::string uuid{std::get<2>(session_info)};
          media::Player::Configuration config
+         {
@@ -105,11 +127,21 @@
              impl->access_service()->add_object_for_path(op)
          };
++        std::cout << "Session created by request of: " << msg->sender() << ", uuid: " << uuid << ", path:" << op << std::endl;
++
          try
+         {
              configuration.player_store->add_player_for_key(key, impl->create_session(config));
++            uuid_player_map.insert(std::make_pair(uuid, key));
++
++            request_context_resolver->resolve_context_for_dbus_name_async(msg->sender(), [this, key, msg](const media::apparmor::ubuntu::Context& context)
++            {
++                fprintf(stderr, "%s():%d -- app_name='%s', attached\n", __func__, __LINE__, context.str().c_str());
++                player_owner_map.insert(std::make_pair(key, std::make_tuple(context.str(), true, msg->sender())));
++            });
++
              auto reply = dbus::Message::make_method_return(msg);
--            reply->writer() << op;
++            reply->writer() << std::make_tuple(op, uuid);
              impl->access_bus()->send(reply);
          } catch(const std::runtime_error& e)
@@ -122,6 +154,174 @@
+         }
+     }
++    void handle_detach_session(const core::dbus::Message::Ptr& msg)
++    {
++        try
++        {
++            std::string uuid;
++            msg->reader() >> uuid;
++
++            auto key = uuid_player_map.at(uuid);
++
++            if (player_owner_map.count(key) != 0) {
++                auto info = player_owner_map.at(key);
++                // Check if session is attached(1) and that the detachment
++                // request comes from the same peer(2) that created the session.
++                if (std::get<1>(info) && (std::get<2>(info) == msg->sender())) { // Player is attached
++                    std::get<1>(info) = false; // Detached
++                    std::get<2>(info).clear(); // Clear registered sender/peer
++                    auto player = configuration.player_store->player_for_key(key);
++                    player->lifetime().set(media::Player::Lifetime::resumable);
++                }
++            }
++
++            auto reply = dbus::Message::make_method_return(msg);
++            impl->access_bus()->send(reply);
++
++        } catch(const std::runtime_error& e)
++        {
++            auto reply = dbus::Message::make_error(
++                        msg,
++                        mpris::Service::Errors::DetachingSession::name(),
++                        e.what());
++            impl->access_bus()->send(reply);
++        }
++    }
++
++    void handle_reattach_session(const core::dbus::Message::Ptr& msg)
++    {
++        try
++        {
++            std::string uuid;
++            msg->reader() >> uuid;
++
++            if (uuid_player_map.count(uuid) != 0) {
++                auto key = uuid_player_map.at(uuid);
++                if (not configuration.player_store->has_player_for_key(key)) {
++                    auto reply = dbus::Message::make_error(
++                                msg,
++                                mpris::Service::Errors::ReattachingSession::name(),
++                                "Unable to locate player session");
++                    impl->access_bus()->send(reply);
++                    return;
++                }
++                std::stringstream ss;
++                ss << "/core/ubuntu/media/Service/sessions/" << key;
++                dbus::types::ObjectPath op{ss.str()};
++
++                request_context_resolver->resolve_context_for_dbus_name_async(msg->sender(), [this, msg, key, op](const media::apparmor::ubuntu::Context& context)
++                {
++                    auto info = player_owner_map.at(key);
++                    fprintf(stderr, "%s():%d -- reattach app_name='%s', info='%s', '%s'\n", __func__, __LINE__, context.str().c_str(), std::get<0>(info).c_str(), std::get<2>(info).c_str());
++                    if (std::get<0>(info) == context.str()) {
++                        std::get<1>(info) = true; // Set to Attached
++                        std::get<2>(info) = msg->sender(); // Register new owner
++
++                        // Signal player reconnection
++                        auto player = configuration.player_store->player_for_key(key);
++                        player->reconnect();
++
++                        auto reply = dbus::Message::make_method_return(msg);
++                        reply->writer() << op;
++
++                        impl->access_bus()->send(reply);
++                    }
++                    else {
++                        auto reply = dbus::Message::make_error(
++                                    msg,
++                                    mpris::Service::Errors::ReattachingSession::name(),
++                                    "Invalid permissions for the requested session");
++                        impl->access_bus()->send(reply);
++                        return;
++                    }
++                });
++            }
++            else {
++               auto reply = dbus::Message::make_error(
++                           msg,
++                           mpris::Service::Errors::ReattachingSession::name(),
++                           "Invalid session");
++               impl->access_bus()->send(reply);
++               return;
++            }
++        } catch(const std::runtime_error& e)
++        {
++            auto reply = dbus::Message::make_error(
++                        msg,
++                        mpris::Service::Errors::ReattachingSession::name(),
++                        e.what());
++            impl->access_bus()->send(reply);
++        }
++    }
++
++    void handle_destroy_session(const core::dbus::Message::Ptr& msg)
++    {
++
++        try
++        {
++            std::string uuid;
++            msg->reader() >> uuid;
++
++            if (uuid_player_map.count(uuid) != 0) {
++                auto key = uuid_player_map.at(uuid);
++                if (not configuration.player_store->has_player_for_key(key)) {
++                    auto reply = dbus::Message::make_error(
++                                msg,
++                                mpris::Service::Errors::DestroyingSession::name(),
++                                "Unable to locate player session");
++                    impl->access_bus()->send(reply);
++                    return;
++                }
++
++                // Remove control entries from the map, at this point
++                // the session is no longer usable.
++                uuid_player_map.erase(uuid);
++
++                request_context_resolver->resolve_context_for_dbus_name_async(msg->sender(), [this, msg, key](const media::apparmor::ubuntu::Context& context)
++                {
++                    auto info = player_owner_map.at(key);
++                    fprintf(stderr, "%s():%d -- Destroying app_name='%s', info='%s', '%s'\n", __func__, __LINE__, context.str().c_str(), std::get<0>(info).c_str(), std::get<2>(info).c_str());
++                    if (std::get<0>(info) == context.str()) {
++                        player_owner_map.erase(key);
++
++                        // Reset lifecycle to non-resumable on the now-abandoned session
++                        auto player = configuration.player_store->player_for_key(key);
++
++                        // Delete player instance by abandonment
++                        player->lifetime().set(media::Player::Lifetime::normal);
++                        player->abandon();
++
++                        auto reply = dbus::Message::make_method_return(msg);
++                        impl->access_bus()->send(reply);
++                    }
++                    else {
++                        auto reply = dbus::Message::make_error(
++                                    msg,
++                                    mpris::Service::Errors::DestroyingSession::name(),
++                                    "Invalid permissions for the requested session");
++                        impl->access_bus()->send(reply);
++                        return;
++                    }
++                });
++            }
++            else {
++               auto reply = dbus::Message::make_error(
++                           msg,
++                           mpris::Service::Errors::DestroyingSession::name(),
++                           "Invalid session");
++               impl->access_bus()->send(reply);
++               return;
++            }
++        } catch(const std::runtime_error& e)
++        {
++            auto reply = dbus::Message::make_error(
++                        msg,
++                        mpris::Service::Errors::DestroyingSession::name(),
++                        e.what());
++            impl->access_bus()->send(reply);
++        }
++    }
++
      void handle_create_fixed_session(const core::dbus::Message::Ptr& msg)
+     {
          try
@@ -133,8 +333,8 @@
                  // Create new session
                  auto  session_info = create_session_info();
--                dbus::types::ObjectPath op{session_info.first};
--                media::Player::PlayerKey key{session_info.second};
++                dbus::types::ObjectPath op{std::get<0>(session_info)};
++                media::Player::PlayerKey key{std::get<1>(session_info)};
                  media::Player::Configuration config
+                 {
@@ -149,7 +349,6 @@
                  configuration.player_store->add_player_for_key(key, session);
--
                  named_player_map.insert(std::make_pair(name, key));
                  auto reply = dbus::Message::make_method_return(msg);
@@ -233,6 +432,7 @@
          impl->access_bus()->send(reply);
+     }
++    media::apparmor::ubuntu::RequestContextResolver::Ptr request_context_resolver;
      media::ServiceSkeleton* impl;
      dbus::Object::Ptr object;
@@ -240,6 +440,14 @@
      ServiceSkeleton::Configuration configuration;
      // We map named/fixed player instances to their respective keys.
      std::map<std::string, media::Player::PlayerKey> named_player_map;
++    // We map UUIDs to their respective keys.
++    std::map<std::string, media::Player::PlayerKey> uuid_player_map;
++    // We keep a list of keys and their respective owners and states.
++    // value: (owner context, attached state, attached dbus name)
++    std::map<media::Player::PlayerKey, std::tuple<std::string, bool, std::string>> player_owner_map;
++
++    boost::uuids::random_generator gen;
++
      // We expose the entire service as an MPRIS player.
      struct Exported
+     {
@@ -527,6 +735,21 @@
      return d->configuration.impl->create_session(config);
+ }
++void media::ServiceSkeleton::detach_session(const std::string& uuid, const media::Player::Configuration& config)
++{
++    return d->configuration.impl->detach_session(uuid, config);
++}
++
++std::shared_ptr<media::Player> media::ServiceSkeleton::reattach_session(const std::string& uuid, const media::Player::Configuration& config)
++{
++    return d->configuration.impl->reattach_session(uuid, config);
++}
++
++void media::ServiceSkeleton::destroy_session(const std::string& uuid, const media::Player::Configuration& config)
++{
++    return d->configuration.impl->destroy_session(uuid, config);
++}
++
  std::shared_ptr<media::Player> media::ServiceSkeleton::create_fixed_session(const std::string& name, const media::Player::Configuration&config)
+ {
      return d->configuration.impl->create_fixed_session(name, config);
 === modified file 'src/core/media/service_skeleton.h'
 --- src/core/media/service_skeleton.h	2014-12-15 14:43:44 +0000
 +++ src/core/media/service_skeleton.h	2015-04-20 15:56:34 +0000
@@ -19,6 +19,8 @@
  #ifndef CORE_UBUNTU_MEDIA_SERVICE_SKELETON_H_
  #define CORE_UBUNTU_MEDIA_SERVICE_SKELETON_H_
++#include "apparmor/ubuntu.h"
++
  #include <core/media/service.h>
  #include "cover_art_resolver.h"
@@ -43,6 +45,7 @@
+     {
          std::shared_ptr<Service> impl;
          KeyedPlayerStore::Ptr player_store;
++        helper::ExternalServices& external_services;
          CoverArtResolver cover_art_resolver;
      };
@@ -51,6 +54,9 @@
      // From media::Service
      std::shared_ptr<Player> create_session(const Player::Configuration&);
++    void detach_session(const std::string&, const Player::Configuration&);
++    std::shared_ptr<Player> reattach_session(const std::string&, const Player::Configuration&);
++    void destroy_session(const std::string&, const media::Player::Configuration&);
      std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&);
      std::shared_ptr<Player> resume_session(Player::PlayerKey);
      void pause_other_sessions(Player::PlayerKey key);
 === modified file 'src/core/media/service_stub.cpp'
 --- src/core/media/service_stub.cpp	2015-04-20 15:56:34 +0000
 +++ src/core/media/service_stub.cpp	2015-04-20 15:56:34 +0000
@@ -58,7 +58,7 @@
  std::shared_ptr<media::Player> media::ServiceStub::create_session(const media::Player::Configuration&)
+ {
      auto op = d->object->invoke_method_synchronously<mpris::Service::CreateSession,
--         dbus::types::ObjectPath>();
++         std::tuple<dbus::types::ObjectPath, std::string>>();
      if (op.is_error())
          throw std::runtime_error("Problem creating session: " + op.error());
@@ -67,8 +67,44 @@
+     {
          shared_from_this(),
          access_service(),
--        access_service()->object_for_path(op.value())
--    });
++        access_service()->object_for_path(std::get<0>(op.value())),
++        std::get<1>(op.value())
++    });
++}
++
++void media::ServiceStub::detach_session(const std::string& uuid, const media::Player::Configuration&)
++{
++    auto op = d->object->invoke_method_synchronously<mpris::Service::DetachSession,
++         void>(uuid);
++
++    if (op.is_error())
++        throw std::runtime_error("Problem detaching session: " + op.error());
++}
++
++std::shared_ptr<media::Player> media::ServiceStub::reattach_session(const std::string& uuid, const media::Player::Configuration&)
++{
++    auto op = d->object->invoke_method_synchronously<mpris::Service::ReattachSession,
++         dbus::types::ObjectPath>(uuid);
++
++    if (op.is_error())
++        throw std::runtime_error("Problem reattaching session: " + op.error());
++
++    return std::shared_ptr<media::Player>(new media::PlayerStub
++    {
++        shared_from_this(),
++        access_service(),
++        access_service()->object_for_path(op.value()),
++        uuid
++    });
++}
++
++void media::ServiceStub::destroy_session(const std::string& uuid, const media::Player::Configuration&)
++{
++    auto op = d->object->invoke_method_synchronously<mpris::Service::DestroySession,
++         void>(uuid);
++
++    if (op.is_error())
++        throw std::runtime_error("Problem destroying session: " + op.error());
+ }
  std::shared_ptr<media::Player> media::ServiceStub::create_fixed_session(const std::string& name, const media::Player::Configuration&)
 === modified file 'src/core/media/service_stub.h'
 --- src/core/media/service_stub.h	2014-10-23 14:42:59 +0000
 +++ src/core/media/service_stub.h	2015-04-20 15:56:34 +0000
@@ -40,6 +40,9 @@
      ~ServiceStub();
      std::shared_ptr<Player> create_session(const Player::Configuration&);
++    void detach_session(const std::string& uuid, const Player::Configuration&);
++    std::shared_ptr<Player> reattach_session(const std::string& uuid, const Player::Configuration&);
++    void destroy_session(const std::string& uuid, const Player::Configuration&);
      std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&);
      std::shared_ptr<Player> resume_session(Player::PlayerKey key);
      void pause_other_sessions(Player::PlayerKey key);
 === modified file 'tests/test-track-list/test_track_list.cpp'
 --- tests/test-track-list/test_track_list.cpp	2015-04-20 15:56:34 +0000
 +++ tests/test-track-list/test_track_list.cpp	2015-04-20 15:56:34 +0000
@@ -44,7 +44,7 @@
+ {
+ }
--void media::TestTrackList::create_new_player_session()
++std::string media::TestTrackList::create_new_player_session()
+ {
      try {
          m_hubPlayerSession = m_hubService->create_session(media::Player::Client::default_configuration());
@@ -59,12 +59,53 @@
      catch (std::runtime_error &e) {
          cerr << "FATAL: Failed to retrieve the current player's TrackList: " << e.what() << endl;
+     }
--}
--
--void media::TestTrackList::destroy_player_session()
--{
--    // TODO: explicitly add a destroy session to the Service class after ricmm lands his new creation_session
--    // that returns a session ID. This will allow me to clear the tracklist after each test.
++
++    std::string uuid;
++    try {
++        uuid.assign(m_hubPlayerSession->uuid());
++    }
++    catch (std::runtime_error &e) {
++        cerr << "FATAL: Failed to retrieve the current player's uuid: " << e.what() << endl;
++    }
++
++    cout << "Connected to session " << uuid << endl;
++    return uuid;
++}
++
++void media::TestTrackList::detach_player_session(const std::string &uuid)
++{
++    try {
++        m_hubService->detach_session(uuid, media::Player::Client::default_configuration());
++    }
++    catch (std::runtime_error &e) {
++        cerr << "FATAL: Failed to detach the media-hub player session: " << e.what() << endl;
++    }
++
++    cout << "Detached session " << uuid << endl;
++}
++
++void media::TestTrackList::reattach_player_session(const std::string &uuid)
++{
++    try {
++        m_hubPlayerSession = m_hubService->reattach_session(uuid, media::Player::Client::default_configuration());
++    }
++    catch (std::runtime_error &e) {
++        cerr << "FATAL: Failed to reattach the media-hub player session: " << e.what() << endl;
++    }
++
++    try {
++        m_hubTrackList = m_hubPlayerSession->track_list();
++    }
++    catch (std::runtime_error &e) {
++        cerr << "FATAL: Failed to retrieve the current player's TrackList: " << e.what() << endl;
++    }
++
++    cout << "Re-connected to session " << uuid << endl;
++}
++
++void media::TestTrackList::destroy_player_session(const std::string &uuid)
++{
++    m_hubService->destroy_session(uuid, media::Player::Client::default_configuration());
      m_hubPlayerSession.reset();
+ }
@@ -92,13 +133,13 @@
+ {
      cout << "--> Running test: test_basic_playback" << std::endl;
--    core::Connection c =m_hubTrackList->on_track_added().connect([](const media::Track::Id &new_id)
++    const std::string uuid = create_new_player_session();
++
++    core::Connection c = m_hubTrackList->on_track_added().connect([](const media::Track::Id &new_id)
+     {
          cout << "Added track to TrackList with Id: " << new_id << endl;
      });
--    //create_new_player_session();
--
      m_hubPlayerSession->open_uri(uri1);
      if (!uri2.empty())
          add_track(uri2);
@@ -118,13 +159,34 @@
      c.disconnect();
--    //destroy_player_session();
++    destroy_player_session(uuid);
++}
++
++void media::TestTrackList::test_tracklist_resume(const std::string &uri1, const std::string &uri2, const std::string &uuid)
++{
++    cout << "--> Running test: test_tracklist_resume" << std::endl;
++
++    add_track(uri1);
++    add_track(uri2);
++
++    const size_t initial_size = m_hubTrackList->tracks().get().size();
++    cout << "Tracklist size: " << initial_size << endl;
++    detach_player_session(uuid);
++    reattach_player_session(uuid);
++    cout << "Tracklist size: " << m_hubTrackList->tracks().get().size() << endl;
++
++    m_hubPlayerSession->play();
++
++    if (initial_size != m_hubTrackList->tracks().get().size())
++        cout << "Tracklist sizes are different, error in resuming" << endl;
+ }
  void media::TestTrackList::test_ensure_tracklist_is_not_empty(const std::string &uri1, const std::string &uri2)
+ {
      cout << "--> Running test: test_ensure_tracklist_is_not_empty" << std::endl;
++    const std::string uuid = create_new_player_session();
++
      add_track(uri1);
      if (!uri2.empty())
          add_track(uri2);
@@ -133,13 +195,15 @@
          cout << "TrackList is not empty, test success" << endl;
      else
          cout << "TrackList is empty, test failure" << endl;
++
++    destroy_player_session(uuid);
+ }
  void media::TestTrackList::test_has_next_track(const std::string &uri1, const std::string &uri2)
+ {
      cout << "--> Running test: test_has_next_track" << std::endl;
--    //create_new_player_session();
++    const std::string uuid = create_new_player_session();
      add_track(uri1);
      add_track(uri2);
@@ -158,13 +222,15 @@
      else
          cerr << "Playback did not start successfully" << endl;
--    //destroy_player_session();
++    destroy_player_session(uuid);
+ }
  void media::TestTrackList::test_shuffle(const std::string &uri1, const std::string &uri2, const std::string &uri3)
+ {
      cout << "--> Running test: test_shuffle" << std::endl;
++    const std::string uuid = create_new_player_session();
++
      add_track(uri1);
      add_track(uri2);
      add_track(uri3);
@@ -189,22 +255,27 @@
          cout << "Waiting for second track to finish playing..." << endl;
          wait_for_about_to_finish();
--        cout << "Going straight to the Track with Id of '/core/ubuntu/media/Service/sessions/0/TrackList/4'" << std::endl;
--        const media::Track::Id id{"/core/ubuntu/media/Service/sessions/0/TrackList/4"};
++        const media::Track::Id id{m_hubTrackList->tracks().get()[3]};
++        cout << "Going straight to the Track with Id " << id << std::endl;
          const bool toggle_player_state = true;
          m_hubTrackList->go_to(id, toggle_player_state);
++
          cout << "Waiting for third track to finish playing..." << endl;
          wait_for_about_to_finish();
+     }
      else
          cerr << "Playback did not start successfully" << endl;
++
++    destroy_player_session(uuid);
+ }
  void media::TestTrackList::test_remove_track(const std::string &uri1, const std::string &uri2, const std::string &uri3)
+ {
      cout << "--> Running test: test_remove_track" << std::endl;
--    core::Connection c =m_hubTrackList->on_track_removed().connect([](const media::Track::Id &new_id)
++    const std::string uuid = create_new_player_session();
++
++    core::Connection c = m_hubTrackList->on_track_removed().connect([](const media::Track::Id &new_id)
+     {
          cout << "Removed track from TrackList with Id: " << new_id << endl;
      });
@@ -220,7 +291,8 @@
          cout << "Waiting for first track to finish playing..." << endl;
          wait_for_about_to_finish();
--        const media::Track::Id id{"/core/ubuntu/media/Service/sessions/0/TrackList/1"};
++        const media::Track::Id id{m_hubTrackList->tracks().get()[1]};
++        cout << "Removing Track with Id of '" << id << "'" << std::endl;
          m_hubTrackList->remove_track(id);
          cout << "Waiting for track after removed track to finish playing..." << endl;
@@ -228,6 +300,8 @@
+     }
      else
          cerr << "Playback did not start successfully" << endl;
++
++    destroy_player_session(uuid);
+ }
  template<class T>
@@ -316,26 +390,31 @@
      if (argc == 2)
+     {
--        tracklist->create_new_player_session();
          tracklist->test_basic_playback(argv[1]);
          tracklist->test_ensure_tracklist_is_not_empty(argv[1]);
+     }
      else if (argc == 3)
+     {
--        tracklist->create_new_player_session();
          tracklist->test_basic_playback(argv[1], argv[2]);
          tracklist->test_ensure_tracklist_is_not_empty(argv[1], argv[2]);
          tracklist->test_has_next_track(argv[1], argv[2]);
+     }
      else if (argc == 4)
+     {
--        tracklist->create_new_player_session();
          tracklist->test_basic_playback(argv[1]);
          tracklist->test_ensure_tracklist_is_not_empty(argv[1], argv[2]);
          tracklist->test_has_next_track(argv[1], argv[2]);
          tracklist->test_shuffle(argv[1], argv[2], argv[3]);
          tracklist->test_remove_track(argv[1], argv[2], argv[3]);
+     }
++    else if (argc == 5)
++    {
++        std::string uuid = tracklist->create_new_player_session();
++
++        tracklist->test_tracklist_resume(argv[1], argv[2], uuid);
++
++        tracklist->destroy_player_session(uuid);
++    }
      else
+     {
          cout << "Can't test TrackList, no track(s) specified." << endl;
 === modified file 'tests/test-track-list/test_track_list.h'
 --- tests/test-track-list/test_track_list.h	2015-04-20 15:56:34 +0000
 +++ tests/test-track-list/test_track_list.h	2015-04-20 15:56:34 +0000
@@ -45,14 +45,19 @@
      TestTrackList();
      ~TestTrackList();
--    void create_new_player_session();
--    void destroy_player_session();
++    std::string create_new_player_session();
++    void detach_player_session(const std::string &uuid);
++    void reattach_player_session(const std::string &uuid);
++    void destroy_player_session(const std::string &uuid);
      void add_track(const std::string &uri, bool make_current = false);
      // Takes in one or two files for playback, adds it/them to the TrackList, and plays
      void test_basic_playback(const std::string &uri1, const std::string &uri2 = std::string{});
++    // Takes two uris and confirms that they remain after a detach/reattach
++    void test_tracklist_resume(const std::string &uri1, const std::string &uri2, const std::string &uuid);
++
      void test_ensure_tracklist_is_not_empty(const std::string &uri1, const std::string &uri2 = std::string{});
      // Takes in one or two files for playback, adds it/them to the TrackList, plays and makes sure