Mir

Merge lp:~robert-ancell/mir/frontend-class-renaming into lp:~mir-team/mir/trunk

  1. frontend-class-renaming
  2. Merge into trunk
Proposed by Robert Ancell
Status: Rejected
Rejected by: Robert Ancell
Proposed branch: lp:~robert-ancell/mir/frontend-class-renaming
Merge into: lp:~mir-team/mir/trunk
Diff against target: 486 lines (+73/-117)
11 files modified
src/server/frontend/CMakeLists.txt (+3/-4)
src/server/frontend/make_socket_communicator.cpp (+2/-2)
src/server/frontend/message_processor.cpp (+31/-31)
src/server/frontend/message_processor.h (+8/-8)
src/server/frontend/message_sender.h (+9/-25)
src/server/frontend/null_message_processor.cpp (+0/-26)
src/server/frontend/socket_communicator.cpp (+10/-10)
src/server/frontend/socket_communicator.h (+6/-6)
src/server/frontend/socket_session.cpp (+1/-1)
src/server/frontend/socket_session.h (+1/-2)
tests/mir_test_doubles/test_protobuf_socket_server.cpp (+2/-2)
To merge this branch: bzr merge lp:~robert-ancell/mir/frontend-class-renaming
Reviewer Review Type Date Requested Status
Alexandros Frantzis (community) Needs Fixing
Alan Griffiths Needs Fixing
Daniel van Vugt Approve
Kevin DuBois (community) Needs Information
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+156430@code.launchpad.net

Commit message

Refactor frontend code to use simpler and more consistent class naming

Description of the change

Rename some of the classes in the frontend to make it simpler to understand. This is based on my first impression of understanding the frontend code. While I could see the intention to make the transport layer (socket, binder) and protocol (protocol buffers) flexible I found this to confuse the classes and be overly abstracted.

1. Rename ProtobufSocketCommunicator to SocketCommicator

This is because the Protobuf part is confusing
 - It makes the relationship to SocketSession less clear.
 - The Protobuf prefix is not consistently applied, i.e. SessionMediator is also Protobuf dependent but doesn't use the prefix.
 - One prefix (Socket) is simpler than two (ProtobufSocket).

2. Rename ProtobufMessageProcessor to MessageProcessor

This is the last remaining "Protobuf" class so removing it seems ideal.
 - Removed the NullMessageProcessor since it was only used in one place - a simple NULL check replaces it.
 - message_process.h was overloaded with two classes (MessageProcessor and MessageSender) - Move MessageSender to its own header file and just use protobuf_message_processor.h as the new message_processor.h.

The one part that doesn't feel great now is the fact that there is a clear abstraction for MessageSender, but not for MessageProcessor (which is tied to protocol buffers). I didn't have a good name for using MessageProcessor as the interface and ?MessageProcessor as the implementation. We could use SocketMessageProcessor except the message processor is independent from the transport layer.

The use of "Processor" as a name seems a bit vague to me (i.e. most classes are "processing" something). I think it would be better to use names that are clearer inverses e.g. Sender/Receiver, Encoder/Decoder or Serializer/Deserializer.

