Mir

Merge lp:~raof/mir/xserver-spawner into lp:mir

Proposed by Chris Halse Rogers
Status: Work in progress
Proposed branch: lp:~raof/mir/xserver-spawner
Merge into: lp:mir
Prerequisite: lp:~raof/mir/process-wrapper
Diff against target: 725 lines (+566/-2)
16 files modified
.clang-format (+1/-1)
include/server/mir/default_server_configuration.h (+11/-0)
include/server/mir/xserver/null_server_spawner.h (+25/-0)
include/server/mir/xserver/xserver_launcher.h (+59/-0)
src/server/CMakeLists.txt (+3/-1)
src/server/default_server_configuration.cpp (+6/-0)
src/server/xserver/CMakeLists.txt (+12/-0)
src/server/xserver/global_socket_listening_server_spawner.cpp (+71/-0)
src/server/xserver/global_socket_listening_server_spawner.h (+52/-0)
src/server/xserver/null_server_spawner.cpp (+34/-0)
tests/CMakeLists.txt (+1/-0)
tests/acceptance-tests/CMakeLists.txt (+2/-0)
tests/acceptance-tests/test_xserver_spawner.cpp (+73/-0)
tests/unit-tests/CMakeLists.txt (+2/-0)
tests/unit-tests/xserver/CMakeLists.txt (+8/-0)
tests/unit-tests/xserver/test_xserver_spawner.cpp (+206/-0)
To merge this branch: bzr merge lp:~raof/mir/xserver-spawner
Reviewer Review Type Date Requested Status
Andreas Pokorny (community) Needs Fixing
Daniel van Vugt Needs Fixing
Alan Griffiths Needs Information
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+203475@code.launchpad.net

This proposal supersedes a proposal from 2014-01-28.

Description of the change

A first cut at the infrastructure required for seamless nesting of X11 clients.

In order to do this we need to be able to spawn an Xorg server, wait until it's
ready to accept connections, connect a Mir X11 client to act as a bridging WM,
and *then* offer the X11 socket to clients.

This branch accomplishes the first two requirements.

I'm proposing it now because it's a reasonable checkpoint and I need to go and work on something else.

To post a comment you must log in.
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Please rethink class names ending in "er". That's almost always a bad decision, and an indication that a different cleaner design is possible...

http://www.benhallbenhall.com/2013/01/naming-objects-er-object-names/

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) :
review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

How much should Mir know about launching xservers?

Is it essential to all the uses we envisage: unity8? system compositor? Other (hypothetical at this time) shells?

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

could
+const char* mir::X::NullServerContext::client_connection_string()
return a string instead of a const char*?

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

> How much should Mir know about launching xservers?
>
> Is it essential to all the uses we envisage: unity8? system compositor? Other
> (hypothetical at this time) shells?

Last I heard from the architectural-level view is that lightdm was going to negotiate starting mir and friends and hooking up the FD's between them appropriately. That was something I heard a (relatively) long time ago though.

Revision history for this message
Chris Halse Rogers (raof) wrote :

@Alan:
I think most non-trivial shells are going to want to support running X11 apps. It's not interesting for system-compositor usage, true.

As to why Mir should know about launching xservers - the process of tying together foreign X11 windows needs Mir support. Mir will need to include an X11 window manager in order to proxy between X11 EWMH window management and Mir - this could be in U8, but any shell that wants to support X11 apps will need identical code.

