Mir

Merge lp:~alan-griffiths/mir/fix-1377968 into lp:mir

fix-1377968
Merge into development-branch

Proposed by Alan Griffiths on 2014-10-06

Status:

Merged

Approved by:

Nick Dedekind on 2014-10-07

Approved revision:

no longer in the source branch.

Merged at revision:

1967

Proposed branch:

lp:~alan-griffiths/mir/fix-1377968

Merge into:

lp:mir

Diff against target:

77 lines (+33/-8)

3 files modified

src/server/scene/prompt_session_manager_impl.cpp (+14/-8)
tests/acceptance-tests/test_prompt_session_client_api.cpp (+17/-0)
tests/integration-tests/frontend/test_session_mediator_report.cpp (+2/-0)

To merge this branch:

bzr merge lp:~alan-griffiths/mir/fix-1377968

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Nick Dedekind (community)		2014-10-06	Approve on 2014-10-07
Andreas Pokorny (community)		2014-10-06	Approve on 2014-10-07
PS Jenkins bot (community)	continuous-integration		Approve on 2014-10-06
Review via email: mp+237287@code.launchpad.net

Commit message

trusted prompt sessions: starting a prompt session with an invalid application pid reports an error

Description of the change

trusted prompt sessions: starting a prompt session with an invalid application pid reports an error

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2014-10-06:

36 + prompt_session_container->insert_participant(prompt_session.get(), application_session, PromptSessionContainer::ParticipantType::application);

Indentation.

ServerShutdownWithThreadException.server_releases_resources_on_abnormal_compositor_thread_termination doesn't end for me, but doesn't seem related.

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-06:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/1748/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-06:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/1749/rebuild

review: Approve (continuous-integration)

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-10-07:

test run fine with the change and fails without.

to be even more nit-picky: acquiring the application session and bailing out otherwise could be an extracted method.

review: Approve

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2014-10-07:

Looks good to me. New test is successful.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Alan Griffiths

Emanuele Antonio Faraone

Gerry Boland

Mir development team

 === modified file 'src/server/scene/prompt_session_manager_impl.cpp'
 --- src/server/scene/prompt_session_manager_impl.cpp	2014-09-11 05:51:44 +0000
 +++ src/server/scene/prompt_session_manager_impl.cpp	2014-10-06 15:50:47 +0000
@@ -109,6 +109,19 @@
      PromptSessionCreationParameters const& params) const
+ {
      auto prompt_session = std::make_shared<PromptSession>();
++    std::shared_ptr<Session> application_session;
++
++    app_container->for_each(
++    [&](std::shared_ptr<Session> const& session)
++    {
++        if (session->process_id() == params.application_pid)
++        {
++            application_session = session;
++        }
++    });
++
++    if (!application_session)
++        BOOST_THROW_EXCEPTION(std::runtime_error("Could not identify application session"));
      std::lock_guard<std::mutex> lock(prompt_sessions_mutex);
@@ -119,14 +132,7 @@
      session->start_prompt_session();
      prompt_session_listener->starting(prompt_session);
--    app_container->for_each(
--    [&](std::shared_ptr<Session> const& session)
--    {
--        if (session->process_id() == params.application_pid)
--        {
--            prompt_session_container->insert_participant(prompt_session.get(), session, PromptSessionContainer::ParticipantType::application);
--        }
--    });
++    prompt_session_container->insert_participant(prompt_session.get(), application_session, PromptSessionContainer::ParticipantType::application);
      return prompt_session;
+ }
 === modified file 'tests/acceptance-tests/test_prompt_session_client_api.cpp'
 --- tests/acceptance-tests/test_prompt_session_client_api.cpp	2014-09-30 06:11:33 +0000
 +++ tests/acceptance-tests/test_prompt_session_client_api.cpp	2014-10-06 15:50:47 +0000
@@ -552,3 +552,20 @@
      mir_prompt_session_release_sync(prompt_session);
+ }
++
++// lp:1377968
++TEST_F(PromptSessionClientAPI, when_application_pid_is_invalid_starting_a_prompt_session_fails)
++{
++    connection = mir_connect_sync(new_prompt_connection().c_str(), __PRETTY_FUNCTION__);
++
++    EXPECT_CALL(*the_mock_prompt_session_listener(), starting(_)).Times(0);
++
++    MirPromptSession* prompt_session = mir_connection_create_prompt_session_sync(
++        connection, ~application_session_pid, null_state_change_callback, this);
++
++    EXPECT_THAT(mir_prompt_session_is_valid(prompt_session), Eq(false));
++    EXPECT_THAT(mir_prompt_session_error_message(prompt_session),
++        HasSubstr("Could not identify application"));
++
++    mir_prompt_session_release_sync(prompt_session);
++}
 === modified file 'tests/integration-tests/frontend/test_session_mediator_report.cpp'
 --- tests/integration-tests/frontend/test_session_mediator_report.cpp	2014-09-24 03:38:59 +0000
 +++ tests/integration-tests/frontend/test_session_mediator_report.cpp	2014-10-06 15:50:47 +0000
@@ -510,6 +510,8 @@
              client.wait_for_connect_done();
++            client.prompt_session_parameters.set_application_pid(getpid());
++
              client.display_server.start_prompt_session(
 ,
                  &client.prompt_session_parameters,