Mir

Merge lp://staging/~alan-griffiths/mir/socket-connection into lp://staging/mir

  1. socket-connection
  2. Merge into development-branch
Proposed by Alan Griffiths
Status: Merged
Approved by: Robert Ancell
Approved revision: no longer in the source branch.
Merged at revision: 1087
Proposed branch: lp://staging/~alan-griffiths/mir/socket-connection
Merge into: lp://staging/mir
Diff against target: 1779 lines (+594/-283)
32 files modified
examples/basic_server.cpp (+39/-4)
examples/render_surfaces.cpp (+5/-4)
include/server/mir/default_server_configuration.h (+10/-3)
include/server/mir/frontend/connector.h (+14/-14)
include/server/mir/frontend/connector_report.h (+9/-9)
include/server/mir/frontend/session_creator.h (+44/-0)
include/server/mir/server_configuration.h (+2/-2)
include/test/mir_test/test_protobuf_server.h (+4/-4)
src/server/default_server_configuration.cpp (+2/-0)
src/server/display_server.cpp (+9/-9)
src/server/frontend/CMakeLists.txt (+6/-7)
src/server/frontend/default_configuration.cpp (+49/-21)
src/server/frontend/protobuf_session_creator.cpp (+73/-0)
src/server/frontend/protobuf_session_creator.h (+60/-0)
src/server/frontend/published_socket_connector.cpp (+79/-76)
src/server/frontend/published_socket_connector.h (+37/-38)
src/server/frontend/socket_messenger.cpp (+2/-1)
tests/acceptance-tests/test_client_library.cpp (+1/-1)
tests/acceptance-tests/test_server_shutdown.cpp (+3/-3)
tests/acceptance-tests/test_test_framework.cpp (+1/-1)
tests/integration-tests/client/test_client_render.cpp (+1/-1)
tests/integration-tests/shell/test_session.cpp (+5/-4)
tests/integration-tests/test_display_server_main_loop_events.cpp (+33/-32)
tests/integration-tests/test_error_reporting.cpp (+1/-1)
tests/mir_test_doubles/test_protobuf_socket_server.cpp (+10/-10)
tests/unit-tests/client/test_client_mir_surface.cpp (+1/-1)
tests/unit-tests/frontend/CMakeLists.txt (+1/-1)
tests/unit-tests/frontend/stress_protobuf_communicator.cpp (+1/-1)
tests/unit-tests/frontend/test_protobuf_reports_errors.cpp (+1/-1)
tests/unit-tests/frontend/test_protobuf_sends_fds.cpp (+1/-1)
tests/unit-tests/frontend/test_protobuf_surface_apis.cpp (+1/-1)
tests/unit-tests/frontend/test_published_socket_connector.cpp (+89/-32)
To merge this branch: bzr merge lp://staging/~alan-griffiths/mir/socket-connection
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Kevin DuBois (community) Approve
Alexandros Frantzis (community) Approve
Review via email: mp+187326@code.staging.launchpad.net

Commit message

frontend, config: Mechanism for connecting via a socket pair.

Description of the change

frontend, config: Mechanism for connecting via a socket pair.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1099
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/3/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2103
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1988
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/3/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-team-mir-development-branch-ci/3/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

read through once, looks okay to me. want to look it over again, abstain for now

review: Abstain
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1099
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/15/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2116
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2001
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/15/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-team-mir-development-branch-ci/15/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Not a thorough review yet, but I like the new design separating the connection and session creation concerns.

A couple of nits I noticed:

952 +// Makes provides a client-side socket fd for each connection

1734 + EXPECT_CALL(*client, next_buffer_done()).Times(8);
1735 +
1736 + for (int i = 0; i != 8; ++i)

int const num_buffer_requests{8} ?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1100
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/19/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2121
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2006
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/19/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-team-mir-development-branch-ci/19/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1101
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/21/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2123
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2008
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/21
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/21/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-team-mir-development-branch-ci/21/rebuild

review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Looks good.

Nits:

496 +char const* const no_server_socket_opt = "no-file";

Not used.

504 + return std::make_shared<mf::ProtobufSessionCreator>(
505 + the_ipc_factory(the_frontend_shell(),
506 + the_buffer_allocator()),

Current indentation makes the_buffer_allocator() seem like an argument of std::make_shared<mf::ProtobufSessionCreator>() instead of (the correct) the_ipc_factory().

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1103
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/23/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2128
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2013
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/23
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/23/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-team-mir-development-branch-ci/23/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
Approved revid is not set in launchpad. This is most likely a launchpad issue and re-approve should fix it. There is also a chance (although a very small one) this is a permission problem of the ps-jenkins bot.
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-autolanding/5/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2129
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2014
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-autolanding/5
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-autolanding/5/artifact/work/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

client_socket_fd()
the function looks like its making a new session, and creating an fd for a new client. The name could be create_fd_for_new_client()?

create_session_for()
are we creating a session for the socket? or creating a session with a socket? create_session_from_socket()

those are pretty minor naming issues though, lgtm

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches