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

[Chris Halse Rogers]

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.

[Daniel van Vugt (vanvugt)]

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

[Daniel van Vugt (vanvugt)]

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1349
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/~raof/mir/xserver-spawner/+merge/203475/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/738/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/752/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/749/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/353/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/468/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/472/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/354/console

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

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

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

[Kevin DuBois (kdub)]

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

[Kevin DuBois (kdub)]

> 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.

[Chris Halse Rogers (raof)]

@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.

[Daniel van Vugt (vanvugt)]

(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

[Chris Halse Rogers (raof)]

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.

[Andreas Pokorny (andreas-pokorny)]

@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 on 2014-01-28

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

1348. By Chris Halse Rogers on 2014-01-28

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 on 2014-01-28

Get the Xserver to connect to a Mir socket

1346. By Chris Halse Rogers on 2014-01-28

Update for process::Spawner API change

1345. By Chris Halse Rogers on 2014-01-28

Merged subprocess-wrapper into xserver-spawner.

1344. By Chris Halse Rogers on 2014-01-28

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 on 2014-01-28

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 on 2014-01-24

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 on 2014-01-24

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

1340. By Chris Halse Rogers on 2014-01-22

More basic unittests for GlobalSocketListeningServerSpawner

=== modified file '.clang-format'
--- .clang-format	2014-01-10 15:48:08 +0000
+++ .clang-format	2014-01-28 06:35:36 +0000
@@ -14,7 +14,7 @@
 BreakConstructorInitializersBeforeComma: false
 BinPackParameters: true
 ColumnLimit:     120
-ConstructorInitializerAllOnOneLineOrOnePerLine: false
+ConstructorInitializerAllOnOneLineOrOnePerLine: true
 DerivePointerBinding: true
 ExperimentalAutoDetectBinPacking: true
 IndentCaseLabels: false
 
=== modified file 'include/server/mir/default_server_configuration.h'
--- include/server/mir/default_server_configuration.h	2014-01-21 15:29:52 +0000
+++ include/server/mir/default_server_configuration.h	2014-01-28 06:35:36 +0000
@@ -113,6 +113,11 @@
 class Logger;
 }
 