Ideally you want to start the X11 WM before any app connects to the server to avoid unexpected behaviours; the easiest way to do this is to start the X server, wait for it to come up, connect the WM proxy, and only then consider the server to be started (that's the next part of X11 integration).

I (obviously) think it makes sense to have Mir know how to start the X server.
@kdub:
This is somewhat of a circular dependency - the X server needs Mir running in order to be started (it needs the mir socket), and Mir's X11 WM needs the X server to be running. You can't easily start both from LightDM, as it would need to start Mir, it to be ready to accept connections, then start the X server.

Also, U8 wants to (a) start an X server only when an app needs it, and (b) start one-Xserver-per-app.

re: client_connection_string - Yeah, it could return a std::string (and did, at one point). I switched to char const* because I only ever wanted to consume it from things that took a char const*, but can easily switch it back.

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

(1) "er" issues as described above.

(2) I'm concerned about mentioning "X"-anything in the Mir source too. It feels like we're preventing Mir from being a success in its own right if it even has to mention X in its source. Even if the coupling is weak, we should strive for a better answer. Generalize, move it out-of-project, etc. We almost certainly don't want an "X" namespace. That's a reasonable indication that we're going in the wrong direction.

(3) You've introduced a build-dep on libx11-dev:
  +#include <X11/Xlib.h>
Definitely don't do that, at least :)

review: Needs Fixing
Revision history for this message
Chris Halse Rogers (raof) wrote :

On Wed, 2014-01-29 at 03:09 +0000, Daniel van Vugt wrote:
> Review: Needs Fixing
>
> (1) "er" issues as described above.

Would you prefer XServerFactory?

> (2) I'm concerned about mentioning "X"-anything in the Mir source too. It feels like we're preventing Mir from being a success in its own right if it even has to mention X in its source. Even if the coupling is weak, we should strive for a better answer. Generalize, move it out-of-project, etc. We almost certainly don't want an "X" namespace. That's a reasonable indication that we're going in the wrong direction.

We're going to have to support running X11 apps for the foreseeable
future. Certainly for the life of 14.04, probably for the life of 16.04.

I don't think it's unreasonable for the Mir codebase to reflect the
importance of legacy X11 support.

Indeed, the actual implementations should be in a separate opt-in
libmirserverX11integration.so library, and will be once they start to
depend on X11 libraries.

>
> (3) You've introduced a build-dep on libx11-dev:
> +#include <X11/Xlib.h>
> Definitely don't do that, at least :)
>

Ah, yeah. This is currently only for the (disabled) acceptance test.
Once there's mirserver code that depends on X11 libs it'll be optional
and in a separate library.

Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

@er:
I still dont get the argument about "er" - If RAOF renames "XServerSpawner" into "XServerFactory" and people are satisfied then there is something wrong with that rule (of thumb?).
We then switched from a noun that was derived from a verb that meant something like: "creates and launches a process or activity" to something that only means: "creates something". So from a clearer specific term to a generic one. A specific term that describes what should be in this class and what not - A term that suggest that this is actually a very specific functor that launches an XServer.

So I would prefer XServerSpawner::create_server over XServerFactory::create_server. Something like is XServerFactory::spawn_server also a good name - I just dont get the reasoning with the "no-er"-rule.

@library: I do believe that this functionality is something a current linux compositor framework needs to provide, but probably as a separate library. -> Needs Fixing

@clang-format: thanks for fixing!

review: Needs Fixing

Unmerged revisions

1349. By Chris Halse Rogers

clang-format: We always split constructor initialisers on new lines

1348. By Chris Halse Rogers

Disable X11ClientConnects acceptance test.

This needs a bit more fiddling - it all roughly works, but in the acceptance
test framework we don't have a real graphics card, so XMir currently fails.

Needs at least an xorg.conf specifying the dummy graphics driver

1347. By Chris Halse Rogers

Get the Xserver to connect to a Mir socket

1346. By Chris Halse Rogers

Update for process::Spawner API change

1345. By Chris Halse Rogers

Merged subprocess-wrapper into xserver-spawner.

1344. By Chris Halse Rogers

X::ServerSpawner::create_server must take a shared_ptr<process::Spawner>

create_server does things asynchronously, so it has to take shared ownership
of the process::Spawner it's using

1343. By Chris Halse Rogers

Rejigger X::ServerSpawner API.

spawn_server() now returns a future<ServerHandle> that will become a present ServerHandle once
the server is ready, rather than returning a ServerHandle now that has methods that will only
work in the future.

1342. By Chris Halse Rogers

Implement -displayfd handling for GlobalSocketListeningServerSpawner.

client_connection_string now returns a string that should be associated with an Xorg server that's ready to accept connections

1341. By Chris Halse Rogers

Merge process::Spawner work required to do Xserver startup properly

1340. By Chris Halse Rogers

More basic unittests for GlobalSocketListeningServerSpawner

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file '.clang-format'
2--- .clang-format 2014-01-10 15:48:08 +0000
3+++ .clang-format 2014-01-28 06:35:36 +0000
4@@ -14,7 +14,7 @@
5 BreakConstructorInitializersBeforeComma: false
6 BinPackParameters: true
7 ColumnLimit: 120
8-ConstructorInitializerAllOnOneLineOrOnePerLine: false
9+ConstructorInitializerAllOnOneLineOrOnePerLine: true
10 DerivePointerBinding: true
11 ExperimentalAutoDetectBinPacking: true
12 IndentCaseLabels: false
13
14=== modified file 'include/server/mir/default_server_configuration.h'
15--- include/server/mir/default_server_configuration.h 2014-01-21 15:29:52 +0000
16+++ include/server/mir/default_server_configuration.h 2014-01-28 06:35:36 +0000
17@@ -113,6 +113,11 @@
18 class Logger;
19 }
20
21+namespace X
22+{
23+class ServerSpawner;
24+}
25+
26 class DefaultServerConfiguration : public virtual ServerConfiguration, DefaultConfigurationOptions
27 {
28 public:
29@@ -232,6 +237,12 @@
30
31 virtual std::shared_ptr<time::Clock> the_clock();
32
33+ /** @name X11 server integration - customization
34+ * configurable interfaces for integration with legacy X11 apps
35+ * @{ */
36+ virtual std::shared_ptr<X::ServerSpawner> the_xserver_spawner();
37+ /** @} */
38+
39 protected:
40 using DefaultConfigurationOptions::the_options;
41 using DefaultConfigurationOptions::add_options;
42
43=== added directory 'include/server/mir/xserver'
44=== added file 'include/server/mir/xserver/null_server_spawner.h'
45--- include/server/mir/xserver/null_server_spawner.h 1970-01-01 00:00:00 +0000
46+++ include/server/mir/xserver/null_server_spawner.h 2014-01-28 06:35:36 +0000
47@@ -0,0 +1,25 @@
48+#ifndef MIR_X_NULL_SERVER_SPAWNER_H_
49+#define MIR_X_NULL_SERVER_SPAWNER_H_
50+
51+#include <mir/xserver/xserver_launcher.h>
52+
53+namespace mir
54+{
55+namespace X
56+{
57+class NullServerContext : public ServerContext
58+{
59+public:
60+ char const* client_connection_string() override;
61+};
62+
63+class NullServerSpawner : public ServerSpawner
64+{
65+public:
66+ std::future<std::unique_ptr<ServerContext>> create_server(std::shared_ptr<process::Spawner> const& spawner, std::shared_ptr<mir::frontend::Connector> const& connector) override;
67+};
68+
69+}
70+}
71+
72+#endif // MIR_X_NULL_SERVER_SPAWNER_H_
73
74=== added file 'include/server/mir/xserver/xserver_launcher.h'
75--- include/server/mir/xserver/xserver_launcher.h 1970-01-01 00:00:00 +0000
76+++ include/server/mir/xserver/xserver_launcher.h 2014-01-28 06:35:36 +0000
77@@ -0,0 +1,59 @@
78+/*
79+ * Copyright © 2014 Canonical Ltd.
80+ *
81+ * This program is free software: you can redistribute it and/or modify
82+ * it under the terms of the GNU General Public License version 3 as
83+ * published by the Free Software Foundation.
84+ *
85+ * This program is distributed in the hope that it will be useful,
86+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
87+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
88+ * GNU General Public License for more details.
89+ *
90+ * You should have received a copy of the GNU General Public License
91+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
92+ *
93+ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
94+ */
95+
96+#ifndef MIR_X_XSERVER_LAUNCHER_H_
97+#define MIR_X_XSERVER_LAUNCHER_H_
98+
99+#include <future>
100+#include <memory>
101+
102+#include "mir/process/spawner.h"
103+#include "mir/frontend/connector.h"
104+
105+namespace mir
106+{
107+namespace X
108+{
109+
110+class ServerContext
111+{
112+public:
113+ virtual ~ServerContext() = default;
114+
115+ /**
116+ * \brief Get the XLib connection string to connect to this server
117+ *
118+ * This string can be passed by the client to XOpenDisplay to connect
119+ * to this server instance, or set in the DISPLAY environment variable
120+ * to be used as the default display.
121+ */
122+ virtual char const* client_connection_string() = 0;
123+};
124+
125+class ServerSpawner
126+{
127+public:
128+ virtual ~ServerSpawner() = default;
129+
130+ virtual std::future<std::unique_ptr<ServerContext>> create_server(std::shared_ptr<mir::process::Spawner> const& spawner,
131+ std::shared_ptr<mir::frontend::Connector> const& connector) = 0;
132+};
133+}
134+}
135+
136+#endif // MIR_X_XSERVER_LAUNCHER_H_
137
138=== modified file 'src/server/CMakeLists.txt'
139--- src/server/CMakeLists.txt 2014-01-28 06:35:36 +0000
140+++ src/server/CMakeLists.txt 2014-01-28 06:35:36 +0000
141@@ -13,6 +13,7 @@
142 add_subdirectory(shell/)
143 add_subdirectory(time/)
144 add_subdirectory(process/)
145+add_subdirectory(xserver/)
146
147 set(PREFIX "${CMAKE_INSTALL_PREFIX}")
148 set(EXEC_PREFIX "${CMAKE_INSTALL_PREFIX}")
149@@ -57,8 +58,9 @@
150 mirlttng
151 mirnestedgraphics
152 miroffscreengraphics
153+ mirsharedpipe
154 mirprocess
155- mirsharedpipe
156+ mirxserverintegration
157 )
158
159 list(APPEND MIRSERVER_LINKS
160
161=== modified file 'src/server/default_server_configuration.cpp'
162--- src/server/default_server_configuration.cpp 2014-01-13 06:12:33 +0000
163+++ src/server/default_server_configuration.cpp 2014-01-28 06:35:36 +0000
164@@ -42,6 +42,7 @@
165 #include "mir/time/high_resolution_clock.h"
166 #include "mir/geometry/rectangles.h"
167 #include "mir/default_configuration.h"
168+#include "mir/xserver/null_server_spawner.h"
169
170 #include <map>
171
172@@ -249,3 +250,8 @@
173 return std::make_shared<mir::DefaultServerStatusListener>();
174 });
175 }
176+
177+std::shared_ptr<mir::X::ServerSpawner> mir::DefaultServerConfiguration::the_xserver_spawner()
178+{
179+ return std::make_shared<mir::X::NullServerSpawner>();
180+}
181
182=== added directory 'src/server/xserver'
183=== added file 'src/server/xserver/CMakeLists.txt'
184--- src/server/xserver/CMakeLists.txt 1970-01-01 00:00:00 +0000
185+++ src/server/xserver/CMakeLists.txt 2014-01-28 06:35:36 +0000
186@@ -0,0 +1,12 @@
187+set(
188+ MIR_XSERVER_SRCS
189+
190+ global_socket_listening_server_spawner.cpp
191+ null_server_spawner.cpp
192+)
193+
194+ADD_LIBRARY(
195+ mirxserverintegration STATIC
196+
197+ ${MIR_XSERVER_SRCS}
198+)
199
200=== added file 'src/server/xserver/global_socket_listening_server_spawner.cpp'
201--- src/server/xserver/global_socket_listening_server_spawner.cpp 1970-01-01 00:00:00 +0000
202+++ src/server/xserver/global_socket_listening_server_spawner.cpp 2014-01-28 06:35:36 +0000
203@@ -0,0 +1,71 @@
204+/*
205+ * Copyright © 2014 Canonical Ltd.
206+ *
207+ * This program is free software: you can redistribute it and/or modify
208+ * it under the terms of the GNU General Public License version 3 as
209+ * published by the Free Software Foundation.
210+ *
211+ * This program is distributed in the hope that it will be useful,
212+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
213+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
214+ * GNU General Public License for more details.
215+ *
216+ * You should have received a copy of the GNU General Public License
217+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
218+ *
219+ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
220+ */
221+
222+#include <unistd.h>
223+#include <errno.h>
224+
225+#include <boost/throw_exception.hpp>
226+#include <boost/exception/errinfo_errno.hpp>
227+
228+#include "global_socket_listening_server_spawner.h"
229+#include "mir/process/handle.h"
230+
231+namespace mx = mir::X;
232+
233+mx::GlobalSocketListeningServerContext::GlobalSocketListeningServerContext(std::unique_ptr<mir::process::Handle> server_handle, std::string connection_string)
234+ : server_handle(std::move(server_handle)),
235+ connection_string(connection_string)
236+{
237+}
238+
239+char const* mx::GlobalSocketListeningServerContext::client_connection_string()
240+{
241+ return connection_string.c_str();
242+}
243+
244+std::future<std::unique_ptr<mx::ServerContext>> mx::GlobalSocketListeningServerSpawner::create_server(std::shared_ptr<mir::process::Spawner> const& spawner, std::shared_ptr<mir::frontend::Connector> const& connector)
245+{
246+ return std::async(std::launch::async, [spawner, connector]()
247+ {
248+ mir::pipe::Pipe displayfd_pipe;
249+ auto displayfd = std::to_string(displayfd_pipe.write_fd());
250+ int mir_fd = connector->client_socket_fd();
251+ auto mir_fd_arg = std::string("fd://") + std::to_string(mir_fd);
252+
253+ auto future_handle = spawner->run_from_path("Xorg",
254+ {"-displayfd", displayfd.c_str(),
255+ "-mir", "xserver",
256+ "-mirSocket", mir_fd_arg.c_str()},
257+ {displayfd_pipe.write_fd(), mir_fd});
258+
259+ char display_number[10];
260+ errno = 0;
261+ int bytes_read = read(displayfd_pipe.read_fd(), display_number, sizeof display_number);
262+
263+ while (bytes_read == -1 && errno == EINTR)
264+ bytes_read = read(displayfd_pipe.read_fd(), display_number, sizeof display_number);;
265+
266+ if (errno != 0)
267+ BOOST_THROW_EXCEPTION(boost::enable_error_info(std::runtime_error("Failed to receive display number from Xserver"))
268+ << boost::errinfo_errno(errno));
269+
270+ display_number[bytes_read] = '\0';
271+
272+ return std::unique_ptr<mx::ServerContext>(new mx::GlobalSocketListeningServerContext(future_handle.get(), std::string(":") + display_number));
273+ });
274+}
275
276=== added file 'src/server/xserver/global_socket_listening_server_spawner.h'
277--- src/server/xserver/global_socket_listening_server_spawner.h 1970-01-01 00:00:00 +0000
278+++ src/server/xserver/global_socket_listening_server_spawner.h 2014-01-28 06:35:36 +0000
279@@ -0,0 +1,52 @@
280+/*
281+ * Copyright © 2014 Canonical Ltd.
282+ *
283+ * This program is free software: you can redistribute it and/or modify
284+ * it under the terms of the GNU General Public License version 3 as
285+ * published by the Free Software Foundation.
286+ *
287+ * This program is distributed in the hope that it will be useful,
288+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
289+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
290+ * GNU General Public License for more details.
291+ *
292+ * You should have received a copy of the GNU General Public License
293+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
294+ *
295+ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
296+ */
297+
298+#ifndef MIR_X_GLOBAL_SOCKET_LISTENING_SERVER_SPAWNER_H_
299+#define MIR_X_GLOBAL_SOCKET_LISTENING_SERVER_SPAWNER_H_
300+
301+#include "mir/xserver/xserver_launcher.h"
302+#include "mir/process/spawner.h"
303+#include "mir/pipe.h"
304+
305+#include <memory>
306+
307+namespace mir
308+{
309+namespace X
310+{
311+class GlobalSocketListeningServerContext : public ServerContext
312+{
313+public:
314+ GlobalSocketListeningServerContext(std::unique_ptr<mir::process::Handle> server_handle, std::string connection_string);
315+ char const* client_connection_string() override;
316+
317+private:
318+ std::unique_ptr<mir::process::Handle> server_handle;
319+ std::string connection_string;
320+};
321+
322+class GlobalSocketListeningServerSpawner : public ServerSpawner
323+{
324+public:
325+ std::future<std::unique_ptr<ServerContext>> create_server(std::shared_ptr<mir::process::Spawner> const& spawner, std::shared_ptr<mir::frontend::Connector> const& connector) override;
326+};
327+
328+}
329+}
330+
331+#endif // MIR_X_GLOBAL_SOCKET_LISTENING_SERVER_SPAWNER_H_
332
333=== added file 'src/server/xserver/null_server_spawner.cpp'
334--- src/server/xserver/null_server_spawner.cpp 1970-01-01 00:00:00 +0000
335+++ src/server/xserver/null_server_spawner.cpp 2014-01-28 06:35:36 +0000
336@@ -0,0 +1,34 @@
337+/*
338+ * Copyright © 2014 Canonical Ltd.
339+ *
340+ * This program is free software: you can redistribute it and/or modify
341+ * it under the terms of the GNU General Public License version 3 as
342+ * published by the Free Software Foundation.
343+ *
344+ * This program is distributed in the hope that it will be useful,
345+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
346+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
347+ * GNU General Public License for more details.
348+ *
349+ * You should have received a copy of the GNU General Public License
350+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
351+ *
352+ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
353+ */
354+
355+#include "mir/xserver/null_server_spawner.h"
356+
357+const char* mir::X::NullServerContext::client_connection_string()
358+{
359+ return "";
360+}
361+
362+std::future<std::unique_ptr<mir::X::ServerContext>> mir::X::NullServerSpawner::create_server(
363+ std::shared_ptr<mir::process::Spawner> const& unused, std::shared_ptr<mir::frontend::Connector> const& unuseder)
364+{
365+ static_cast<void>(unused);
366+ static_cast<void>(unuseder);
367+ std::promise<std::unique_ptr<mir::X::ServerContext>> boring_promise;
368+ boring_promise.set_value(std::unique_ptr<mir::X::ServerContext>(new mir::X::NullServerContext));
369+ return boring_promise.get_future();
370+}
371
372=== modified file 'tests/CMakeLists.txt'
373--- tests/CMakeLists.txt 2014-01-23 08:06:05 +0000
374+++ tests/CMakeLists.txt 2014-01-28 06:35:36 +0000
375@@ -36,6 +36,7 @@
376
377 if (MIR_BUILD_ACCEPTANCE_TESTS)
378 add_subdirectory(acceptance-tests/)
379+ pkg_check_modules(X11 REQUIRED x11)
380 endif (MIR_BUILD_ACCEPTANCE_TESTS)
381
382 if (MIR_BUILD_INTEGRATION_TESTS)
383
384=== modified file 'tests/acceptance-tests/CMakeLists.txt'
385--- tests/acceptance-tests/CMakeLists.txt 2014-01-28 06:35:36 +0000
386+++ tests/acceptance-tests/CMakeLists.txt 2014-01-28 06:35:36 +0000
387@@ -28,6 +28,7 @@
388 test_client_library_drm.cpp
389 test_protobuf.cpp
390 test_subprocess.cpp
391+ test_xserver_spawner.cpp
392 ${GENERATED_PROTOBUF_SRCS}
393 ${GENERATED_PROTOBUF_HDRS}
394 )
395@@ -66,6 +67,7 @@
396 ${GMOCK_LIBRARY}
397 ${GMOCK_MAIN_LIBRARY}
398 ${CMAKE_THREAD_LIBS_INIT} # Link in pthread.
399+ ${X11_LDFLAGS}
400 )
401
402 CMAKE_DEPENDENT_OPTION(
403
404=== added file 'tests/acceptance-tests/test_xserver_spawner.cpp'
405--- tests/acceptance-tests/test_xserver_spawner.cpp 1970-01-01 00:00:00 +0000
406+++ tests/acceptance-tests/test_xserver_spawner.cpp 2014-01-28 06:35:36 +0000
407@@ -0,0 +1,73 @@
408+/*
409+ * Copyright © 2014 Canonical Ltd.
410+ *
411+ * This program is free software: you can redistribute it and/or modify
412+ * it under the terms of the GNU General Public License version 3 as
413+ * published by the Free Software Foundation.
414+ *
415+ * This program is distributed in the hope that it will be useful,
416+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
417+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
418+ * GNU General Public License for more details.
419+ *
420+ * You should have received a copy of the GNU General Public License
421+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
422+ *
423+ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
424+ */
425+
426+#include "src/server/xserver/global_socket_listening_server_spawner.h"
427+#include "mir_test_framework/testing_server_configuration.h"
428+#include "mir_test_framework/in_process_server.h"
429+#include "mir/xserver/xserver_launcher.h"
430+#include "src/server/process/fork_spawner.h"
431+
432+#include <X11/Xlib.h>
433+#include <stdlib.h>
434+
435+#include <gtest/gtest.h>
436+
437+namespace mtf = mir_test_framework;
438+namespace mx = mir::X;
439+
440+namespace
441+{
442+struct XserverSpawningServer : public mtf::InProcessServer
443+{
444+public:
445+ std::shared_ptr<mx::ServerSpawner> the_xserver_spawner()
446+ {
447+ return config.the_xserver_spawner();
448+ }
449+
450+ mir::DefaultServerConfiguration& server_config() override
451+ {
452+ return config;
453+ }
454+
455+ class SocketListeningXServerConfig : public mtf::StubbedServerConfiguration
456+ {
457+ public:
458+ std::shared_ptr<mx::ServerSpawner> the_xserver_spawner() override
459+ {
460+ return std::make_shared<mx::GlobalSocketListeningServerSpawner> ();
461+ }
462+ } config;
463+};
464+}
465+
466+// This requires a bit more fiddling before it will work.
467+// Particularly, it needs an xorg.conf that specifies dummy
468+// devices, so the real devices don't fail to load.
469+TEST_F(XserverSpawningServer, DISABLED_X11ClientConnects)
470+{
471+ // Ensure the surrounding environment doesn't mess with the test
472+ unsetenv("DISPLAY");
473+
474+ auto xserver = the_xserver_spawner()->create_server(std::make_shared<mir::process::ForkSpawner>(), config.the_connector());
475+ Display* disp = XOpenDisplay(xserver.get()->client_connection_string());
476+
477+ ASSERT_TRUE(disp != NULL);
478+
479+ XCloseDisplay(disp);
480+}
481
482=== modified file 'tests/unit-tests/CMakeLists.txt'
483--- tests/unit-tests/CMakeLists.txt 2014-01-28 06:35:36 +0000
484+++ tests/unit-tests/CMakeLists.txt 2014-01-28 06:35:36 +0000
485@@ -30,6 +30,7 @@
486 add_subdirectory(android_input/)
487 add_subdirectory(scene/)
488 add_subdirectory(draw/)
489+add_subdirectory(xserver/)
490
491 add_executable(mir_unit_tests ${UNIT_TEST_SOURCES})
492 uses_android_input(mir_unit_tests)
493@@ -43,6 +44,7 @@
494 mirdraw
495 mirtestdraw
496 mirlogging
497+ mirxserverintegration
498
499 mir-test
500 mir-test-doubles
501
502=== added directory 'tests/unit-tests/xserver'
503=== added file 'tests/unit-tests/xserver/CMakeLists.txt'
504--- tests/unit-tests/xserver/CMakeLists.txt 1970-01-01 00:00:00 +0000
505+++ tests/unit-tests/xserver/CMakeLists.txt 2014-01-28 06:35:36 +0000
506@@ -0,0 +1,8 @@
507+list(APPEND UNIT_TEST_SOURCES
508+ ${CMAKE_CURRENT_SOURCE_DIR}/test_xserver_spawner.cpp
509+)
510+
511+set(
512+ UNIT_TEST_SOURCES
513+ ${UNIT_TEST_SOURCES}
514+ PARENT_SCOPE)
515
516=== added file 'tests/unit-tests/xserver/test_xserver_spawner.cpp'
517--- tests/unit-tests/xserver/test_xserver_spawner.cpp 1970-01-01 00:00:00 +0000
518+++ tests/unit-tests/xserver/test_xserver_spawner.cpp 2014-01-28 06:35:36 +0000
519@@ -0,0 +1,206 @@
520+/*
521+ * Copyright © 2014 Canonical Ltd.
522+ *
523+ * This program is free software: you can redistribute it and/or modify
524+ * it under the terms of the GNU General Public License version 3 as
525+ * published by the Free Software Foundation.
526+ *
527+ * This program is distributed in the hope that it will be useful,
528+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
529+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
530+ * GNU General Public License for more details.
531+ *
532+ * You should have received a copy of the GNU General Public License
533+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
534+ *
535+ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
536+ */
537+
538+#include <future>
539+#include <vector>
540+#include <thread>
541+#include <condition_variable>
542+
543+#include <gtest/gtest.h>
544+#include <gmock/gmock.h>
545+
546+#include "mir/process/spawner.h"
547+#include "mir/process/handle.h"
548+
549+#include "src/server/xserver/global_socket_listening_server_spawner.h"
550+
551+using namespace ::testing;
552+
553+namespace
554+{
555+struct MockConnector : public mir::frontend::Connector
556+{
557+ MOCK_METHOD0(start, void(void));
558+ MOCK_METHOD0(stop, void(void));
559+ MOCK_CONST_METHOD0(client_socket_fd, int(void));
560+ MOCK_CONST_METHOD0(remove_endpoint, void(void));
561+};
562+
563+struct MockProcessSpawner : public mir::process::Spawner
564+{
565+ MOCK_CONST_METHOD3(run, mir::process::Handle*(std::string, std::vector<char const*>, std::vector<int>));
566+
567+ std::future<std::unique_ptr<mir::process::Handle>> run_from_path(char const* binary) const override
568+ {
569+ std::promise<std::unique_ptr<mir::process::Handle>> dummy_promise;
570+ dummy_promise.set_value(std::unique_ptr<mir::process::Handle>(
571+ run(binary, std::initializer_list<char const*>(), std::initializer_list<int>())));
572+ return dummy_promise.get_future();
573+ }
574+ std::future<std::unique_ptr<mir::process::Handle>> run_from_path(char const* binary,
575+ std::initializer_list<char const*> args) const
576+ override
577+ {
578+ std::promise<std::unique_ptr<mir::process::Handle>> dummy_promise;
579+ dummy_promise.set_value(std::unique_ptr<mir::process::Handle>(run(binary, args, std::initializer_list<int>())));
580+ return dummy_promise.get_future();
581+ }
582+
583+ std::future<std::unique_ptr<mir::process::Handle>> run_from_path(char const* binary,
584+ std::initializer_list<char const*> args,
585+ std::initializer_list<int> fds) const override
586+ {
587+ std::promise<std::unique_ptr<mir::process::Handle>> dummy_promise;
588+ dummy_promise.set_value(std::unique_ptr<mir::process::Handle>(run(binary, args, fds)));
589+ return dummy_promise.get_future();
590+ }
591+};
592+
593+struct SocketListeningServerTest : public testing::Test
594+{
595+ SocketListeningServerTest()
596+ : default_server_number("100"),
597+ spawner(std::make_shared<NiceMock<MockProcessSpawner>>()),
598+ connector(std::make_shared<NiceMock<MockConnector>>())
599+ {
600+ ON_CALL(*spawner, run(_, _, _))
601+ .WillByDefault(DoAll(SaveArg<0>(&binary),
602+ SaveArg<1>(&args),
603+ SaveArg<2>(&fds),
604+ InvokeWithoutArgs([this]()
605+ { write_server_string(default_server_number); }),
606+ Return(nullptr)));
607+ ON_CALL(*connector, client_socket_fd())
608+ .WillByDefault(Return(22));
609+ }
610+
611+ void write_server_string(std::string server_number)
612+ {
613+ auto location = std::find_if(args.begin(), args.end(), [](char const* a)
614+ { return strcmp(a, "-displayfd") == 0; });
615+ ASSERT_NE(location, args.end());
616+ ASSERT_NE(++location, args.end());
617+ int server_fd = atoi(*location);
618+ write(server_fd, server_number.data(), server_number.length());
619+ close(server_fd);
620+ }
621+
622+ std::string default_server_number;
623+ std::string binary;
624+ std::vector<char const*> args;
625+ std::vector<int> fds;
626+ std::shared_ptr<NiceMock<MockProcessSpawner>> spawner;
627+ std::shared_ptr<NiceMock<MockConnector>> connector;
628+};
629+}
630+
631+TEST_F(SocketListeningServerTest, CreateServerAlwaysValid)
632+{
633+ mir::X::GlobalSocketListeningServerSpawner factory;
634+
635+ auto server_context = factory.create_server(spawner, connector);
636+ ASSERT_NE(server_context.get(), nullptr);
637+}
638+
639+TEST_F(SocketListeningServerTest, SpawnsCorrectExecutable)
640+{
641+ mir::X::GlobalSocketListeningServerSpawner factory;
642+
643+ auto server_context = factory.create_server(spawner, connector);
644+ server_context.get();
645+
646+ EXPECT_EQ(binary, "Xorg");
647+}
648+
649+namespace
650+{
651+MATCHER_P(ContainsSubsequence, subsequence, "")
652+{
653+ auto location =
654+ std::search(arg.begin(), arg.end(), subsequence.begin(), subsequence.end(), [](char const* a, std::string b)
655+ { return strcmp(a, b.c_str()) == 0; });
656+ return location != arg.end();
657+}
658+}
659+
660+TEST_F(SocketListeningServerTest, SpawnsWithDisplayFDSet)
661+{
662+ mir::X::GlobalSocketListeningServerSpawner factory;
663+
664+ auto server_context = factory.create_server(spawner, connector);
665+ server_context.get();
666+
667+ ASSERT_THAT(args, Not(IsEmpty()));
668+ ASSERT_THAT(fds, Not(IsEmpty()));
669+
670+ Matcher<std::vector<char const*>> fd_matcher = Not(_);
671+ for (auto fd : fds)
672+ {
673+ fd_matcher = AnyOf(fd_matcher, ContainsSubsequence(std::vector<std::string>{"-displayfd", std::to_string(fd)}));
674+ }
675+ EXPECT_THAT(args, fd_matcher);
676+}
677+
678+TEST_F(SocketListeningServerTest, ReturnsCorrectDisplayString)
679+{
680+ mir::X::GlobalSocketListeningServerSpawner factory;
681+
682+ default_server_number = "20";
683+ auto server_context = factory.create_server(spawner, connector);
684+
685+ EXPECT_STREQ(":20", server_context.get()->client_connection_string());
686+}
687+
688+TEST_F(SocketListeningServerTest, HandlesSpawnerLifecycleCorrectly)
689+{
690+ mir::X::GlobalSocketListeningServerSpawner factory;
691+
692+ std::future<std::unique_ptr<mir::X::ServerContext>> server_context;
693+
694+ {
695+ auto tmp_spawner = std::make_shared<MockProcessSpawner>();
696+ ON_CALL(*tmp_spawner, run(_, _, _))
697+ .WillByDefault(DoAll(SaveArg<0>(&binary),
698+ SaveArg<1>(&args),
699+ SaveArg<2>(&fds),
700+ InvokeWithoutArgs([this]()
701+ { write_server_string(default_server_number); }),
702+ Return(nullptr)));
703+ EXPECT_CALL(*tmp_spawner, run(_, _, _));
704+ server_context = factory.create_server(tmp_spawner, connector);
705+ }
706+
707+ ASSERT_NE(server_context.get(), nullptr);
708+}
709+
710+TEST_F(SocketListeningServerTest, PassesMirSocketCorrectly)
711+{
712+ mir::X::GlobalSocketListeningServerSpawner factory;
713+
714+ int const mir_fd{32};
715+ std::string const mir_connect_str{std::string("fd://") + std::to_string(mir_fd)};
716+ EXPECT_CALL(*connector, client_socket_fd())
717+ .WillOnce(Return(mir_fd));
718+
719+ auto server_context = factory.create_server(spawner, connector);
720+
721+ server_context.get();
722+
723+ EXPECT_THAT(args, ContainsSubsequence(std::vector<std::string>{"-mirSocket", mir_connect_str}));
724+ EXPECT_THAT(fds, Contains(Eq(mir_fd)));
725+}

Subscribers

People subscribed via source and target branches