Merge lp:~alan-griffiths/miral/rework-titlebars into lp:miral

[Alan Griffiths]

Re-implement dummy titlebars without using mirserver APIs

Re-implement dummy titlebars without using mirserver APIs

This completes the separation of miral-shell from libmirserver and allows the removal of the awkward build_window() and destroy() methods from the WindowManagerTools interface.

[Alexandros Frantzis (afrantzis)]

Looks good.

review: Approve

[Alan Griffiths (alan-griffiths)]

I've found a broken usecase:

Running Xenial + recent build of lp:mir (I should update this and retest).

Run miral-shell on mesa.

Launch all the mir_demo_client_* together.

The titlebars do not get associated correctly with the apps:

1. They all appear grey.
2. The apps can be closed (Alt+F4), but titlebars remain.
3. When all apps can are closed Alt+F4 closes the server. (I guess a titlebar gets focus and the internal client PID is kill(SIGTERM)'d).

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

> I've found a broken usecase:
>
> Running Xenial + recent build of lp:mir (I should update this and retest).

Tested with current lp:mir tip - behaviour is less bad (E.g. there's a bright "focused" titlebar and focus is indicated correctly.)

But, Z-order is wrong as unrelated are sometimes placed between surface and titlebar resulting in the titlebar overlaying a surface above the main window.

And after closing all apps there can be an "orphan" titlebar that gets focus and kills the server on Alt+F4.

review: Needs Fixing

[lp:~alan-griffiths/miral/rework-titlebars]

222. By Alan Griffiths on 2016-07-07

Avoid suicide

223. By Alan Griffiths on 2016-07-07

Don't overspecified function name

224. By Alan Griffiths on 2016-07-07

Ensure that titlebars are together with main surface in z-order

[Alan Griffiths (alan-griffiths)]

> > I've found a broken usecase:

fixed

review: Abstain

[Alexandros Frantzis (afrantzis)]

Still looks good (with mir 0.23.1 packages on xenial).

review: Approve

=== modified file 'include/miral/window_manager_tools.h'
--- include/miral/window_manager_tools.h	2016-06-17 16:50:29 +0000
+++ include/miral/window_manager_tools.h	2016-07-07 15:12:19 +0000
@@ -49,8 +49,6 @@
  *  I.e. they should only be used by a thread that has called the WindowManagementPolicy methods
  *  (where any necessary locks are held) or via a invoke_under_lock() callback.
  *  @{ */
-    virtual auto build_window(Application const& application, WindowSpecification const& parameters)
-    -> WindowInfo& = 0;
     virtual auto count_applications() const -> unsigned int = 0;
     virtual void for_each_application(std::function<void(ApplicationInfo& info)> const& functor) = 0;
     virtual auto find_application(std::function<bool(ApplicationInfo const& info)> const& predicate)
@@ -66,7 +64,6 @@
     virtual void focus_next_within_application() = 0;
     virtual auto window_at(mir::geometry::Point cursor) const -> Window = 0;
     virtual auto active_display() -> mir::geometry::Rectangle const = 0;
-    virtual void destroy(Window& window) = 0;
     virtual void raise_tree(Window const& root) = 0;
     virtual void modify_window(WindowInfo& window_info, WindowSpecification const& modifications) = 0;
     virtual void place_and_size(WindowInfo& window_info, Point const& new_pos, Size const& new_size) = 0;

=== modified file 'miral-shell/CMakeLists.txt'
--- miral-shell/CMakeLists.txt	2016-06-17 16:50:29 +0000
+++ miral-shell/CMakeLists.txt	2016-07-07 15:12:19 +0000
@@ -1,6 +1,5 @@
 add_subdirectory(spinner)
 add_subdirectory(desktop)
-add_subdirectory(titlebar)
 
 add_custom_target(miral-run ALL
     cp ${CMAKE_CURRENT_SOURCE_DIR}/miral-run.sh ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/miral-run
@@ -12,12 +11,13 @@
 
 add_executable(miral-shell
     shell_main.cpp
-    tiling_window_manager.cpp    tiling_window_manager.h
-    titlebar_window_manager.cpp titlebar_window_manager.h)
+    tiling_window_manager.cpp   tiling_window_manager.h
+    titlebar_window_manager.cpp titlebar_window_manager.h
+    titlebar_provider.cpp       titlebar_provider.h
+)
 
 target_link_libraries(miral-shell
     miral-spinner
-    miral-titlebar
     miral
 )
 

=== modified file 'miral-shell/shell_main.cpp'
--- miral-shell/shell_main.cpp	2016-06-17 16:50:29 +0000
+++ miral-shell/shell_main.cpp	2016-07-07 15:12:19 +0000
@@ -35,7 +35,7 @@
     InternalClientLauncher launcher;
     WindowManagerOptions window_managers
         {
-            add_window_manager_policy<TitlebarWindowManagerPolicy>("titlebar", spinner),
+            add_window_manager_policy<TitlebarWindowManagerPolicy>("titlebar", spinner, launcher),
             add_window_manager_policy<CanonicalWindowManagerPolicy>("canonical"),
             add_window_manager_policy<TilingWindowManagerPolicy>("tiling", spinner, launcher),
         };

=== removed directory 'miral-shell/titlebar'
=== removed file 'miral-shell/titlebar/CMakeLists.txt'
--- miral-shell/titlebar/CMakeLists.txt	2016-06-07 09:24:55 +0000
+++ miral-shell/titlebar/CMakeLists.txt	1970-01-01 00:00:00 +0000
@@ -1,13 +0,0 @@
-# TODO remove dependency on libmirserver-dev
-# (Only needed by awkward painting code)
-pkg_check_modules(MIRSERVER mirserver>=0.20 REQUIRED)
-include_directories(include SYSTEM ${MIRSERVER_INCLUDE_DIRS})
-
-
-add_library(miral-titlebar STATIC
-        titlebar_user_data.cpp titlebar_user_data.h
-)
-
-target_link_libraries(miral-titlebar
-    miral
-)

=== removed file 'miral-shell/titlebar/titlebar_user_data.cpp'
--- miral-shell/titlebar/titlebar_user_data.cpp	2016-06-07 19:05:10 +0000
+++ miral-shell/titlebar/titlebar_user_data.cpp	1970-01-01 00:00:00 +0000
@@ -1,171 +0,0 @@
-/*
- * Copyright © 2016 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: Alan Griffiths <[e-mail]>
- */
-
-#include "titlebar_user_data.h"
-
-// TODO We need a better way to support painting stuff inside the window manager
-#include <mir/frontend/buffer_stream.h>
-#include <mir/graphics/buffer.h>
-#include <mir/graphics/buffer_properties.h>
-#include <mir/scene/session.h>
-#include <mir/scene/surface.h>
-
-#include <mir/version.h>
-
-#include <atomic>
-
-using namespace mir::geometry;
-
-struct TitlebarUserData::StreamPainter
-{
-    virtual void paint(int) = 0;
-    virtual ~StreamPainter() = default;
-    StreamPainter() = default;
-    StreamPainter(StreamPainter const&) = delete;
-    StreamPainter& operator=(StreamPainter const&) = delete;
-};
-
-struct TitlebarUserData::SwappingPainter
-    : TitlebarUserData::StreamPainter
-{
-    SwappingPainter(std::shared_ptr<mir::frontend::BufferStream> const& buffer_stream) :
-        buffer_stream{buffer_stream}, buffer{nullptr}
-    {
-        swap_buffers(nullptr);
-    }
-
-    void swap_buffers(mir::graphics::Buffer* buf)
-    {
-        auto const callback = [this](mir::graphics::Buffer* new_buffer)
-            {
-                buffer.store(new_buffer);
-            };
-
-        buffer_stream->swap_buffers(buf, callback);
-    }
-
-    void paint(int intensity) override
-    {
-        if (auto const buf = buffer.exchange(nullptr))
-        {
-            auto const format = buffer_stream->pixel_format();
-            auto const sz = buf->size().height.as_int() *
-                            buf->size().width.as_int() * MIR_BYTES_PER_PIXEL(format);
-            std::vector<unsigned char> pixels(sz, intensity);
-            buf->write(pixels.data(), sz);
-            swap_buffers(buf);
-        }
-    }
-
-    std::shared_ptr<mir::frontend::BufferStream> const buffer_stream;
-    std::atomic<mir::graphics::Buffer*> buffer;
-};
-
-struct TitlebarUserData::AllocatingPainter
-    : TitlebarUserData::StreamPainter
-{
-    AllocatingPainter(
-        std::shared_ptr<mir::frontend::BufferStream> const& buffer_stream,
-#if MIR_SERVER_VERSION >= MIR_VERSION_NUMBER(0, 24, 0)
-        std::shared_ptr<mir::scene::Session> const& session,
-#endif
-    Size size) :
-        buffer_stream(buffer_stream),
-#if MIR_SERVER_VERSION >= MIR_VERSION_NUMBER(0, 24, 0)
-        session(session),
-#endif
-        properties({
-                       size,
-                       buffer_stream->pixel_format(),
-                       mir::graphics::BufferUsage::software
-                   }),
-#if MIR_SERVER_VERSION >= MIR_VERSION_NUMBER(0, 24, 0)
-        front_buffer(session->create_buffer(properties)),
-        back_buffer(session->create_buffer(properties))
-#else
-        front_buffer(buffer_stream->allocate_buffer(properties)),
-        back_buffer(buffer_stream->allocate_buffer(properties))
-#endif
-    {
-    }
-
-    void paint(int intensity) override
-    {
-#if MIR_SERVER_VERSION >= MIR_VERSION_NUMBER(0, 24, 0)
-        auto buffer = session->get_buffer(back_buffer);
-
-        auto const format = buffer->pixel_format();
-        auto const sz = buffer->size().height.as_int() *
-                        buffer->size().width.as_int() * MIR_BYTES_PER_PIXEL(format);
-        std::vector<unsigned char> pixels(sz, intensity);
-        buffer->write(pixels.data(), sz);
-        buffer_stream->swap_buffers(buffer.get(), [](mir::graphics::Buffer*){});
-#else
-        buffer_stream->with_buffer(back_buffer,
-            [this, intensity](mir::graphics::Buffer& buffer)
-            {
-                auto const format = buffer.pixel_format();
-                auto const sz = buffer.size().height.as_int() * buffer.size().width.as_int() * MIR_BYTES_PER_PIXEL(format);
-                std::vector<unsigned char> pixels(sz, intensity);
-                buffer.write(pixels.data(), sz);
-                buffer_stream->swap_buffers(&buffer, [](mir::graphics::Buffer*){});
-            });
-#endif
-        std::swap(front_buffer, back_buffer);
-    }
-
-    ~AllocatingPainter()
-    {
-#if MIR_SERVER_VERSION >= MIR_VERSION_NUMBER(0, 24, 0)
-        session->destroy_buffer(front_buffer);
-        session->destroy_buffer(back_buffer);
-#else
-        buffer_stream->remove_buffer(front_buffer);
-        buffer_stream->remove_buffer(back_buffer);
-#endif
-    }
-
-    std::shared_ptr<mir::frontend::BufferStream> const buffer_stream;
-    std::shared_ptr<mir::scene::Session> const session;
-    mir::graphics::BufferProperties properties;
-    mir::graphics::BufferID front_buffer;
-    mir::graphics::BufferID back_buffer;
-};
-
-void TitlebarUserData::paint_titlebar(int intensity)
-{
-    if (!stream_painter)
-    {
-        auto stream = std::shared_ptr<mir::scene::Surface>(window)->primary_buffer_stream();
-        try
-        {
-            stream_painter = std::make_shared<AllocatingPainter>(
-                stream,
-#if MIR_SERVER_VERSION >= MIR_VERSION_NUMBER(0, 24, 0)
-                window.application(),
-#endif
-            window.size());
-        }
-        catch (...)
-        {
-            stream_painter = std::make_shared<SwappingPainter>(stream);
-        }
-    }
-
-    stream_painter->paint(intensity);
-}

=== removed file 'miral-shell/titlebar/titlebar_user_data.h'
--- miral-shell/titlebar/titlebar_user_data.h	2016-06-07 09:24:55 +0000
+++ miral-shell/titlebar/titlebar_user_data.h	1970-01-01 00:00:00 +0000
@@ -1,41 +0,0 @@
-/*
- * Copyright © 2016 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: Alan Griffiths <[e-mail]>
- */
-
-#ifndef MIRAL_SHELL_TITLEBAR_USER_DATA_H
-#define MIRAL_SHELL_TITLEBAR_USER_DATA_H
-
-#include <miral/window.h>
-
-class TitlebarUserData
-{
-public:
-    TitlebarUserData(miral::Window window) : window{window} { }
-
-    void paint_titlebar(int intensity);
-
-    miral::Window window;
-
-private:
-    struct StreamPainter;
-    struct AllocatingPainter;
-    struct SwappingPainter;
-
-    std::shared_ptr<StreamPainter> stream_painter;
-};
-
-#endif //MIRAL_SHELL_TITLEBAR_USER_DATA_H

=== added file 'miral-shell/titlebar_provider.cpp'
--- miral-shell/titlebar_provider.cpp	1970-01-01 00:00:00 +0000
+++ miral-shell/titlebar_provider.cpp	2016-07-07 15:12:19 +0000
@@ -0,0 +1,276 @@
+/*
+ * Copyright © 2016 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: Alan Griffiths <[e-mail]>
+ */
+
+#include "titlebar_provider.h"
+
+#include <miral/toolkit/surface_spec.h>
+
+#include <mir_toolkit/mir_buffer_stream.h>
+
+#include <cstring>
+#include <sstream>
+
+namespace
+{
+int const title_bar_height = 10;
+
+void null_surface_callback(MirSurface*, void*) {}
+
+void paint_surface(MirSurface* surface, int const intensity)
+{
+    MirBufferStream* buffer_stream = mir_surface_get_buffer_stream(surface);
+
+    // TODO sometimes buffer_stream is nullptr - find out why (and fix).
+    // (Only observed when creating a lot of clients at once)
+    if (!buffer_stream)
+        return;
+
+    MirGraphicsRegion region;
+    mir_buffer_stream_get_graphics_region(buffer_stream, &region);
+
+    char* row = region.vaddr;
+
+    for (int j = 0; j != region.height; ++j)
+    {
+        memset(row, intensity, 4*region.width);
+        row += region.stride;
+    }
+
+    mir_buffer_stream_swap_buffers_sync(buffer_stream);
+}
+}
+
+using namespace miral::toolkit;
+using namespace mir::geometry;
+
+TitlebarProvider::TitlebarProvider(miral::WindowManagerTools* const tools) : tools{tools}
+{
+
+}
+
+TitlebarProvider::~TitlebarProvider()
+{
+    stop();
+}
+
+void TitlebarProvider::stop()
+{
+    enqueue_work([this]
+        {
+            std::lock_guard<decltype(mutex)> lock{mutex};
+            window_to_titlebar.clear();
+            connection.reset();
+            stop_work();
+        });
+}
+
+void TitlebarProvider::operator()(miral::toolkit::Connection connection)
+{
+    this->connection = connection;
+    start_work();
+}
+
+void TitlebarProvider::operator()(std::weak_ptr<mir::scene::Session> const& session)
+{
+    std::lock_guard<decltype(mutex)> lock{mutex};
+    this->weak_session = session;
+}
+
+auto TitlebarProvider::session() const -> std::shared_ptr<mir::scene::Session>
+{
+    std::lock_guard<decltype(mutex)> lock{mutex};
+    return weak_session.lock();
+}
+
+void TitlebarProvider::create_titlebar_for(miral::Window const& window)
+{
+    enqueue_work([this, window]
+        {
+            std::ostringstream buffer;
+
+            buffer << std::shared_ptr<mir::scene::Surface>(window).get();
+
+            auto const spec = SurfaceSpec::for_normal_surface(
+                connection, window.size().width.as_int(), title_bar_height, mir_pixel_format_xrgb_8888)
+                .set_buffer_usage(mir_buffer_usage_software)
+                .set_type(mir_surface_type_gloss)
+                .set_name(buffer.str().c_str());
+
+            std::lock_guard<decltype(mutex)> lock{mutex};
+            spec.create_surface(insert, &window_to_titlebar[window]);
+        });
+}
+
+void TitlebarProvider::paint_titlebar_for(miral::Window const& window, int intensity)
+{
+    if (auto data = find_titlebar_data(window))
+    {
+        if (auto surface = data->titlebar.load())
+        {
+            enqueue_work([this, surface, intensity]{ paint_surface(surface, intensity); });
+        }
+        else
+        {
+            data->on_create = [this, intensity](MirSurface* surface)
+                { enqueue_work([this, surface, intensity]{ paint_surface(surface, intensity); }); };
+        }
+    }
+}
+
+void TitlebarProvider::destroy_titlebar_for(miral::Window const& window)
+{
+    enqueue_work([this, window]
+        {
+            std::lock_guard<decltype(mutex)> lock{mutex};
+            window_to_titlebar.erase(window);
+        });
+}
+
+void TitlebarProvider::resize_titlebar_for(miral::Window const& window, Size const& size)
+{
+    if (window.size().width == size.width)
+        return;
+
+    if (auto titlebar_window = find_titlebar_window(window))
+    {
+        titlebar_window.resize({size.width, title_bar_height});
+    }
+}
+
+void TitlebarProvider::advise_new_titlebar(miral::WindowInfo& window_info)
+{
+    std::istringstream buffer{window_info.name()};
+
+    void* parent = nullptr;
+    buffer >> parent;
+
+    std::lock_guard<decltype(mutex)> lock{mutex};
+
+    for (auto& element : window_to_titlebar)
+    {
+        auto scene_surface = std::shared_ptr<mir::scene::Surface>(element.first);
+        if (scene_surface.get() == parent)
+        {
+            auto window = window_info.window();
+            element.second.window = window;
+            auto& parent_info = tools->info_for(scene_surface);
+            parent_info.add_child(window);
+            window_info.parent(parent_info.window());
+            window.move_to(parent_info.window().top_left() - Displacement{0, title_bar_height});
+            break;
+        }
+    }
+}
+
+void TitlebarProvider::advise_state_change(miral::WindowInfo const& window_info, MirSurfaceState state, Rectangle const& display_area)
+{
+    if (auto window = find_titlebar_window(window_info.window()))
+    {
+        switch (state)
+        {
+        case mir_surface_state_restored:
+            window.resize({window_info.restore_rect().size.width, title_bar_height});
+            window.show();
+            break;
+
+        case mir_surface_state_maximized:
+        case mir_surface_state_vertmaximized:
+        case mir_surface_state_hidden:
+        case mir_surface_state_minimized:
+            window.hide();
+            break;
+
+        case mir_surface_state_horizmaximized:
+            window.resize({display_area.size.width, title_bar_height});
+            window.show();
+            break;
+
+        case mir_surface_state_fullscreen:
+        default:
+            break;
+        }
+    }
+}
+
+TitlebarProvider::Data::~Data()
+{
+    if (auto const surface = titlebar.load())
+        mir_surface_release(surface, &null_surface_callback, nullptr);
+}
+
+void TitlebarProvider::insert(MirSurface* surface, Data* data)
+{
+    data->on_create(surface);
+    data->titlebar = surface;
+}
+
+TitlebarProvider::Data* TitlebarProvider::find_titlebar_data(miral::Window const& window)
+{
+    std::lock_guard<decltype(mutex)> lock{mutex};
+
+    auto const find = window_to_titlebar.find(window);
+
+    return (find != window_to_titlebar.end()) ? &find->second : nullptr;
+}
+
+miral::Window TitlebarProvider::find_titlebar_window(miral::Window const& window) const
+{
+    std::lock_guard<decltype(mutex)> lock{mutex};
+
+    auto const find = window_to_titlebar.find(window);
+
+    return (find != window_to_titlebar.end()) ? find->second.window : miral::Window{};
+}
+
+Worker::~Worker()
+{
+    if (worker.joinable()) worker.join();
+}
+
+void Worker::do_work()
+{
+    while (!work_done)
+    {
+        WorkQueue::value_type work;
+        {
+            std::unique_lock<decltype(work_mutex)> lock{work_mutex};
+            work_cv.wait(lock, [this] { return !work_queue.empty(); });
+            work = work_queue.front();
+            work_queue.pop();
+        }
+
+        work();
+    }
+}
+
+void Worker::enqueue_work(std::function<void()> const& functor)
+{
+    std::lock_guard<decltype(work_mutex)> lock{work_mutex};
+    work_queue.push(functor);
+    work_cv.notify_one();
+}
+
+void Worker::start_work()
+{
+    worker = std::thread{[this] { do_work(); }};
+}
+
+void Worker::stop_work()
+{
+    enqueue_work([this] { work_done = true; });
+}
\ No newline at end of file

=== added file 'miral-shell/titlebar_provider.h'
--- miral-shell/titlebar_provider.h	1970-01-01 00:00:00 +0000
+++ miral-shell/titlebar_provider.h	2016-07-07 15:12:19 +0000
@@ -0,0 +1,102 @@
+/*
+ * Copyright © 2016 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: Alan Griffiths <[e-mail]>
+ */
+
+#ifndef MIRAL_SHELL_TITLEBAR_PROVIDER_H
+#define MIRAL_SHELL_TITLEBAR_PROVIDER_H
+
+
+#include <miral/window_manager_tools.h>
+
+#include <mir/geometry/rectangle.h>
+#include <mir_toolkit/client_types.h>
+
+#include <atomic>
+#include <map>
+#include <mutex>
+#include <miral/toolkit/connection.h>
+#include <thread>
+#include <condition_variable>
+#include <queue>
+
+class Worker
+{
+public:
+    ~Worker();
+
+    void start_work();
+    void enqueue_work(std::function<void()> const& functor);
+    void stop_work();
+
+private:
+    using WorkQueue = std::queue<std::function<void()>>;
+
+    std::mutex mutable work_mutex;
+    std::condition_variable work_cv;
+    WorkQueue work_queue;
+    bool work_done = false;
+    std::thread worker;
+
+    void do_work();
+};
+
+class TitlebarProvider : Worker
+{
+public:
+    TitlebarProvider(miral::WindowManagerTools* const tools);
+    ~TitlebarProvider();
+
+    void operator()(miral::toolkit::Connection connection);
+    void operator()(std::weak_ptr<mir::scene::Session> const& session);
+
+    auto session() const -> std::shared_ptr<mir::scene::Session>;
+
+    void create_titlebar_for(miral::Window const& window);
+    void paint_titlebar_for(miral::Window const& window, int intensity);
+    void destroy_titlebar_for(miral::Window const& window);
+    void resize_titlebar_for(miral::Window const& window, mir::geometry::Size const& size);
+    void advise_new_titlebar(miral::WindowInfo& window_info);
+    void advise_state_change(miral::WindowInfo const& window_info, MirSurfaceState state, mir::geometry::Rectangle const& display_area);
+
+    void stop();
+
+private:
+    struct Data
+    {
+        std::atomic<MirSurface*> titlebar{nullptr};
+        std::function<void(MirSurface* surface)> on_create{[](MirSurface*){}};
+        miral::Window window;
+
+        ~Data();
+    };
+
+    using SurfaceMap = std::map<std::weak_ptr<mir::scene::Surface>, Data, std::owner_less<std::weak_ptr<mir::scene::Surface>>>;
+
+    miral::WindowManagerTools* const tools;
+    std::mutex mutable mutex;
+    miral::toolkit::Connection connection;
+    std::weak_ptr<mir::scene::Session> weak_session;
+
+    SurfaceMap window_to_titlebar;
+
+    static void insert(MirSurface* surface, Data* data);
+    Data* find_titlebar_data(miral::Window const& window);
+    miral::Window find_titlebar_window(miral::Window const& window) const;
+};
+
+
+#endif //MIRAL_SHELL_TITLEBAR_PROVIDER_H

=== modified file 'miral-shell/titlebar_window_manager.cpp'
--- miral-shell/titlebar_window_manager.cpp	2016-06-17 16:50:29 +0000
+++ miral-shell/titlebar_window_manager.cpp	2016-07-07 15:12:19 +0000
@@ -17,38 +17,32 @@
  */
 
 #include "titlebar_window_manager.h"
-#include "titlebar/titlebar_user_data.h"
+#include "titlebar_provider.h"
 
 #include <miral/application_info.h>
+#include <miral/internal_client.h>
 #include <miral/window_info.h>
 #include <miral/window_manager_tools.h>
 
+#include <linux/input.h>
+
 using namespace miral;
 
-namespace
-{
-int const title_bar_height = 10;
-Size titlebar_size_for_window(Size window_size)
-{
-    return {window_size.width, Height{title_bar_height}};
-}
-
-Point titlebar_position_for_window(Point window_position)
-{
-    return {
-        window_position.x,
-        window_position.y - DeltaY(title_bar_height)
-    };
-}
-}
-
-TitlebarWindowManagerPolicy::TitlebarWindowManagerPolicy(WindowManagerTools* const tools, SpinnerSplash const& spinner) :
+
+TitlebarWindowManagerPolicy::TitlebarWindowManagerPolicy(
+    WindowManagerTools* const tools,
+    SpinnerSplash const& spinner,
+    miral::InternalClientLauncher const& launcher) :
     CanonicalWindowManagerPolicy(tools),
     tools(tools),
-    spinner{spinner}
+    spinner{spinner},
+    titlebar_provider{std::make_unique<TitlebarProvider>(tools)}
 {
+    launcher.launch("decorations", *titlebar_provider);
 }
 
+TitlebarWindowManagerPolicy::~TitlebarWindowManagerPolicy() = default;
+
 bool TitlebarWindowManagerPolicy::handle_pointer_event(MirPointerEvent const* event)
 {
     auto consumes_event = CanonicalWindowManagerPolicy::handle_pointer_event(event);
@@ -64,24 +58,14 @@
     {
         if (mir_pointer_event_button_state(event, mir_pointer_button_primary))
         {
-            // TODO this is a rather roundabout way to detect a titlebar
             if (auto const possible_titlebar = tools->window_at(old_cursor))
             {
-                if (auto const parent = tools->info_for(possible_titlebar).parent())
+                if (possible_titlebar.application() == titlebar_provider->session())
                 {
-                    if (auto const& parent_userdata =
-                        std::static_pointer_cast<TitlebarUserData>(tools->info_for(parent).userdata()))
-                    {
-                        if (possible_titlebar == parent_userdata->window)
-                        {
-                            if (auto const target = tools->window_at(old_cursor))
-                            {
-                                tools->select_active_window(target);
-                                tools->drag_active_window(cursor - old_cursor);
-                            }
-                            consumes_event = true;
-                        }
-                    }
+                    auto const& info = tools->info_for(possible_titlebar);
+                    tools->select_active_window(info.parent());
+                    tools->drag_active_window(cursor - old_cursor);
+                    consumes_event = true;
                 }
             }
         }
@@ -96,47 +80,33 @@
 {
     CanonicalWindowManagerPolicy::advise_new_window(window_info);
 
-    if (!window_info.needs_titlebar(window_info.type()))
-        return;
-
-    Window const& window = window_info.window();
-
-    auto format = mir_pixel_format_xrgb_8888;
-    WindowSpecification params;
-    params.size() = titlebar_size_for_window(window.size());
-    params.name() = "decoration";
-    params.pixel_format() = format;
-    params.buffer_usage() = WindowSpecification::BufferUsage::software;
-    params.top_left() = titlebar_position_for_window(window.top_left());
-    params.type() = mir_surface_type_gloss;
-
-    auto& titlebar_info = tools->build_window(window.application(), params);
-    titlebar_info.window().set_alpha(0.9);
-    titlebar_info.parent(window);
-
-    auto data = std::make_shared<TitlebarUserData>(titlebar_info.window());
-    window_info.userdata(data);
-    window_info.add_child(titlebar_info.window());
+    auto const application = window_info.window().application();
+
+    if (application == titlebar_provider->session())
+    {
+        titlebar_provider->advise_new_titlebar(window_info);
+        tools->raise_tree(window_info.parent());
+        return;
+    }
+
+    if (application == spinner.session() || !window_info.needs_titlebar(window_info.type()))
+        return;
+
+    titlebar_provider->create_titlebar_for(window_info.window());
 }
 
 void TitlebarWindowManagerPolicy::advise_focus_lost(WindowInfo const& info)
 {
     CanonicalWindowManagerPolicy::advise_focus_lost(info);
 
-    if (auto const titlebar = std::static_pointer_cast<TitlebarUserData>(info.userdata()))
-    {
-        titlebar->paint_titlebar(0x3F);
-    }
+    titlebar_provider->paint_titlebar_for(info.window(), 0x3F);
 }
 
 void TitlebarWindowManagerPolicy::advise_focus_gained(WindowInfo const& info)
 {
     CanonicalWindowManagerPolicy::advise_focus_gained(info);
 
-    if (auto const titlebar = std::static_pointer_cast<TitlebarUserData>(info.userdata()))
-    {
-        titlebar->paint_titlebar(0xFF);
-    }
+    titlebar_provider->paint_titlebar_for(info.window(), 0xFF);
 
     // Frig to force the spinner to the top
     if (auto const spinner_session = spinner.session())
@@ -152,52 +122,21 @@
 {
     CanonicalWindowManagerPolicy::advise_state_change(window_info, state);
 
-    if (auto const titlebar = std::static_pointer_cast<TitlebarUserData>(window_info.userdata()))
-    {
-        switch (state)
-        {
-        case mir_surface_state_restored:
-            titlebar->window.resize(titlebar_size_for_window(window_info.restore_rect().size));
-            titlebar->window.show();
-            break;
-
-        case mir_surface_state_maximized:
-        case mir_surface_state_vertmaximized:
-        case mir_surface_state_hidden:
-        case mir_surface_state_minimized:
-            titlebar->window.hide();
-            break;
-
-        case mir_surface_state_horizmaximized:
-            titlebar->window.resize(titlebar_size_for_window({display_area.size.width, window_info.restore_rect().size.height}));
-            titlebar->window.show();
-            break;
-
-        case mir_surface_state_fullscreen:
-        default:
-            break;
-        }
-    }
+    titlebar_provider->advise_state_change(window_info, state, display_area);
 }
 
 void TitlebarWindowManagerPolicy::advise_resize(WindowInfo const& window_info, Size const& new_size)
 {
     CanonicalWindowManagerPolicy::advise_resize(window_info, new_size);
 
-    if (auto const titlebar = std::static_pointer_cast<TitlebarUserData>(window_info.userdata()))
-    {
-        titlebar->window.resize({new_size.width, Height{title_bar_height}});
-    }
+    titlebar_provider->resize_titlebar_for(window_info.window(), new_size);
 }
 
 void TitlebarWindowManagerPolicy::advise_delete_window(WindowInfo const& window_info)
 {
     CanonicalWindowManagerPolicy::advise_delete_window(window_info);
 
-    if (auto const titlebar = std::static_pointer_cast<TitlebarUserData>(window_info.userdata()))
-    {
-        tools->destroy(titlebar->window);
-    }
+    titlebar_provider->destroy_titlebar_for(window_info.window());
 }
 
 void TitlebarWindowManagerPolicy::handle_displays_updated(Rectangles const& displays)
@@ -206,3 +145,23 @@
 
     display_area = displays.bounding_rectangle();
 }
+
+bool TitlebarWindowManagerPolicy::handle_keyboard_event(MirKeyboardEvent const* event)
+{
+    if (miral::CanonicalWindowManagerPolicy::handle_keyboard_event(event))
+        return true;
+
+    // TODO this is a workaround for the lack of a way to detect server exit (Mir bug lp:1593655)
+    // We need to exit the titlebar_provider "client" thread before the server exits
+    auto const action = mir_keyboard_event_action(event);
+    auto const scan_code = mir_keyboard_event_scan_code(event);
+    auto const modifiers = mir_keyboard_event_modifiers(event) & modifier_mask;
+
+    if (action == mir_keyboard_action_down && scan_code == KEY_BACKSPACE &&
+        (modifiers == (mir_input_event_modifier_alt | mir_input_event_modifier_ctrl)))
+    {
+        titlebar_provider->stop();
+    }
+
+    return false;
+}

=== modified file 'miral-shell/titlebar_window_manager.h'
--- miral-shell/titlebar_window_manager.h	2016-06-17 16:50:29 +0000
+++ miral-shell/titlebar_window_manager.h	2016-07-07 15:12:19 +0000
@@ -23,14 +23,20 @@
 
 #include <miral/canonical_window_manager.h>
 
+namespace miral { class InternalClientLauncher; }
+
 using namespace mir::geometry;
 
+class TitlebarProvider;
+
 class TitlebarWindowManagerPolicy : public miral::CanonicalWindowManagerPolicy
 {
 public:
-    TitlebarWindowManagerPolicy(miral::WindowManagerTools* const tools, SpinnerSplash const& spinner);
+    TitlebarWindowManagerPolicy(miral::WindowManagerTools* const tools, SpinnerSplash const& spinner, miral::InternalClientLauncher const& launcher);
+    ~TitlebarWindowManagerPolicy();
 
     bool handle_pointer_event(MirPointerEvent const* event) override;
+    bool handle_keyboard_event(MirKeyboardEvent const* event) override;
 
     void advise_new_window(miral::WindowInfo& window_info) override;
     void advise_focus_lost(miral::WindowInfo const& info) override;
@@ -40,10 +46,13 @@
     void advise_delete_window(miral::WindowInfo const& window_info) override;
 
     void handle_displays_updated(Rectangles const& displays) override;
+
 private:
     miral::WindowManagerTools* const tools;
     SpinnerSplash const spinner;
 
+    std::unique_ptr<TitlebarProvider> const titlebar_provider;
+
     Rectangle display_area;
     Point old_cursor{};
 };

=== modified file 'miral/application.cpp'
--- miral/application.cpp	2016-04-22 09:24:24 +0000
+++ miral/application.cpp	2016-07-07 15:12:19 +0000
@@ -20,10 +20,17 @@
 
 #include <mir/scene/session.h>
 
+#include <unistd.h>
+
 #include <csignal>
 
 void miral::kill(Application const& application, int sig)
 {
     if (application)
-        ::kill(application->process_id(), sig);
+    {
+        auto const pid = application->process_id();
+
+        if (pid != getpid())
+            ::kill(pid, sig);
+    }
 }

=== modified file 'miral/basic_window_manager.cpp'
--- miral/basic_window_manager.cpp	2016-06-17 16:50:29 +0000
+++ miral/basic_window_manager.cpp	2016-07-07 15:12:19 +0000
@@ -69,15 +69,6 @@
 {
     auto spec = spec_;
 
-#if MIR_SERVER_VERSION >= MIR_VERSION_NUMBER(0, 22, 0)
-    // Quick, dirty hack to support titlebar creation - really need an API for buffer stream creation
-    if (!spec.content_id().is_set() && !spec.streams().is_set())
-    {
-        mir::graphics::BufferProperties properties(spec.size().value(), spec.pixel_format().value(), mir::graphics::BufferUsage::software);
-        spec.content_id() = BufferStreamId{application->create_buffer_stream(properties).as_value()};
-    }
-#endif
-
     auto result = surface_builder(application, spec);
     auto& info = window_info.emplace(result, WindowInfo{result, spec}).first->second;
     if (spec.parent().is_set() && spec.parent().value().lock())
@@ -202,13 +193,6 @@
     }
 }
 
-void miral::BasicWindowManager::destroy(Window& window)
-{
-    erase(info_for(window));
-
-    window.application()->destroy_surface(window);
-}
-
 void miral::BasicWindowManager::erase(miral::WindowInfo const& info)
 {
     if (auto const parent = info.parent())
@@ -901,7 +885,7 @@
         {
             // select_active_window() calls set_focus_to() which updates mru_active_windows and changes window
             auto const w = window;
-            return w.application() != session || !(new_focus = miral::BasicWindowManager::select_active_window(w));
+            return w.application() != session || !(new_focus = select_active_window(w));
         });
 
     return new_focus;

=== modified file 'miral/basic_window_manager.h'
--- miral/basic_window_manager.h	2016-06-17 16:50:29 +0000
+++ miral/basic_window_manager.h	2016-07-07 15:12:19 +0000
@@ -55,9 +55,6 @@
         std::shared_ptr<mir::shell::DisplayLayout> const& display_layout,
         WindowManagementPolicyBuilder const& build);
 
-    auto build_window(Application const& application, WindowSpecification const& spec)
-    -> WindowInfo& override;
-
     void add_session(std::shared_ptr<mir::scene::Session> const& session) override;
 
     void remove_session(std::shared_ptr<mir::scene::Session> const& session) override;
@@ -77,8 +74,6 @@
         std::shared_ptr<mir::scene::Session> const& session,
         std::weak_ptr<mir::scene::Surface> const& surface) override;
 
-    void destroy(Window& window) override;
-
     void add_display(mir::geometry::Rectangle const& area) override;
 
     void remove_display(mir::geometry::Rectangle const& area) override;
@@ -168,6 +163,8 @@
 
     auto place_new_surface(ApplicationInfo const& app_info, WindowSpecification parameters)
         -> WindowSpecification;
+    auto build_window(Application const& application, WindowSpecification const& spec)
+        -> WindowInfo&;
     void move_tree(miral::WindowInfo& root, mir::geometry::Displacement movement);
     void erase(miral::WindowInfo const& info);
 };