+namespace X
+{
+class ServerSpawner;
+}
+
 class DefaultServerConfiguration : public virtual ServerConfiguration, DefaultConfigurationOptions
 {
 public:
@@ -232,6 +237,12 @@
 
     virtual std::shared_ptr<time::Clock> the_clock();
 
+    /** @name X11 server integration - customization
+     * configurable interfaces for integration with legacy X11 apps
+     * @{ */
+    virtual std::shared_ptr<X::ServerSpawner> the_xserver_spawner();
+    /** @} */
+
 protected:
     using DefaultConfigurationOptions::the_options;
     using DefaultConfigurationOptions::add_options;
 
=== added directory 'include/server/mir/xserver'
=== added file 'include/server/mir/xserver/null_server_spawner.h'
--- include/server/mir/xserver/null_server_spawner.h	1970-01-01 00:00:00 +0000
+++ include/server/mir/xserver/null_server_spawner.h	2014-01-28 06:35:36 +0000
@@ -0,0 +1,25 @@
+#ifndef MIR_X_NULL_SERVER_SPAWNER_H_
+#define MIR_X_NULL_SERVER_SPAWNER_H_
+
+#include <mir/xserver/xserver_launcher.h>
+
+namespace mir
+{
+namespace X
+{
+class NullServerContext : public ServerContext
+{
+public:
+    char const* client_connection_string() override;
+};
+
+class NullServerSpawner : public ServerSpawner
+{
+public:
+    std::future<std::unique_ptr<ServerContext>> create_server(std::shared_ptr<process::Spawner> const& spawner, std::shared_ptr<mir::frontend::Connector> const& connector) override;
+};
+
+}
+}
+
+#endif // MIR_X_NULL_SERVER_SPAWNER_H_
 
=== added file 'include/server/mir/xserver/xserver_launcher.h'
--- include/server/mir/xserver/xserver_launcher.h	1970-01-01 00:00:00 +0000
+++ include/server/mir/xserver/xserver_launcher.h	2014-01-28 06:35:36 +0000
@@ -0,0 +1,59 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Christopher James Halse Rogers <[e-mail]>
+ */
+
+#ifndef MIR_X_XSERVER_LAUNCHER_H_
+#define MIR_X_XSERVER_LAUNCHER_H_
+
+#include <future>
+#include <memory>
+
+#include "mir/process/spawner.h"
+#include "mir/frontend/connector.h"
+
+namespace mir
+{
+namespace X
+{
+
+class ServerContext
+{
+public:
+    virtual ~ServerContext() = default;
+
+    /**
+     * \brief Get the XLib connection string to connect to this server
+     *
+     * This string can be passed by the client to XOpenDisplay to connect
+     * to this server instance, or set in the DISPLAY environment variable
+     * to be used as the default display.
+     */
+    virtual char const* client_connection_string() = 0;
+};
+
+class ServerSpawner
+{
+public:
+    virtual ~ServerSpawner() = default;
+
+    virtual std::future<std::unique_ptr<ServerContext>> create_server(std::shared_ptr<mir::process::Spawner> const& spawner,
+                                                                      std::shared_ptr<mir::frontend::Connector> const& connector) = 0;
+};
+}
+}
+
+#endif // MIR_X_XSERVER_LAUNCHER_H_
 
=== modified file 'src/server/CMakeLists.txt'
--- src/server/CMakeLists.txt	2014-01-28 06:35:36 +0000
+++ src/server/CMakeLists.txt	2014-01-28 06:35:36 +0000
@@ -13,6 +13,7 @@
 add_subdirectory(shell/)
 add_subdirectory(time/)
 add_subdirectory(process/)
+add_subdirectory(xserver/)
 
 set(PREFIX "${CMAKE_INSTALL_PREFIX}")
 set(EXEC_PREFIX "${CMAKE_INSTALL_PREFIX}")
@@ -57,8 +58,9 @@
   mirlttng
   mirnestedgraphics
   miroffscreengraphics
+  mirsharedpipe
   mirprocess
-  mirsharedpipe
+  mirxserverintegration
 )
 
 list(APPEND MIRSERVER_LINKS
 
=== modified file 'src/server/default_server_configuration.cpp'
--- src/server/default_server_configuration.cpp	2014-01-13 06:12:33 +0000
+++ src/server/default_server_configuration.cpp	2014-01-28 06:35:36 +0000
@@ -42,6 +42,7 @@
 #include "mir/time/high_resolution_clock.h"
 #include "mir/geometry/rectangles.h"
 #include "mir/default_configuration.h"
+#include "mir/xserver/null_server_spawner.h"
 
 #include <map>
 
@@ -249,3 +250,8 @@
             return std::make_shared<mir::DefaultServerStatusListener>();
         });
 }
+
+std::shared_ptr<mir::X::ServerSpawner> mir::DefaultServerConfiguration::the_xserver_spawner()
+{
+    return std::make_shared<mir::X::NullServerSpawner>();
+}
 
=== added directory 'src/server/xserver'
=== added file 'src/server/xserver/CMakeLists.txt'
--- src/server/xserver/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ src/server/xserver/CMakeLists.txt	2014-01-28 06:35:36 +0000
@@ -0,0 +1,12 @@
+set(
+  MIR_XSERVER_SRCS
+
+  global_socket_listening_server_spawner.cpp
+  null_server_spawner.cpp
+)
+
+ADD_LIBRARY(
+  mirxserverintegration STATIC
+
+  ${MIR_XSERVER_SRCS}
+)
 