(The last two points are worth discussing but I don't think need to be solved by this merge).

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

PASSED: Continuous integration, rev:550
http://jenkins.qa.ubuntu.com/job/mir-ci/271/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/220
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/103
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/272
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/272/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/80

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

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

i'm going to mark as 'needs information' but what I really mean is 'needs discussion'. I guess if the binder code that drove these abstractions is now gone, it would make sense for the abstractions to be reworked. The only part that I don't like is that we're getting rid of MessageProcessor's interface class, will make it harder to mock.

I guess overall I agree with the reasons and the substance for the changes, but don't see the test scenario that's driving the changes that go beyond the name changes (eg, making process_message public instead of private)

review: Needs Information
Revision history for this message
Kevin DuBois (kdub) wrote :

> I guess overall I agree with the reasons and the substance for the changes,
reasons for, and substance of, the changes

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Looks good to me. Tests are unchanged and coverage should slightly increase with this.

If we need to increase abstraction again later for more mocking, then that's an exercise for later.

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

There is a possible argument that "Protobuf" prefixes are appropriate, should anyone want to add support for other protocols later. Though I'm not entirely sure the existing design is sufficient to support a clean replacement of the protocol layer like that. So simpler is better for now.

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

I'm happy with the motivation. Not so happy with some aspects of the proposed changes.

Dependencies should always be on interfaces, not concrete classes - so I think it wrong to conflate MessageProcessor and ProtobufMessageProcessor.

We've a rule in the codebase that we don't have null dependencies (and use null objects instead). I don't think we should be making an exception here just to avoid having a trivial NullMessageProcessor class.

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

It's not a rule; it's a convention. And any convention which results in trivial un(der)used classes should probably be changed, or at least bent. As I said above; "If we need to increase abstraction again later for more mocking, then that's an exercise for later".

I feel that Mir has a growing problem with its code complexity out-pacing functionality. That's a particular concern for our ability to maintain and enhance the code. So any successful attempt to simplify things should surely be welcomed.

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

> There is a possible argument that "Protobuf" prefixes are appropriate, should anyone want to add support for other protocols later

It's also about descriptive naming. ProtobufSocketCommunicator is a communicator that uses protobuf over sockets, all the important information is in the name.

> The only part that I don't like is that we're getting rid of MessageProcessor's interface class, will make it harder to mock.

> Dependencies should always be on interfaces, not concrete classes - so I think it wrong to conflate MessageProcessor and ProtobufMessageProcessor.

The fact that we can make MessageProcessor concrete without any consequences, tells me that we are missing some tests where the interface is used (in SocketSession, e.g. message_processed_when_new_message_arrives), not that we don't need to have it as an interface.

> > We've a rule in the codebase that we don't have null dependencies (and use null objects instead).
> It's not a rule; it's a convention. And any convention which results in trivial un(der)used classes should probably be changed, or at least bent.

However you call it, the point is that there are reasons for it, we didn't just flip a coin and went with NullObjects. If we turn this around: "any convention which results in the circulation of null smart pointers should probably be changed, or at least bent." :) As with any engineering decision, there are trade-offs involved, and the decision was that NullObject trade-offs are worth it.

review: Needs Fixing
Revision history for this message
Robert Ancell (robert-ancell) wrote :

> We've a rule in the codebase that we don't have null dependencies (and use
> null objects instead). I don't think we should be making an exception here
> just to avoid having a trivial NullMessageProcessor class.

I wasn't aware that is the case, I'll work on fixing that. Can you please add this information to the style guide?

Revision history for this message
Robert Ancell (robert-ancell) wrote :

My motivation for this is gone. I will pick it up later if it returns :)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/server/frontend/CMakeLists.txt'
--- src/server/frontend/CMakeLists.txt 2013-03-28 09:50:09 +0000
+++ src/server/frontend/CMakeLists.txt 2013-04-01 21:52:22 +0000
@@ -4,8 +4,7 @@
4 session_mediator.cpp4 session_mediator.cpp
5 null_session_mediator_report.cpp5 null_session_mediator_report.cpp
6 null_message_processor_report.cpp6 null_message_processor_report.cpp
7 protobuf_message_processor.cpp7 message_processor.cpp
8 null_message_processor.cpp
9 surface_creation_parameters.cpp8 surface_creation_parameters.cpp
109
11 resource_cache.cpp10 resource_cache.cpp
@@ -13,8 +12,8 @@
13)12)
1413
15list(APPEND FRONTEND_SOURCES14list(APPEND FRONTEND_SOURCES
16 protobuf_socket_communicator.cpp15 socket_communicator.cpp
17 make_protobuf_socket_communicator.cpp16 make_socket_communicator.cpp
18 socket_session.cpp17 socket_session.cpp
19)18)
2019
2120
=== renamed file 'src/server/frontend/make_protobuf_socket_communicator.cpp' => 'src/server/frontend/make_socket_communicator.cpp'
--- src/server/frontend/make_protobuf_socket_communicator.cpp 2013-03-21 03:32:59 +0000
+++ src/server/frontend/make_socket_communicator.cpp 2013-04-01 21:52:22 +0000
@@ -18,7 +18,7 @@
1818
19#include "mir/default_server_configuration.h"19#include "mir/default_server_configuration.h"
20#include "mir/options/option.h"20#include "mir/options/option.h"
21#include "protobuf_socket_communicator.h"21#include "socket_communicator.h"
2222
23namespace mf = mir::frontend;23namespace mf = mir::frontend;
24namespace mg = mir::graphics;24namespace mg = mir::graphics;
@@ -31,7 +31,7 @@
31 [&,this]() -> std::shared_ptr<mf::Communicator>31 [&,this]() -> std::shared_ptr<mf::Communicator>
32 {32 {
33 auto const threads = the_options()->get("ipc-thread-pool", 10);33 auto const threads = the_options()->get("ipc-thread-pool", 10);
34 return std::make_shared<mf::ProtobufSocketCommunicator>(34 return std::make_shared<mf::SocketCommunicator>(
35 the_socket_file(),35 the_socket_file(),
36 the_ipc_factory(the_frontend_shell(), the_viewable_area(), the_buffer_allocator()),36 the_ipc_factory(the_frontend_shell(), the_viewable_area(), the_buffer_allocator()),
37 threads);37 threads);
3838
=== renamed file 'src/server/frontend/protobuf_message_processor.cpp' => 'src/server/frontend/message_processor.cpp'
--- src/server/frontend/protobuf_message_processor.cpp 2013-03-22 03:33:00 +0000
+++ src/server/frontend/message_processor.cpp 2013-04-01 21:52:22 +0000
@@ -16,7 +16,7 @@
16 * Authored by: Alan Griffiths <alan@octopull.co.uk>16 * Authored by: Alan Griffiths <alan@octopull.co.uk>
17 */17 */
1818
19#include "protobuf_message_processor.h"19#include "message_processor.h"
20#include "mir/frontend/message_processor_report.h"20#include "mir/frontend/message_processor_report.h"
21#include "mir/frontend/resource_cache.h"21#include "mir/frontend/resource_cache.h"
2222
@@ -26,7 +26,7 @@
2626
27namespace mfd = mir::frontend::detail;27namespace mfd = mir::frontend::detail;
2828
29mfd::ProtobufMessageProcessor::ProtobufMessageProcessor(29mfd::MessageProcessor::MessageProcessor(
30 MessageSender* sender,30 MessageSender* sender,
31 std::shared_ptr<protobuf::DisplayServer> const& display_server,31 std::shared_ptr<protobuf::DisplayServer> const& display_server,
32 std::shared_ptr<ResourceCache> const& resource_cache,32 std::shared_ptr<ResourceCache> const& resource_cache,
@@ -39,28 +39,28 @@
39}39}
4040
41template<class ResultMessage>41template<class ResultMessage>
42void mfd::ProtobufMessageProcessor::send_response(::google::protobuf::uint32 id, ResultMessage* response)42void mfd::MessageProcessor::send_response(::google::protobuf::uint32 id, ResultMessage* response)
43{43{
44 send_response(id, static_cast<google::protobuf::Message*>(response));44 send_response(id, static_cast<google::protobuf::Message*>(response));
45}45}
4646
47void mfd::ProtobufMessageProcessor::send_response(::google::protobuf::uint32 id, mir::protobuf::Buffer* response)47void mfd::MessageProcessor::send_response(::google::protobuf::uint32 id, mir::protobuf::Buffer* response)
48{48{
49 const auto& fd = extract_fds_from(response);49 const auto& fd = extract_fds_from(response);
50 send_response(id, static_cast<google::protobuf::Message*>(response));50 send_response(id, static_cast<google::protobuf::Message*>(response));
51 sender->send_fds(fd);51 sender->send_fds(fd);
52 resource_cache->free_resource(response);52 resource_cache->free_resource(response);
53}53}
5454
55void mfd::ProtobufMessageProcessor::send_response(::google::protobuf::uint32 id, mir::protobuf::Platform* response)55void mfd::MessageProcessor::send_response(::google::protobuf::uint32 id, mir::protobuf::Platform* response)
56{56{
57 const auto& fd = extract_fds_from(response);57 const auto& fd = extract_fds_from(response);
58 send_response(id, static_cast<google::protobuf::Message*>(response));58 send_response(id, static_cast<google::protobuf::Message*>(response));
59 sender->send_fds(fd);59 sender->send_fds(fd);
60 resource_cache->free_resource(response);60 resource_cache->free_resource(response);
61}61}
6262
63void mfd::ProtobufMessageProcessor::send_response(::google::protobuf::uint32 id, mir::protobuf::Connection* response)63void mfd::MessageProcessor::send_response(::google::protobuf::uint32 id, mir::protobuf::Connection* response)
64{64{
65 const auto& fd = response->has_platform() ?65 const auto& fd = response->has_platform() ?
66 extract_fds_from(response->mutable_platform()) :66 extract_fds_from(response->mutable_platform()) :
@@ -71,7 +71,7 @@
71 resource_cache->free_resource(response);71 resource_cache->free_resource(response);
72}72}
7373
74void mfd::ProtobufMessageProcessor::send_response(::google::protobuf::uint32 id, mir::protobuf::Surface* response)74void mfd::MessageProcessor::send_response(::google::protobuf::uint32 id, mir::protobuf::Surface* response)
75{75{
76 const auto& fd = response->has_buffer() ?76 const auto& fd = response->has_buffer() ?
77 extract_fds_from(response->mutable_buffer()) :77 extract_fds_from(response->mutable_buffer()) :
@@ -83,7 +83,7 @@
83}83}
8484
85template<class Response>85template<class Response>
86std::vector<int32_t> mfd::ProtobufMessageProcessor::extract_fds_from(Response* response)86std::vector<int32_t> mfd::MessageProcessor::extract_fds_from(Response* response)
87{87{
88 std::vector<int32_t> fd(response->fd().data(), response->fd().data() + response->fd().size());88 std::vector<int32_t> fd(response->fd().data(), response->fd().data() + response->fd().size());
89 response->clear_fd();89 response->clear_fd();
@@ -92,7 +92,7 @@
92}92}
9393
94template<class ParameterMessage, class ResultMessage>94template<class ParameterMessage, class ResultMessage>
95void mfd::ProtobufMessageProcessor::invoke(95void mfd::MessageProcessor::invoke(
96 void (protobuf::DisplayServer::*function)(96 void (protobuf::DisplayServer::*function)(
97 ::google::protobuf::RpcController* controller,97 ::google::protobuf::RpcController* controller,
98 const ParameterMessage* request,98 const ParameterMessage* request,
@@ -108,7 +108,7 @@
108 {108 {
109 std::unique_ptr<google::protobuf::Closure> callback(109 std::unique_ptr<google::protobuf::Closure> callback(
110 google::protobuf::NewPermanentCallback(this,110 google::protobuf::NewPermanentCallback(this,
111 &ProtobufMessageProcessor::send_response,111 &MessageProcessor::send_response,
112 invocation.id(),112 invocation.id(),
113 &result_message));113 &result_message));
114114
@@ -125,7 +125,7 @@
125 }125 }
126}126}
127127
128void mfd::ProtobufMessageProcessor::send_response(128void mfd::MessageProcessor::send_response(
129 ::google::protobuf::uint32 id,129 ::google::protobuf::uint32 id,
130 google::protobuf::Message* response)130 google::protobuf::Message* response)
131{131{
@@ -142,7 +142,7 @@
142 sender->send(buffer2);142 sender->send(buffer2);
143}143}
144144
145bool mfd::ProtobufMessageProcessor::dispatch(mir::protobuf::wire::Invocation const& invocation)145bool mfd::MessageProcessor::dispatch(mir::protobuf::wire::Invocation const& invocation)
146{146{
147 report->received_invocation(display_server.get(), invocation.id(), invocation.method_name());147 report->received_invocation(display_server.get(), invocation.id(), invocation.method_name());
148148
@@ -207,7 +207,7 @@
207}207}
208208
209209
210bool mfd::ProtobufMessageProcessor::process_message(std::istream& msg)210bool mfd::MessageProcessor::process_message(std::istream& msg)
211{211{
212 try212 try
213 {213 {
214214
=== renamed file 'src/server/frontend/protobuf_message_processor.h' => 'src/server/frontend/message_processor.h'
--- src/server/frontend/protobuf_message_processor.h 2013-03-22 03:33:00 +0000
+++ src/server/frontend/message_processor.h 2013-04-01 21:52:22 +0000
@@ -17,10 +17,10 @@
17 */17 */
1818
1919
20#ifndef MIR_FRONTEND_PROTOBUF_MESSAGE_PROCESSOR_H_20#ifndef MIR_FRONTEND_MESSAGE_PROCESSOR_H_
21#define MIR_FRONTEND_PROTOBUF_MESSAGE_PROCESSOR_H_21#define MIR_FRONTEND_MESSAGE_PROCESSOR_H_
2222
23#include "message_processor.h"23#include "message_sender.h"
2424
25#include "mir_protobuf.pb.h"25#include "mir_protobuf.pb.h"
26#include "mir_protobuf_wire.pb.h"26#include "mir_protobuf_wire.pb.h"
@@ -42,14 +42,16 @@
42namespace detail42namespace detail
43{43{
4444
45struct ProtobufMessageProcessor : MessageProcessor45struct MessageProcessor
46{46{
47 ProtobufMessageProcessor(47 MessageProcessor(
48 MessageSender* sender,48 MessageSender* sender,
49 std::shared_ptr<protobuf::DisplayServer> const& display_server,49 std::shared_ptr<protobuf::DisplayServer> const& display_server,
50 std::shared_ptr<ResourceCache> const& resource_cache,50 std::shared_ptr<ResourceCache> const& resource_cache,
51 std::shared_ptr<MessageProcessorReport> const& report);51 std::shared_ptr<MessageProcessorReport> const& report);
5252
53 bool process_message(std::istream& msg);
54
53private:55private:
54 void send_response(::google::protobuf::uint32 id, google::protobuf::Message* response);56 void send_response(::google::protobuf::uint32 id, google::protobuf::Message* response);
5557
@@ -75,8 +77,6 @@
75 template<class Response>77 template<class Response>
76 std::vector<int32_t> extract_fds_from(Response* response);78 std::vector<int32_t> extract_fds_from(Response* response);
7779
78 bool process_message(std::istream& msg);
79
80 bool dispatch(mir::protobuf::wire::Invocation const& invocation);80 bool dispatch(mir::protobuf::wire::Invocation const& invocation);
8181
82 template<class ParameterMessage, class ResultMessage>82 template<class ParameterMessage, class ResultMessage>
@@ -97,4 +97,4 @@
97}97}
98}98}
9999
100#endif /* PROTOBUF_MESSAGE_PROCESSOR_H_ */100#endif /* MIR_FRONTEND_MESSAGE_PROCESSOR_H_ */
101101
=== renamed file 'src/server/frontend/message_processor.h' => 'src/server/frontend/message_sender.h'
--- src/server/frontend/message_processor.h 2013-03-13 04:54:15 +0000
+++ src/server/frontend/message_sender.h 2013-04-01 21:52:22 +0000
@@ -17,8 +17,8 @@
17 */17 */
1818
1919
20#ifndef MIR_FRONTEND_MESSAGE_PROCESSOR_H_20#ifndef MIR_FRONTEND_MESSAGE_SENDER_H_
21#define MIR_FRONTEND_MESSAGE_PROCESSOR_H_21#define MIR_FRONTEND_MESSAGE_SENDER_H_
2222
23#include <vector>23#include <vector>
24#include <memory>24#include <memory>
@@ -41,26 +41,10 @@
41 MessageSender(MessageSender const&) = delete;41 MessageSender(MessageSender const&) = delete;
42 MessageSender& operator=(MessageSender const&) = delete;42 MessageSender& operator=(MessageSender const&) = delete;
43};43};
4444}
45struct MessageProcessor45}
46{46}
47 virtual bool process_message(std::istream& msg) = 0;47
48protected:48
49 MessageProcessor() = default;49
50 ~MessageProcessor() = default;50#endif /* MIR_FRONTEND_MESSAGE_SENDER_H_ */
51 MessageProcessor(MessageProcessor const&) = delete;
52 MessageProcessor& operator=(MessageProcessor const&) = delete;
53};
54
55struct NullMessageProcessor : MessageProcessor
56{
57 bool process_message(std::istream&);
58};
59
60}
61}
62}
63
64
65
66#endif /* PROTOBUF_MESSAGE_PROCESSOR_H_ */
6751
=== removed file 'src/server/frontend/null_message_processor.cpp'
--- src/server/frontend/null_message_processor.cpp 2013-03-13 04:54:15 +0000
+++ src/server/frontend/null_message_processor.cpp 1970-01-01 00:00:00 +0000
@@ -1,26 +0,0 @@
1/*
2 * Copyright © 2012 Canonical Ltd.
3 *
4 * This program is free software: you can redistribute it and/or modify it
5 * under the terms of the GNU Lesser General Public License version 3,
6 * as published by the Free Software Foundation.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU General Public License for more details.
12 *
13 * You should have received a copy of the GNU Lesser General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 *
16 * Authored by: Alan Griffiths <alan@octopull.co.uk>
17 */
18
19#include "message_processor.h"
20
21namespace mfd = mir::frontend::detail;
22
23bool mfd::NullMessageProcessor::process_message(std::istream& )
24{
25 return false;
26}
270
=== renamed file 'src/server/frontend/protobuf_socket_communicator.cpp' => 'src/server/frontend/socket_communicator.cpp'
--- src/server/frontend/protobuf_socket_communicator.cpp 2013-03-22 03:33:00 +0000
+++ src/server/frontend/socket_communicator.cpp 2013-04-01 21:52:22 +0000
@@ -16,8 +16,8 @@
16 * Authored by: Thomas Guest <thomas.guest@canonical.com>16 * Authored by: Thomas Guest <thomas.guest@canonical.com>
17 */17 */
1818
19#include "protobuf_socket_communicator.h"19#include "socket_communicator.h"
20#include "protobuf_message_processor.h"20#include "message_processor.h"
21#include "socket_session.h"21#include "socket_session.h"
2222
23#include "mir/frontend/protobuf_ipc_factory.h"23#include "mir/frontend/protobuf_ipc_factory.h"
@@ -31,7 +31,7 @@
31namespace ba = boost::asio;31namespace ba = boost::asio;
3232
3333
34mf::ProtobufSocketCommunicator::ProtobufSocketCommunicator(34mf::SocketCommunicator::SocketCommunicator(
35 std::string const& socket_file,35 std::string const& socket_file,
36 std::shared_ptr<ProtobufIpcFactory> const& ipc_factory,36 std::shared_ptr<ProtobufIpcFactory> const& ipc_factory,
37 int threads)37 int threads)
@@ -45,14 +45,14 @@
45 start_accept();45 start_accept();
46}46}
4747
48void mf::ProtobufSocketCommunicator::start_accept()48void mf::SocketCommunicator::start_accept()
49{49{
50 auto const& socket_session = std::make_shared<mfd::SocketSession>(50 auto const& socket_session = std::make_shared<mfd::SocketSession>(
51 io_service,51 io_service,
52 next_id(),52 next_id(),
53 connected_sessions);53 connected_sessions);
5454
55 auto session = std::make_shared<detail::ProtobufMessageProcessor>(55 auto session = std::make_shared<detail::MessageProcessor>(
56 socket_session.get(),56 socket_session.get(),
57 ipc_factory->make_ipc_server(),57 ipc_factory->make_ipc_server(),
58 ipc_factory->resource_cache(),58 ipc_factory->resource_cache(),
@@ -63,13 +63,13 @@
63 acceptor.async_accept(63 acceptor.async_accept(
64 socket_session->get_socket(),64 socket_session->get_socket(),
65 boost::bind(65 boost::bind(
66 &ProtobufSocketCommunicator::on_new_connection,66 &SocketCommunicator::on_new_connection,
67 this,67 this,
68 socket_session,68 socket_session,
69 ba::placeholders::error));69 ba::placeholders::error));
70}70}
7171
72int mf::ProtobufSocketCommunicator::next_id()72int mf::SocketCommunicator::next_id()
73{73{
74 int id = next_session_id.load();74 int id = next_session_id.load();
75 while (!next_session_id.compare_exchange_weak(id, id + 1)) std::this_thread::yield();75 while (!next_session_id.compare_exchange_weak(id, id + 1)) std::this_thread::yield();
@@ -77,7 +77,7 @@
77}77}
7878
7979
80void mf::ProtobufSocketCommunicator::start()80void mf::SocketCommunicator::start()
81{81{
82 auto run_io_service = boost::bind(&ba::io_service::run, &io_service);82 auto run_io_service = boost::bind(&ba::io_service::run, &io_service);
8383
@@ -87,7 +87,7 @@
87 }87 }
88}88}
8989
90mf::ProtobufSocketCommunicator::~ProtobufSocketCommunicator()90mf::SocketCommunicator::~SocketCommunicator()
91{91{
92 io_service.stop();92 io_service.stop();
9393
@@ -104,7 +104,7 @@
104 std::remove(socket_file.c_str());104 std::remove(socket_file.c_str());
105}105}
106106
107void mf::ProtobufSocketCommunicator::on_new_connection(107void mf::SocketCommunicator::on_new_connection(
108 std::shared_ptr<mfd::SocketSession> const& session,108 std::shared_ptr<mfd::SocketSession> const& session,
109 const boost::system::error_code& ec)109 const boost::system::error_code& ec)
110{110{
111111
=== renamed file 'src/server/frontend/protobuf_socket_communicator.h' => 'src/server/frontend/socket_communicator.h'
--- src/server/frontend/protobuf_socket_communicator.h 2013-03-21 03:32:59 +0000
+++ src/server/frontend/socket_communicator.h 2013-04-01 21:52:22 +0000
@@ -16,8 +16,8 @@
16 * Authored by: Thomas Guest <thomas.guest@canonical.com>16 * Authored by: Thomas Guest <thomas.guest@canonical.com>
17 */17 */
1818
19#ifndef MIR_FRONTEND_PROTOBUF_ASIO_COMMUNICATOR_H_19#ifndef MIR_FRONTEND_SOCKET_COMMUNICATOR_H_
20#define MIR_FRONTEND_PROTOBUF_ASIO_COMMUNICATOR_H_20#define MIR_FRONTEND_SOCKET_COMMUNICATOR_H_
2121
22#include "connected_sessions.h"22#include "connected_sessions.h"
2323
@@ -51,16 +51,16 @@
51struct SocketSession;51struct SocketSession;
52}52}
5353
54class ProtobufSocketCommunicator : public Communicator54class SocketCommunicator : public Communicator
55{55{
56public:56public:
57 // Create communicator based on Boost asio and Google protobufs57 // Create communicator based on Boost asio and Google protobufs
58 // using the supplied socket.58 // using the supplied socket.
59 explicit ProtobufSocketCommunicator(59 explicit SocketCommunicator(
60 const std::string& socket_file,60 const std::string& socket_file,
61 std::shared_ptr<ProtobufIpcFactory> const& ipc_factory,61 std::shared_ptr<ProtobufIpcFactory> const& ipc_factory,
62 int threads);62 int threads);
63 ~ProtobufSocketCommunicator();63 ~SocketCommunicator();
64 void start();64 void start();
6565
66private:66private:
@@ -80,4 +80,4 @@
80}80}
81}81}
8282
83#endif // MIR_FRONTEND_PROTOBUF_ASIO_COMMUNICATOR_H_83#endif // MIR_FRONTEND_SOCKET_COMMUNICATOR_H_
8484
=== modified file 'src/server/frontend/socket_session.cpp'
--- src/server/frontend/socket_session.cpp 2013-03-22 15:11:46 +0000
+++ src/server/frontend/socket_session.cpp 2013-04-01 21:52:22 +0000
@@ -89,7 +89,7 @@
8989
90 std::istream msg(&message);90 std::istream msg(&message);
9191
92 if (processor->process_message(msg))92 if (!processor || processor->process_message(msg))
93 {93 {
94 read_next_message();94 read_next_message();
95 }95 }
9696
=== modified file 'src/server/frontend/socket_session.h'
--- src/server/frontend/socket_session.h 2013-03-13 04:54:15 +0000
+++ src/server/frontend/socket_session.h 2013-04-01 21:52:22 +0000
@@ -40,8 +40,7 @@
40 std::shared_ptr<ConnectedSessions<SocketSession>> const& connected_sessions) :40 std::shared_ptr<ConnectedSessions<SocketSession>> const& connected_sessions) :
41 socket(io_service),41 socket(io_service),
42 id_(id_),42 id_(id_),
43 connected_sessions(connected_sessions),43 connected_sessions(connected_sessions) {}
44 processor(std::make_shared<NullMessageProcessor>()) {}
4544
46 int id() const { return id_; }45 int id() const { return id_; }
4746
4847
=== modified file 'tests/mir_test_doubles/test_protobuf_socket_server.cpp'
--- tests/mir_test_doubles/test_protobuf_socket_server.cpp 2013-03-13 08:09:52 +0000
+++ tests/mir_test_doubles/test_protobuf_socket_server.cpp 2013-04-01 21:52:22 +0000
@@ -17,7 +17,7 @@
17 */17 */
1818
19#include "mir_test/test_protobuf_server.h"19#include "mir_test/test_protobuf_server.h"
20#include "src/server/frontend/protobuf_socket_communicator.h"20#include "src/server/frontend/socket_communicator.h"
2121
22namespace mt = mir::test;22namespace mt = mir::test;
23namespace mtd = mir::test::doubles;23namespace mtd = mir::test::doubles;
@@ -34,5 +34,5 @@
3434
35std::shared_ptr<mf::Communicator> mt::TestProtobufServer::make_communicator(const std::string& socket_name, std::shared_ptr<frontend::ProtobufIpcFactory> const& factory)35std::shared_ptr<mf::Communicator> mt::TestProtobufServer::make_communicator(const std::string& socket_name, std::shared_ptr<frontend::ProtobufIpcFactory> const& factory)
36{36{
37 return std::make_shared<mf::ProtobufSocketCommunicator>(socket_name, factory, 10);37 return std::make_shared<mf::SocketCommunicator>(socket_name, factory, 10);
38}38}

Subscribers

People subscribed via source and target branches