=== added file 'src/server/xserver/global_socket_listening_server_spawner.cpp'
--- src/server/xserver/global_socket_listening_server_spawner.cpp	1970-01-01 00:00:00 +0000
+++ src/server/xserver/global_socket_listening_server_spawner.cpp	2014-01-28 06:35:36 +0000
@@ -0,0 +1,71 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Christopher James Halse Rogers <[e-mail]>
+ */
+
+#include <unistd.h>
+#include <errno.h>
+
+#include <boost/throw_exception.hpp>
+#include <boost/exception/errinfo_errno.hpp>
+
+#include "global_socket_listening_server_spawner.h"
+#include "mir/process/handle.h"
+
+namespace mx = mir::X;
+
+mx::GlobalSocketListeningServerContext::GlobalSocketListeningServerContext(std::unique_ptr<mir::process::Handle> server_handle, std::string connection_string)
+    : server_handle(std::move(server_handle)),
+      connection_string(connection_string)
+{
+}
+
+char const* mx::GlobalSocketListeningServerContext::client_connection_string()
+{
+    return connection_string.c_str();
+}
+
+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)
+{
+    return std::async(std::launch::async, [spawner, connector]()
+    {
+        mir::pipe::Pipe displayfd_pipe;
+        auto displayfd = std::to_string(displayfd_pipe.write_fd());
+        int mir_fd = connector->client_socket_fd();
+        auto mir_fd_arg = std::string("fd://") + std::to_string(mir_fd);
+
+        auto future_handle = spawner->run_from_path("Xorg",
+                                                    {"-displayfd", displayfd.c_str(),
+                                                     "-mir", "xserver",
+                                                     "-mirSocket", mir_fd_arg.c_str()},
+                                                    {displayfd_pipe.write_fd(), mir_fd});
+
+        char display_number[10];
+        errno = 0;
+        int bytes_read = read(displayfd_pipe.read_fd(), display_number, sizeof display_number);
+
+        while (bytes_read == -1 && errno == EINTR)
+            bytes_read = read(displayfd_pipe.read_fd(), display_number, sizeof display_number);;
+
+        if (errno != 0)
+            BOOST_THROW_EXCEPTION(boost::enable_error_info(std::runtime_error("Failed to receive display number from Xserver"))
+                                  << boost::errinfo_errno(errno));
+
+        display_number[bytes_read] = '\0';
+
+        return std::unique_ptr<mx::ServerContext>(new mx::GlobalSocketListeningServerContext(future_handle.get(), std::string(":") + display_number));
+    });
+}
 
=== added file 'src/server/xserver/global_socket_listening_server_spawner.h'
--- src/server/xserver/global_socket_listening_server_spawner.h	1970-01-01 00:00:00 +0000
+++ src/server/xserver/global_socket_listening_server_spawner.h	2014-01-28 06:35:36 +0000
@@ -0,0 +1,52 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Christopher James Halse Rogers <[e-mail]>
+ */
+
+#ifndef MIR_X_GLOBAL_SOCKET_LISTENING_SERVER_SPAWNER_H_
+#define MIR_X_GLOBAL_SOCKET_LISTENING_SERVER_SPAWNER_H_
+
+#include "mir/xserver/xserver_launcher.h"
+#include "mir/process/spawner.h"
+#include "mir/pipe.h"
+
+#include <memory>
+
+namespace mir
+{
+namespace X
+{
+class GlobalSocketListeningServerContext : public ServerContext
+{
+public:
+    GlobalSocketListeningServerContext(std::unique_ptr<mir::process::Handle> server_handle, std::string connection_string);
+    char const* client_connection_string() override;
+
+private:
+    std::unique_ptr<mir::process::Handle> server_handle;
+    std::string connection_string;
+};
+
+class GlobalSocketListeningServerSpawner : public ServerSpawner
+{
+public:
+    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;
+};
+
+}
+}
+
+#endif // MIR_X_GLOBAL_SOCKET_LISTENING_SERVER_SPAWNER_H_
 
=== added file 'src/server/xserver/null_server_spawner.cpp'
--- src/server/xserver/null_server_spawner.cpp	1970-01-01 00:00:00 +0000
+++ src/server/xserver/null_server_spawner.cpp	2014-01-28 06:35:36 +0000
@@ -0,0 +1,34 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Christopher James Halse Rogers <[e-mail]>
+ */
+
+#include "mir/xserver/null_server_spawner.h"
+
+const char* mir::X::NullServerContext::client_connection_string()
+{
+    return "";
+}
+
+std::future<std::unique_ptr<mir::X::ServerContext>> mir::X::NullServerSpawner::create_server(
+    std::shared_ptr<mir::process::Spawner> const& unused, std::shared_ptr<mir::frontend::Connector> const& unuseder)
+{
+    static_cast<void>(unused);
+    static_cast<void>(unuseder);
+    std::promise<std::unique_ptr<mir::X::ServerContext>> boring_promise;
+    boring_promise.set_value(std::unique_ptr<mir::X::ServerContext>(new mir::X::NullServerContext));
+    return boring_promise.get_future();
+}
 
=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2014-01-23 08:06:05 +0000
+++ tests/CMakeLists.txt	2014-01-28 06:35:36 +0000
@@ -36,6 +36,7 @@
 
 if (MIR_BUILD_ACCEPTANCE_TESTS)
   add_subdirectory(acceptance-tests/)
+  pkg_check_modules(X11 REQUIRED x11)
 endif (MIR_BUILD_ACCEPTANCE_TESTS)
 
 if (MIR_BUILD_INTEGRATION_TESTS)
 
=== modified file 'tests/acceptance-tests/CMakeLists.txt'
--- tests/acceptance-tests/CMakeLists.txt	2014-01-28 06:35:36 +0000
+++ tests/acceptance-tests/CMakeLists.txt	2014-01-28 06:35:36 +0000
@@ -28,6 +28,7 @@
   test_client_library_drm.cpp
   test_protobuf.cpp
   test_subprocess.cpp
+  test_xserver_spawner.cpp
   ${GENERATED_PROTOBUF_SRCS}
   ${GENERATED_PROTOBUF_HDRS}
 )
@@ -66,6 +67,7 @@
   ${GMOCK_LIBRARY}
   ${GMOCK_MAIN_LIBRARY}
   ${CMAKE_THREAD_LIBS_INIT} # Link in pthread.
+  ${X11_LDFLAGS}
 )
 
 CMAKE_DEPENDENT_OPTION(
 
=== added file 'tests/acceptance-tests/test_xserver_spawner.cpp'
--- tests/acceptance-tests/test_xserver_spawner.cpp	1970-01-01 00:00:00 +0000
+++ tests/acceptance-tests/test_xserver_spawner.cpp	2014-01-28 06:35:36 +0000
@@ -0,0 +1,73 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Christopher James Halse Rogers <[e-mail]>
+ */
+
+#include "src/server/xserver/global_socket_listening_server_spawner.h"
+#include "mir_test_framework/testing_server_configuration.h"
+#include "mir_test_framework/in_process_server.h"
+#include "mir/xserver/xserver_launcher.h"
+#include "src/server/process/fork_spawner.h"
+
+#include <X11/Xlib.h>
+#include <stdlib.h>
+
+#include <gtest/gtest.h>
+
+namespace mtf = mir_test_framework;
+namespace mx = mir::X;
+
+namespace
+{
+struct XserverSpawningServer : public mtf::InProcessServer
+{
+public:
+    std::shared_ptr<mx::ServerSpawner> the_xserver_spawner()
+    {
+        return config.the_xserver_spawner();
+    }
+
+    mir::DefaultServerConfiguration& server_config() override
+    {
+        return config;
+    }
+
+    class SocketListeningXServerConfig : public mtf::StubbedServerConfiguration
+    {
+    public:
+        std::shared_ptr<mx::ServerSpawner> the_xserver_spawner() override
+        {
+            return std::make_shared<mx::GlobalSocketListeningServerSpawner> ();
+        }
+    } config;
+};
+}
+
+// This requires a bit more fiddling before it will work.
+// Particularly, it needs an xorg.conf that specifies dummy
+// devices, so the real devices don't fail to load.
+TEST_F(XserverSpawningServer, DISABLED_X11ClientConnects)
+{
+    // Ensure the surrounding environment doesn't mess with the test
+    unsetenv("DISPLAY");
+
+    auto xserver = the_xserver_spawner()->create_server(std::make_shared<mir::process::ForkSpawner>(), config.the_connector());
+    Display* disp = XOpenDisplay(xserver.get()->client_connection_string());
+
+    ASSERT_TRUE(disp != NULL);
+
+    XCloseDisplay(disp);
+}
 
=== modified file 'tests/unit-tests/CMakeLists.txt'
--- tests/unit-tests/CMakeLists.txt	2014-01-28 06:35:36 +0000
+++ tests/unit-tests/CMakeLists.txt	2014-01-28 06:35:36 +0000
@@ -30,6 +30,7 @@
 add_subdirectory(android_input/)
 add_subdirectory(scene/)
 add_subdirectory(draw/)
+add_subdirectory(xserver/)
 
 add_executable(mir_unit_tests ${UNIT_TEST_SOURCES})
 uses_android_input(mir_unit_tests)
@@ -43,6 +44,7 @@
   mirdraw
   mirtestdraw
   mirlogging
+  mirxserverintegration
 
   mir-test
   mir-test-doubles
 
=== added directory 'tests/unit-tests/xserver'
=== added file 'tests/unit-tests/xserver/CMakeLists.txt'
--- tests/unit-tests/xserver/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/xserver/CMakeLists.txt	2014-01-28 06:35:36 +0000
@@ -0,0 +1,8 @@
+list(APPEND UNIT_TEST_SOURCES
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_xserver_spawner.cpp
+)
+
+set(
+  UNIT_TEST_SOURCES
+  ${UNIT_TEST_SOURCES}
+  PARENT_SCOPE)
 
=== added file 'tests/unit-tests/xserver/test_xserver_spawner.cpp'
--- tests/unit-tests/xserver/test_xserver_spawner.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/xserver/test_xserver_spawner.cpp	2014-01-28 06:35:36 +0000
@@ -0,0 +1,206 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Christopher James Halse Rogers <[e-mail]>
+ */
+
+#include <future>
+#include <vector>
+#include <thread>
+#include <condition_variable>
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+#include "mir/process/spawner.h"
+#include "mir/process/handle.h"
+
+#include "src/server/xserver/global_socket_listening_server_spawner.h"
+
+using namespace ::testing;
+
+namespace
+{
+struct MockConnector : public mir::frontend::Connector
+{
+    MOCK_METHOD0(start, void(void));
+    MOCK_METHOD0(stop, void(void));
+    MOCK_CONST_METHOD0(client_socket_fd, int(void));
+    MOCK_CONST_METHOD0(remove_endpoint, void(void));
+};
+
+struct MockProcessSpawner : public mir::process::Spawner
+{
+    MOCK_CONST_METHOD3(run, mir::process::Handle*(std::string, std::vector<char const*>, std::vector<int>));
+
+    std::future<std::unique_ptr<mir::process::Handle>> run_from_path(char const* binary) const override
+    {
+        std::promise<std::unique_ptr<mir::process::Handle>> dummy_promise;
+        dummy_promise.set_value(std::unique_ptr<mir::process::Handle>(
+            run(binary, std::initializer_list<char const*>(), std::initializer_list<int>())));
+        return dummy_promise.get_future();
+    }
+    std::future<std::unique_ptr<mir::process::Handle>> run_from_path(char const* binary,
+                                                                     std::initializer_list<char const*> args) const
+        override
+    {
+        std::promise<std::unique_ptr<mir::process::Handle>> dummy_promise;
+        dummy_promise.set_value(std::unique_ptr<mir::process::Handle>(run(binary, args, std::initializer_list<int>())));
+        return dummy_promise.get_future();
+    }
+
+    std::future<std::unique_ptr<mir::process::Handle>> run_from_path(char const* binary,
+                                                                     std::initializer_list<char const*> args,
+                                                                     std::initializer_list<int> fds) const override
+    {
+        std::promise<std::unique_ptr<mir::process::Handle>> dummy_promise;
+        dummy_promise.set_value(std::unique_ptr<mir::process::Handle>(run(binary, args, fds)));
+        return dummy_promise.get_future();
+    }
+};
+
+struct SocketListeningServerTest : public testing::Test
+{
+    SocketListeningServerTest()
+        : default_server_number("100"),
+          spawner(std::make_shared<NiceMock<MockProcessSpawner>>()),
+          connector(std::make_shared<NiceMock<MockConnector>>())
+    {
+        ON_CALL(*spawner, run(_, _, _))
+            .WillByDefault(DoAll(SaveArg<0>(&binary),
+                                 SaveArg<1>(&args),
+                                 SaveArg<2>(&fds),
+                                 InvokeWithoutArgs([this]()
+                                                   { write_server_string(default_server_number); }),
+                                 Return(nullptr)));
+        ON_CALL(*connector, client_socket_fd())
+            .WillByDefault(Return(22));
+    }
+
+    void write_server_string(std::string server_number)
+    {
+        auto location = std::find_if(args.begin(), args.end(), [](char const* a)
+                                     { return strcmp(a, "-displayfd") == 0; });
+        ASSERT_NE(location, args.end());
+        ASSERT_NE(++location, args.end());
+        int server_fd = atoi(*location);
+        write(server_fd, server_number.data(), server_number.length());
+        close(server_fd);
+    }
+
+    std::string default_server_number;
+    std::string binary;
+    std::vector<char const*> args;
+    std::vector<int> fds;
+    std::shared_ptr<NiceMock<MockProcessSpawner>> spawner;
+    std::shared_ptr<NiceMock<MockConnector>> connector;
+};
+}
+
+TEST_F(SocketListeningServerTest, CreateServerAlwaysValid)
+{
+    mir::X::GlobalSocketListeningServerSpawner factory;
+
+    auto server_context = factory.create_server(spawner, connector);
+    ASSERT_NE(server_context.get(), nullptr);
+}
+
+TEST_F(SocketListeningServerTest, SpawnsCorrectExecutable)
+{
+    mir::X::GlobalSocketListeningServerSpawner factory;
+
+    auto server_context = factory.create_server(spawner, connector);
+    server_context.get();
+
+    EXPECT_EQ(binary, "Xorg");
+}
+
+namespace
+{
+MATCHER_P(ContainsSubsequence, subsequence, "")
+{
+    auto location =
+        std::search(arg.begin(), arg.end(), subsequence.begin(), subsequence.end(), [](char const* a, std::string b)
+                    { return strcmp(a, b.c_str()) == 0; });
+    return location != arg.end();
+}
+}
+
+TEST_F(SocketListeningServerTest, SpawnsWithDisplayFDSet)
+{
+    mir::X::GlobalSocketListeningServerSpawner factory;
+
+    auto server_context = factory.create_server(spawner, connector);
+    server_context.get();
+
+    ASSERT_THAT(args, Not(IsEmpty()));
+    ASSERT_THAT(fds, Not(IsEmpty()));
+
+    Matcher<std::vector<char const*>> fd_matcher = Not(_);
+    for (auto fd : fds)
+    {
+        fd_matcher = AnyOf(fd_matcher, ContainsSubsequence(std::vector<std::string>{"-displayfd", std::to_string(fd)}));
+    }
+    EXPECT_THAT(args, fd_matcher);
+}
+
+TEST_F(SocketListeningServerTest, ReturnsCorrectDisplayString)
+{
+    mir::X::GlobalSocketListeningServerSpawner factory;
+
+    default_server_number = "20";
+    auto server_context = factory.create_server(spawner, connector);
+
+    EXPECT_STREQ(":20", server_context.get()->client_connection_string());
+}
+
+TEST_F(SocketListeningServerTest, HandlesSpawnerLifecycleCorrectly)
+{
+    mir::X::GlobalSocketListeningServerSpawner factory;
+
+    std::future<std::unique_ptr<mir::X::ServerContext>> server_context;
+
+    {
+        auto tmp_spawner = std::make_shared<MockProcessSpawner>();
+        ON_CALL(*tmp_spawner, run(_, _, _))
+            .WillByDefault(DoAll(SaveArg<0>(&binary),
+                                 SaveArg<1>(&args),
+                                 SaveArg<2>(&fds),
+                                 InvokeWithoutArgs([this]()
+                                                   { write_server_string(default_server_number); }),
+                                 Return(nullptr)));
+        EXPECT_CALL(*tmp_spawner, run(_, _, _));
+        server_context = factory.create_server(tmp_spawner, connector);
+    }
+
+    ASSERT_NE(server_context.get(), nullptr);
+}
+
+TEST_F(SocketListeningServerTest, PassesMirSocketCorrectly)
+{
+    mir::X::GlobalSocketListeningServerSpawner factory;
+
+    int const mir_fd{32};
+    std::string const mir_connect_str{std::string("fd://") + std::to_string(mir_fd)};
+    EXPECT_CALL(*connector, client_socket_fd())
+        .WillOnce(Return(mir_fd));
+
+    auto server_context = factory.create_server(spawner, connector);
+
+    server_context.get();
+
+    EXPECT_THAT(args, ContainsSubsequence(std::vector<std::string>{"-mirSocket", mir_connect_str}));
+    EXPECT_THAT(fds, Contains(Eq(mir_fd)));
+}