Merge lp:~vanvugt/mir/surface-types into lp:~mir-team/mir/trunk

[Daniel van Vugt]

Add support for surface types (to the client API, protocol and server).

This work will also form the foundation of supporting surface states and
necessitates a prototype of the beginnings of a shell interface.

THIS IS A PREVIEW

It's not intended for approval, but intended for early review. I need to write more test cases in the very least. However before then, I have a long weekend to see to :)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:498
http://s-jenkins:8080/job/mir-ci/1116/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/mir-ci/./build=pbuilder,distribution=quantal,flavor=amd64/1116/console

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

87 === modified file 'include/mir/graphics/display.h'
...
94 +#include "mir/shell/shell.h"

This feels very wrong - I don't believe that graphics::Display needs to know about shell::Shell.

Unless there is a compelling reason for something different, there should be an interface in *graphics* that determines the functionality that graphics::Display needs, and that will be implemented by shell::Shell.

~~~~

102 + void set_shell(std::shared_ptr<mir::Shell> s);
103 + bool start_shell();
104 + void stop_shell();
105 + std::shared_ptr<mir::Shell> current_shell() const;

I find it very hard to believe that these are functions that belong to graphics::Display.

In addition, graphics::Display is an interface and should not provide an implementation. (It is used in compositing - it should probably be there, but that's an existing issue.)

~~~~

252 +#include <mir/surface.h>

Don't use system includes for our files.

~~~~

298 +#ifndef __MIR_COMMON_SURFACE_H__
299 +#define __MIR_COMMON_SURFACE_H__

Does not match project naming style

~~~~

307 + MIR_SURFACE_TYPE,
308 + MirSurfaceAttrib_ARRAYSIZE

Does not match project naming style

~~~~

608 === added file 'src/display.cpp'

This is a confusing place to implement mir::graphics::Display

~~~~

633 +using namespace mir;
634 +using namespace mir::graphics;
635 +using namespace mir::shell;

This is a very different style to the rest of the codebase. I don't like it as it isn't clear where the classes mentioned in the code come from. (It can also lead to free functions being defined in global namespace and not the intended namespace.)

~~~~

735 + auto const shell = graphics_display->current_shell();
736 + if (!shell->supports(attrib))
737 + BOOST_THROW_EXCEPTION(std::logic_error("Current shell does not "
738 + "support the specified surface "
739 + "attribute"));

Application logic doesn't belong in ApplicationMediator.

~~~

814 +message SurfaceSetting {
815 + required SurfaceId surfaceid = 1;
816 + required int32 attrib = 2;
817 + optional int32 ivalue = 3;
818 + // optional string svalue = 4; // Expected for future use
819 +
820 + optional string error = 127;
821 +}

As this is returned by the RPC framework all the fields should be optional (as, in the event of an exception, it may be the only field populated).

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Alan,

"Shell in Display"
I kind of agree. This was the last part I wrote and I later saw one or two alternative places to attach Shell other than inside Display. But from a conceptional standpoint, can we discuss why the "shell" is not a 1:1 part of a "display"?

"#include <mir/surface.h>"
Yes this is correct because it is included from a public system header (shell.h). Hence angle brackets are required.

"__MIR_COMMON_SURFACE_H__"
Yeah I can change the style no problem.

"MIR_SURFACE_TYPE"
This is C code. Not C++. What is the established convention for C-compatible enums that I should change to?

"src/display.cpp"
It's not really a confusing place because it's the same directory where other sources relating to it already reside. If I put display.cpp in a subdir then it has to be built into a static library and then bundled into the server library. Seems like an overkill for one small source file right now.

"using namespace mir"
I prefer this because it makes the code much more readable, and will argue to the very end that the convention seen in existing mir code is ugly, hard to read and more time-consuming to maintain. It is never confusing where a symbol comes from if it is only defined in one place in the project.

"Application logic doesn't belong in ApplicationMediator."
Yeah I had the same feeling. Just haven't yet found a cleaner place to put it where the logic had full access to all the objects in question.

"required vs optional"
I don't care enough to argue. Will change it.

All in all, I think that's a good initial review. Good only because I only disagree deeply with one of the points made.

=== modified file 'debian/control'
--- debian/control	2013-03-04 20:11:18 +0000
+++ debian/control	2013-03-07 08:31:22 +0000
@@ -56,11 +56,22 @@
  .
  Contains the shared library needed by server applications for Mir.
 
+Package: mircommon-dev
+Section: libdevel
+Architecture: any
+Depends: ${misc:Depends},
+Description: Display server for Ubuntu - development headers
+ Mir is a display server running on linux systems, with a focus on efficiency, 
+ robust operation and a well-defined driver model.
+ .
+ Contains header files required for server and/or client development.
+
 Package: libmirserver-dev
 Section: libdevel
 Architecture: any
 Depends: libmirserver0 (= ${binary:Version}),
          libmirprotobuf-dev (= ${source:Version}),
+         mircommon-dev (= ${source:Version}),
          ${misc:Depends},
 Description: Display server for Ubuntu - development headers
  Mir is a display server running on linux systems, with a focus on efficiency, 
@@ -84,6 +95,7 @@
 Architecture: any
 Depends: libmirclient0 (= ${binary:Version}),
          libmirprotobuf-dev (= ${source:Version}),
+         mircommon-dev (= ${source:Version}),
          ${misc:Depends},
 Description: Display server for Ubuntu - development headers
  Mir is a display server running on linux systems, with a focus on efficiency, 

=== modified file 'debian/libmirserver-dev.install'
--- debian/libmirserver-dev.install	2013-02-01 11:08:00 +0000
+++ debian/libmirserver-dev.install	2013-03-07 08:31:22 +0000
@@ -1,2 +1,3 @@
+usr/include/mir/server
 usr/lib/pkgconfig/mirserver.pc
 usr/lib/libmirserver.so

=== added file 'debian/mircommon-dev.install'
--- debian/mircommon-dev.install	1970-01-01 00:00:00 +0000
+++ debian/mircommon-dev.install	2013-03-07 08:31:22 +0000
@@ -0,0 +1,1 @@
+usr/include/mir/*.h

=== modified file 'include/mir/frontend/application_mediator.h'
--- include/mir/frontend/application_mediator.h	2013-03-06 12:25:34 +0000
+++ include/mir/frontend/application_mediator.h	2013-03-07 08:31:22 +0000
@@ -103,6 +103,11 @@
                         mir::protobuf::DRMAuthMagicStatus* response,
                         google::protobuf::Closure* done);
 
+    void configure_surface(google::protobuf::RpcController* controller,
+                        const mir::protobuf::SurfaceSetting*,
+                        mir::protobuf::SurfaceSetting*,
+                        google::protobuf::Closure* done);
+    
 private:
     std::shared_ptr<shell::SessionStore> const session_store;
     std::shared_ptr<graphics::Platform> const graphics_platform;

=== modified file 'include/mir/frontend/application_mediator_report.h'
--- include/mir/frontend/application_mediator_report.h	2013-03-04 16:02:02 +0000
+++ include/mir/frontend/application_mediator_report.h	2013-03-07 08:31:22 +0000
@@ -41,6 +41,7 @@
     virtual void application_disconnect_called(std::string const& app_name) = 0;
 
     virtual void application_drm_auth_magic_called(std::string const& app_name) = 0;
+    virtual void application_configure_surface_called(std::string const& app_name) = 0;
 
     virtual void application_error(
         std::string const& app_name,
@@ -63,6 +64,8 @@
 
     virtual void application_drm_auth_magic_called(std::string const& app_name);
 
+    virtual void application_configure_surface_called(std::string const& app_name);
+
     virtual void application_error(
         std::string const& app_name,
         char const* method,

=== modified file 'include/mir/logging/application_mediator_report.h'
--- include/mir/logging/application_mediator_report.h	2013-03-04 16:02:02 +0000
+++ include/mir/logging/application_mediator_report.h	2013-03-07 08:31:22 +0000
@@ -47,6 +47,8 @@
 
     virtual void application_drm_auth_magic_called(std::string const& app_name);
 
+    virtual void application_configure_surface_called(std::string const& app_name);
+
     virtual void application_error(
         std::string const& app_name,
         char const* method,

=== modified file 'include/mir/shell/application_session.h'
--- include/mir/shell/application_session.h	2013-03-06 12:25:34 +0000
+++ include/mir/shell/application_session.h	2013-03-07 08:31:22 +0000
@@ -25,6 +25,7 @@
 
 namespace mir
 {
+class Shell;
 
 namespace shell
 {
@@ -33,7 +34,10 @@
 class ApplicationSession : public Session
 {
 public:
-    explicit ApplicationSession(std::shared_ptr<SurfaceFactory> const& surface_factory, std::string const& session_name);
+    explicit ApplicationSession(
+        std::shared_ptr<SurfaceFactory> const& surface_factory,
+        std::string const& session_name,
+        std::shared_ptr<mir::Shell> sh = nullptr);
     ~ApplicationSession();
 
     SurfaceId create_surface(SurfaceCreationParameters const& params);
@@ -46,6 +50,8 @@
     void hide();
     void show();
 
+    int configure_surface(SurfaceId id, MirSurfaceAttrib attrib, int value);
+
 protected:
     ApplicationSession(ApplicationSession const&) = delete;
     ApplicationSession& operator=(ApplicationSession const&) = delete;
@@ -53,6 +59,7 @@
 private:
     std::shared_ptr<SurfaceFactory> const surface_factory;
     std::string const session_name;
+    std::shared_ptr<mir::Shell> shell;
 
     SurfaceId next_id();
 

=== added file 'include/mir/shell/nullshell.h'
--- include/mir/shell/nullshell.h	1970-01-01 00:00:00 +0000
+++ include/mir/shell/nullshell.h	2013-03-07 08:31:22 +0000
@@ -0,0 +1,43 @@
+/*
+ * Null Shell
+ *
+ * Copyright © 2013 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/>.
+ *
+ * Author: Daniel van Vugt <[e-mail]>
+ */
+
+#ifndef MIR_SHELL_NULLSHELL_H_
+#define MIR_SHELL_NULLSHELL_H_
+
+#include "mir/shell/shell.h"
+
+namespace mir
+{
+namespace shell
+{
+
+class NullShell : public mir::Shell
+{
+public:
+    ~NullShell();
+
+    bool supports(MirSurfaceAttrib attrib) const;
+    bool supports(MirSurfaceAttrib attrib, int value) const;
+};
+
+} // namespace shell
+} // namespace mir
+
+#endif

=== modified file 'include/mir/shell/session.h'
--- include/mir/shell/session.h	2013-03-06 12:25:34 +0000
+++ include/mir/shell/session.h	2013-03-07 08:31:22 +0000
@@ -19,6 +19,7 @@
 #ifndef MIR_SHELL_SESSION_H_
 #define MIR_SHELL_SESSION_H_
 
+#include "mir/surface.h"  // for MirSurfaceAttrib
 #include "mir/shell/surface_id.h"
 
 #include <mutex>
@@ -50,6 +51,8 @@
     virtual void hide() = 0;
     virtual void show() = 0;
 
+    virtual int configure_surface(SurfaceId id, MirSurfaceAttrib attrib, int value) = 0;
+
 protected:
     Session() = default;
     Session(Session const&) = delete;

=== modified file 'include/mir/shell/session_manager.h'
--- include/mir/shell/session_manager.h	2013-03-06 12:25:34 +0000
+++ include/mir/shell/session_manager.h	2013-03-07 08:31:22 +0000
@@ -52,6 +52,8 @@
     virtual void tag_session_with_lightdm_id(std::shared_ptr<Session> const& session, int id);
     virtual void focus_session_with_lightdm_id(int id);
 
+    virtual std::shared_ptr<mir::Shell> current_shell() const;
+
     void focus_next();
 
 protected:
@@ -64,6 +66,8 @@
     std::shared_ptr<FocusSequence> const focus_sequence;
     std::shared_ptr<FocusSetter> const focus_setter;
 
+    mutable std::shared_ptr<mir::Shell> shell;
+
     std::mutex mutex;
     std::weak_ptr<Session> focus_application;
     typedef std::vector<std::pair<int, std::shared_ptr<Session>>> Tags;

=== modified file 'include/mir/shell/session_store.h'
--- include/mir/shell/session_store.h	2013-03-06 12:25:34 +0000
+++ include/mir/shell/session_store.h	2013-03-07 08:31:22 +0000
@@ -20,6 +20,7 @@
 #define MIR_SHELL_SESSION_STORE_H_
 
 #include <memory>
+#include "mir/shell/shell.h"
 
 namespace mir
 {
@@ -29,6 +30,23 @@
 
 class Session;
 
+/*
+ * The word "Session" is overloaded and over-abstracted here. "Session" can
+ * be used to represent either an application or a login. Both are sessional
+ * concepts capable of creating and destroying surfaces. This seems
+ * straightforward but what then is a SessionStore?
+ *
+ * A SessionStore is anything capable of opening and closing sessions. So by
+ * the previous definition, a SessionStore could be any of:
+ *  - a single login (creates App sessions, but is also a session itself)
+ *  - a single seat (usually one per machine; creates login sessions)
+ *
+ * It seems feasible that in future someone will specialize Session and
+ * SessionStore into more concrete classes like "Application", "Login" and
+ * "Seat". That would be easier to comprehend, but is a job for another day.
+ * - Daniel
+ */
+
 class SessionStore
 {
 public:
@@ -42,6 +60,8 @@
 
     virtual void shutdown() = 0;
 
+    virtual std::shared_ptr<mir::Shell> current_shell() const = 0;
+
 protected:
     SessionStore() = default;
     SessionStore(const SessionStore&) = delete;

=== added file 'include/mir/shell/shell.h'
--- include/mir/shell/shell.h	1970-01-01 00:00:00 +0000
+++ include/mir/shell/shell.h	2013-03-07 08:31:22 +0000
@@ -0,0 +1,42 @@
+/*
+ * Mir shell interface PUBLIC HEADER
+ *
+ * Copyright © 2013 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/>.
+ *
+ * Author: Daniel van Vugt <[e-mail]>
+ */
+
+#ifndef MIR_SHELL_SHELL_H_
+#define MIR_SHELL_SHELL_H_
+
+#include <mir/surface.h>
+
+namespace mir
+{
+
+class Shell
+{
+public:
+    virtual ~Shell() {}
+
+    virtual bool supports(MirSurfaceAttrib attrib) const = 0;
+    virtual bool supports(MirSurfaceAttrib attrib, int value) const = 0;
+
+    // TODO: Lots more will go here I guess.
+};
+
+} // namespace mir
+
+#endif

=== modified file 'include/mir/shell/surface.h'
--- include/mir/shell/surface.h	2013-03-06 12:25:34 +0000
+++ include/mir/shell/surface.h	2013-03-07 08:31:22 +0000
@@ -23,6 +23,7 @@
 #include "mir/geometry/pixel_format.h"
 #include "mir/geometry/point.h"
 #include "mir/geometry/size.h"
+#include "mir/surface.h"
 
 #include <memory>
 
@@ -55,6 +56,8 @@
     virtual void advance_client_buffer() = 0;
     virtual std::shared_ptr<compositor::Buffer> client_buffer() const = 0;
 
+    virtual int configure(MirSurfaceAttrib attrib, int value) = 0;
+
 protected:
     Surface() = default;
     Surface(Surface const&) = delete;

=== added file 'include/mir/surface.h'
--- include/mir/surface.h	1970-01-01 00:00:00 +0000
+++ include/mir/surface.h	2013-03-07 08:31:22 +0000
@@ -0,0 +1,48 @@
+/*
+ * Surface-related declarations common to client and server
+ * This is C. Not C++.
+ *
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser 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 Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Author: Daniel van Vugt <[e-mail]>
+ */
+
+#ifndef MIR_SURFACE_H_
+#define MIR_SURFACE_H_
+
+/*
+ * Attributes of a surface that the client and server/shell may wish to
+ * get or set over the wire.
+ */
+typedef enum MirSurfaceAttrib
+{
+    MIR_SURFACE_TYPE,
+    MirSurfaceAttrib_ARRAYSIZE
+} MirSurfaceAttrib;
+
+typedef enum MirSurfaceType
+{
+    MIR_SURFACE_NORMAL,
+    MIR_SURFACE_UTILITY,
+    MIR_SURFACE_DIALOG,
+    MIR_SURFACE_OVERLAY,
+    MIR_SURFACE_FREESTYLE,
+    MIR_SURFACE_POPOVER,
+    MirSurfaceType_ARRAYSIZE
+} MirSurfaceType;
+
+/* TODO: Surface states here */
+
+#endif

=== modified file 'include/mir/surfaces/surface.h'
--- include/mir/surfaces/surface.h	2013-03-04 16:02:02 +0000
+++ include/mir/surfaces/surface.h	2013-03-07 08:31:22 +0000
@@ -22,6 +22,7 @@
 #include "mir/geometry/pixel_format.h"
 #include "mir/graphics/renderable.h"
 #include "mir/compositor/buffer_properties.h"
+#include "mir/surface.h"
 
 #include <memory>
 #include <string>
@@ -71,6 +72,10 @@
     std::shared_ptr<compositor::Buffer> client_buffer() const;
     void shutdown();
 
+    int configure(MirSurfaceAttrib attrib, int value);
+    MirSurfaceType type() const;
+    bool set_type(MirSurfaceType t);
+
 private:
     std::string surface_name;
     std::shared_ptr<BufferBundle> buffer_bundle;
@@ -81,6 +86,8 @@
     float alpha_value;
 
     bool is_hidden;
+
+    MirSurfaceType type_value;
 };
 
 }

=== modified file 'include/mir_toolkit/mir_client_library.h'
--- include/mir_toolkit/mir_client_library.h	2013-03-04 16:02:02 +0000
+++ include/mir_toolkit/mir_client_library.h	2013-03-07 08:31:22 +0000
@@ -18,6 +18,8 @@
 #ifndef MIR_CLIENT_LIBRARY_H
 #define MIR_CLIENT_LIBRARY_H
 
+#include <mir/surface.h>  /* Surface definitions common to client & server */
+
 #ifdef __cplusplus
 /** The C client API
  */
@@ -322,6 +324,23 @@
  */
 int mir_debug_surface_id(MirSurface *surface);
 
+/**
+ * Set the type (purpose) of a surface. This is not guaranteed to always work
+ * with some shell types (e.g. phone/tablet UIs). As such, you may have to
+ * wait on the function and check the result using mir_surface_get_type.
+ *   \param [in] surface  The surface to operate on
+ *   \param [in] type     The new type of the surface
+ *   \return              A wait handle that can be passed to mir_wait_for
+ */
+MirWaitHandle* mir_surface_set_type(MirSurface *surface, MirSurfaceType type);
+
+/**
+ * Get the type (purpose) of a surface from the server.
+ *   \param [in] surface  The surface to query
+ *   \return              The type of the surface
+ */
+MirSurfaceType mir_surface_get_type(MirSurface *surface);
+
 #ifdef __cplusplus
 }
 }

=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2013-03-06 12:25:34 +0000
+++ src/CMakeLists.txt	2013-03-07 08:31:22 +0000
@@ -45,6 +45,7 @@
   mirgraphics
   mirinput
   mirsurfaces
+  mirnullshell
 )
 
 list(APPEND MIRSERVER_LINKS
@@ -63,9 +64,6 @@
     LINK_PUBLIC
       ${MIRSERVER_LINKS}
   )
-  install(TARGETS mirserver
-    LIBRARY DESTINATION lib
-  )
 else()
   target_link_libraries(mirserver
     LINK_PUBLIC
@@ -74,9 +72,19 @@
   )
 endif()
 
+set (MIR_SERVER_HEADERS
+  ${CMAKE_SOURCE_DIR}/include/mir/shell/shell.h
+)
+
 set_target_properties(mirserver PROPERTIES
   VERSION ${MIR_VERSION_MAJOR}.${MIR_VERSION_MINOR}.${MIR_VERSION_PATCH}
   SOVERSION ${MIR_VERSION_MAJOR}
+  PUBLIC_HEADER ${MIR_SERVER_HEADERS}
+)
+
+install (TARGETS mirserver
+  LIBRARY DESTINATION lib
+  PUBLIC_HEADER DESTINATION "include/mir/server/"
 )
 
 target_link_libraries(mirds
@@ -84,6 +92,14 @@
   ${Boost_LIBRARIES}
 )
 
+# Some headers contain common definitions required for both client and
+# server development...
+set (MIR_COMMON_HEADERS
+ ${CMAKE_SOURCE_DIR}/include/mir/surface.h
+)
+
+install (FILES ${MIR_COMMON_HEADERS} DESTINATION "include/mir/")
+
 install(TARGETS mirds
   RUNTIME DESTINATION bin
 )

=== modified file 'src/client/mir_client_library.cpp'
--- src/client/mir_client_library.cpp	2013-03-04 16:02:02 +0000
+++ src/client/mir_client_library.cpp	2013-03-07 08:31:22 +0000
@@ -227,3 +227,25 @@
 {
     // Ignore
 }
+
+mir_toolkit::MirWaitHandle* mir_toolkit::mir_surface_set_type(MirSurface *surf,
+                                                           MirSurfaceType type)
+{
+    return surf ? surf->configure(MIR_SURFACE_TYPE, type) : NULL;
+}
+
+MirSurfaceType mir_toolkit::mir_surface_get_type(MirSurface *surf)
+{
+    MirSurfaceType type = MIR_SURFACE_NORMAL;
+
+    if (surf)
+    {
+        // I assume the type can only change from the client side. Otherwise
+        // we would have to send off a message to retrieve the latest...
+
+        int t = surf->attrib(MIR_SURFACE_TYPE);
+        type = static_cast<MirSurfaceType>(t);
+    }
+
+    return type;
+}

=== modified file 'src/client/mir_surface.cpp'
--- src/client/mir_surface.cpp	2013-03-04 16:02:02 +0000
+++ src/client/mir_surface.cpp	2013-03-07 08:31:22 +0000
@@ -50,6 +50,10 @@
     message.set_buffer_usage(params.buffer_usage);
 
     server.create_surface(0, &message, &surface, gp::NewCallback(this, &MirSurface::created, callback, context));
+
+    for (int i = 0; i < MirSurfaceAttrib_ARRAYSIZE; i++)
+        attrib_cache[i] = -1;
+    attrib_cache[MIR_SURFACE_TYPE] = MIR_SURFACE_NORMAL;
 }
 
 mir_toolkit::MirSurface::~MirSurface()
@@ -226,3 +230,46 @@
 {
     return *accelerated_window;
 }
+
+MirWaitHandle* MirSurface::configure(MirSurfaceAttrib at, int value)
+{
+    mp::SurfaceSetting setting;
+    setting.mutable_surfaceid()->CopyFrom(surface.id());
+    setting.set_attrib(at);
+    setting.set_ivalue(value);
+
+    configure_wait_handle.expect_result();
+    server.configure_surface(0, &setting, &configure_result, 
+              google::protobuf::NewCallback(this, &MirSurface::on_configured));
+
+    return &configure_wait_handle;
+}
+
+void MirSurface::on_configured()
+{
+    if (configure_result.has_surfaceid() &&
+        configure_result.surfaceid().value() == surface.id().value() &&
+        configure_result.has_attrib())
+    {
+        switch (configure_result.attrib())
+        {
+        case MIR_SURFACE_TYPE:
+            if (configure_result.has_ivalue())
+            {
+                int t = configure_result.ivalue();
+                attrib_cache[MIR_SURFACE_TYPE] = t;
+            } // else error is probably set due to an unsupported attrib/value
+            break;
+        default:
+            assert(false);
+            break;
+        }
+
+        configure_wait_handle.result_received();
+    }
+}
+
+int MirSurface::attrib(MirSurfaceAttrib at) const
+{
+    return attrib_cache[at];
+}

=== modified file 'src/client/mir_surface.h'
--- src/client/mir_surface.h	2013-03-04 16:02:02 +0000
+++ src/client/mir_surface.h	2013-03-07 08:31:22 +0000
@@ -23,6 +23,8 @@
 #include "mir/geometry/pixel_format.h"
 #include "mir/geometry/dimensions.h"
 #include "mir_toolkit/mir_client_library.h"
+#include "mir/surface.h"
+#include "mir_logger.h"
 #include "client_buffer_depository.h"
 #include "mir_wait_handle.h"
 #include "mir_client_surface.h"
@@ -72,7 +74,11 @@
     void release_cpu_region();
     EGLNativeWindowType generate_native_window();
 
+    MirWaitHandle* configure(MirSurfaceAttrib a, int value);
+    int  attrib(MirSurfaceAttrib a) const;
+
 private:
+    void on_configured();
     void process_incoming_buffer();
     void populate(MirBufferPackage& buffer_package);
     void created(mir_surface_lifecycle_callback callback, void * context);
@@ -88,6 +94,7 @@
     MirConnection *connection;
     MirWaitHandle create_wait_handle;
     MirWaitHandle next_buffer_wait_handle;
+    MirWaitHandle configure_wait_handle;
 
     int last_buffer_id;
 
@@ -96,6 +103,11 @@
 
     std::shared_ptr<mir::client::Logger> logger;
     std::shared_ptr<EGLNativeWindowType> accelerated_window;
+
+    mir::protobuf::SurfaceSetting configure_result;
+
+    // Cache of latest SurfaceSettings returned from the server
+    int attrib_cache[MirSurfaceAttrib_ARRAYSIZE];
 };
 
 #endif /* MIR_CLIENT_PRIVATE_MIR_WAIT_HANDLE_H_ */

=== modified file 'src/client/mir_wait_handle.cpp'
--- src/client/mir_wait_handle.cpp	2013-03-04 16:02:02 +0000
+++ src/client/mir_wait_handle.cpp	2013-03-07 08:31:22 +0000
@@ -14,6 +14,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  * Authored by: Kevin DuBois <[e-mail]>
+ *              Daniel van Vugt <[e-mail]>
  */
 
 #include "mir_wait_handle.h"
@@ -21,7 +22,8 @@
 mir_toolkit::MirWaitHandle::MirWaitHandle() :
     guard(),
     wait_condition(),
-    result_has_occurred(false)
+    expecting(0),
+    received(0)
 {
 }
 
@@ -29,20 +31,29 @@
 {
 }
 
+void mir_toolkit::MirWaitHandle::expect_result()
+{
+    std::unique_lock<std::mutex> lock(guard);
+
+    expecting++;
+}
+
 void mir_toolkit::MirWaitHandle::result_received()
 {
     std::unique_lock<std::mutex> lock(guard);
-    result_has_occurred = true;
 
+    received++;
     wait_condition.notify_all();
 }
 
 void mir_toolkit::MirWaitHandle::wait_for_result()
 {
     std::unique_lock<std::mutex> lock(guard);
-    while ( (!result_has_occurred) )
+
+    while ((!expecting && !received) || (received < expecting))
         wait_condition.wait(lock);
-    result_has_occurred = false;
+
+    received = 0;
+    expecting = 0;
 }
 
-

=== modified file 'src/client/mir_wait_handle.h'
--- src/client/mir_wait_handle.h	2013-03-04 16:02:02 +0000
+++ src/client/mir_wait_handle.h	2013-03-07 08:31:22 +0000
@@ -14,6 +14,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  * Authored by: Thomas Guest <[e-mail]>
+ *              Daniel van Vugt <[e-mail]>
  */
 
 #ifndef MIR_CLIENT_MIR_WAIT_HANDLE_H_
@@ -30,6 +31,7 @@
     MirWaitHandle();
     ~MirWaitHandle();
 
+    void expect_result();
     void result_received();
     void wait_for_result();
 
@@ -37,7 +39,8 @@
     std::mutex guard;
     std::condition_variable wait_condition;
 
-    bool result_has_occurred;
+    int expecting;
+    int received;
 };
 }
 

=== modified file 'src/frontend/application_mediator.cpp'
--- src/frontend/application_mediator.cpp	2013-03-06 12:25:34 +0000
+++ src/frontend/application_mediator.cpp	2013-03-07 08:31:22 +0000
@@ -24,6 +24,7 @@
 #include "mir/shell/surface_creation_parameters.h"
 #include "mir/frontend/resource_cache.h"
 
+#include "mir/surface.h"
 #include "mir/compositor/buffer_ipc_package.h"
 #include "mir/compositor/buffer_id.h"
 #include "mir/compositor/buffer.h"
@@ -220,3 +221,29 @@
     done->Run();
 }
 
+void mir::frontend::ApplicationMediator::configure_surface(
+    google::protobuf::RpcController*, // controller,
+    const mir::protobuf::SurfaceSetting* request,
+    mir::protobuf::SurfaceSetting* response,
+    google::protobuf::Closure* done)
+{
+    MirSurfaceAttrib attrib = static_cast<MirSurfaceAttrib>(request->attrib());
+
+    // Required response fields:
+    response->mutable_surfaceid()->CopyFrom(request->surfaceid());
+    response->set_attrib(attrib);
+
+    if (application_session.get() == nullptr)
+        BOOST_THROW_EXCEPTION(std::logic_error("Invalid application session"));
+
+    report->application_configure_surface_called(application_session->name());
+
+    auto const id = shell::SurfaceId(request->surfaceid().value());
+    int value = request->ivalue();
+    int newvalue = application_session->configure_surface(id, attrib, value);
+
+    response->set_ivalue(newvalue);
+
+    done->Run();
+}
+

=== modified file 'src/frontend/null_application_mediator_report.cpp'
--- src/frontend/null_application_mediator_report.cpp	2013-03-04 16:02:02 +0000
+++ src/frontend/null_application_mediator_report.cpp	2013-03-07 08:31:22 +0000
@@ -42,6 +42,10 @@
 {
 }
 
+void mir::frontend::NullApplicationMediatorReport::application_configure_surface_called(std::string const&)
+{
+}
+
 void mir::frontend::NullApplicationMediatorReport::application_error(
         std::string const&,
         char const* ,

=== modified file 'src/frontend/protobuf_message_processor.cpp'
--- src/frontend/protobuf_message_processor.cpp	2013-03-04 16:02:02 +0000
+++ src/frontend/protobuf_message_processor.cpp	2013-03-07 08:31:22 +0000
@@ -176,6 +176,10 @@
         {
             invoke(&protobuf::DisplayServer::select_focus_by_lightdm_id, invocation);
         }
+        else if ("configure_surface" == invocation.method_name())
+        {
+            invoke(&protobuf::DisplayServer::configure_surface, invocation);
+        }
         else if ("disconnect" == invocation.method_name())
         {
             invoke(&protobuf::DisplayServer::disconnect, invocation);

=== modified file 'src/logging/application_mediator_report.cpp'
--- src/logging/application_mediator_report.cpp	2013-03-04 16:02:02 +0000
+++ src/logging/application_mediator_report.cpp	2013-03-07 08:31:22 +0000
@@ -63,6 +63,11 @@
     log->log<Logger::informational>("application_drm_auth_magic_called(\"" + app_name + "\")", component);
 }
 
+void ml::ApplicationMediatorReport::application_configure_surface_called(std::string const& app_name)
+{
+    log->log<Logger::informational>("application_configure_surface_called(\"" + app_name + "\")", component);
+}
+
 void ml::ApplicationMediatorReport::application_error(
         std::string const& app_name,
         char const* method,

=== modified file 'src/protobuf/mir_protobuf.proto'
--- src/protobuf/mir_protobuf.proto	2013-01-10 16:58:01 +0000
+++ src/protobuf/mir_protobuf.proto	2013-03-07 08:31:22 +0000
@@ -111,6 +111,14 @@
   optional string error = 127;
 }
 
+message SurfaceSetting {
+  optional SurfaceId surfaceid = 1;
+  optional int32     attrib = 2;
+  optional int32     ivalue = 3;
+  // optional string    svalue = 4;  // Expected for future use
+  optional string error = 127;
+}
+
 service DisplayServer {
   // Platform independent requests
   rpc connect(ConnectParameters) returns (Connection);
@@ -124,5 +132,7 @@
   rpc drm_auth_magic(DRMMagic) returns (DRMAuthMagicStatus);
 
   rpc test_file_descriptors(Void) returns (Buffer);
+
+  rpc configure_surface(SurfaceSetting) returns (SurfaceSetting);
 }
 

=== modified file 'src/shell/CMakeLists.txt'
--- src/shell/CMakeLists.txt	2013-03-06 12:25:34 +0000
+++ src/shell/CMakeLists.txt	2013-03-07 08:31:22 +0000
@@ -16,3 +16,7 @@
 
   ${SHELL_SOURCES}
 )
+
+add_library (mirnullshell STATIC
+  nullshell.cpp
+)

=== modified file 'src/shell/application_session.cpp'
--- src/shell/application_session.cpp	2013-03-06 12:54:33 +0000
+++ src/shell/application_session.cpp	2013-03-07 08:31:22 +0000
@@ -20,6 +20,7 @@
 #include "mir/shell/surface.h"
 
 #include "mir/surfaces/surface_controller.h"
+#include "mir/shell/nullshell.h"
 
 #include <boost/throw_exception.hpp>
 
@@ -33,12 +34,16 @@
 
 msh::ApplicationSession::ApplicationSession(
     std::shared_ptr<msh::SurfaceFactory> const& surface_factory,
-    std::string const& session_name) :
+    std::string const& session_name, std::shared_ptr<mir::Shell> sh) :
     surface_factory(surface_factory),
     session_name(session_name),
+    shell(sh),
     next_surface_id(0)
 {
     assert(surface_factory);
+
+    if (!shell)
+        shell.reset(new mir::shell::NullShell);
 }
 
 msh::ApplicationSession::~ApplicationSession()
@@ -122,3 +127,23 @@
         id_s.second->show();
     }
 }
+
+int msh::ApplicationSession::configure_surface(msh::SurfaceId id,
+                                               MirSurfaceAttrib attrib,
+                                               int value)
+{
+    if (!shell->supports(attrib))
+        BOOST_THROW_EXCEPTION(std::logic_error("Current shell does not "
+                                               "support the specified surface "
+                                               "attribute"));
+
+    if (!shell->supports(attrib, value))
+        BOOST_THROW_EXCEPTION(std::logic_error("Current shell does not "
+                                               "support the specified value "
+                                               "for this attribute"));
+
+    std::unique_lock<std::mutex> lock(surfaces_mutex);
+    std::shared_ptr<msh::Surface> surf(checked_find(id)->second);
+
+    return surf->configure(attrib, value);
+}

=== added file 'src/shell/nullshell.cpp'
--- src/shell/nullshell.cpp	1970-01-01 00:00:00 +0000
+++ src/shell/nullshell.cpp	2013-03-07 08:31:22 +0000
@@ -0,0 +1,49 @@
+/*
+ * Null Shell
+ *
+ * Copyright © 2013 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/>.
+ *
+ * Author: Daniel van Vugt <[e-mail]>
+ */
+
+#include "mir/shell/nullshell.h"
+
+using namespace mir;
+using namespace shell;
+
+NullShell::~NullShell()
+{
+}
+
+bool NullShell::supports(MirSurfaceAttrib attrib) const
+{
+    return (attrib == MIR_SURFACE_TYPE);
+}
+
+bool NullShell::supports(MirSurfaceAttrib attrib, int value) const
+{
+    bool valid = false;
+
+    switch (attrib)
+    {
+    case MIR_SURFACE_TYPE:
+        valid = (value >= 0 && value < MirSurfaceType_ARRAYSIZE);
+        break;
+    default:
+        break;
+    }
+
+    return valid;
+}

=== modified file 'src/shell/session_manager.cpp'
--- src/shell/session_manager.cpp	2013-03-06 12:54:33 +0000
+++ src/shell/session_manager.cpp	2013-03-07 08:31:22 +0000
@@ -22,6 +22,7 @@
 #include "mir/shell/surface_factory.h"
 #include "mir/shell/focus_sequence.h"
 #include "mir/shell/focus_setter.h"
+#include "mir/shell/nullshell.h"
 
 #include <memory>
 #include <cassert>
@@ -51,7 +52,8 @@
 
 std::shared_ptr<msh::Session> msh::SessionManager::open_session(std::string const& name)
 {
-    auto new_session = std::make_shared<msh::ApplicationSession>(surface_factory, name);
+    auto new_session = std::make_shared<msh::ApplicationSession>(
+                                      surface_factory, name, current_shell());
 
     app_container->insert_session(new_session);
 
@@ -134,3 +136,11 @@
         set_focus_to(match->second);
     }
 }
+
+std::shared_ptr<mir::Shell> msh::SessionManager::current_shell() const
+{
+    if (!shell)
+        shell.reset(new mir::shell::NullShell);
+
+    return shell;
+}

=== modified file 'src/surfaces/proxy_surface.cpp'
--- src/surfaces/proxy_surface.cpp	2013-03-06 12:25:34 +0000
+++ src/surfaces/proxy_surface.cpp	2013-03-07 08:31:22 +0000
@@ -109,6 +109,17 @@
     surface_stack->destroy_surface(surface);
 }
 
+int ms::BasicProxySurface::configure(MirSurfaceAttrib attrib, int value)
+{
+    if (auto const& s = surface.lock())
+    {
+        return s->configure(attrib, value);
+    }
+    else
+    {
+        BOOST_THROW_EXCEPTION(std::runtime_error("Invalid surface"));
+    }
+}
 
 ms::ProxySurface::ProxySurface(
         SurfaceStackModel* const surface_stack_,

=== modified file 'src/surfaces/proxy_surface.h'
--- src/surfaces/proxy_surface.h	2013-03-06 12:25:34 +0000
+++ src/surfaces/proxy_surface.h	2013-03-07 08:31:22 +0000
@@ -56,6 +56,8 @@
 
     std::shared_ptr<compositor::Buffer> client_buffer() const;
 
+    int configure(MirSurfaceAttrib attrib, int value);
+
 protected:
     void destroy_surface(SurfaceStackModel* const surface_stack) const;
 

=== modified file 'src/surfaces/surface.cpp'
--- src/surfaces/surface.cpp	2013-03-04 16:02:02 +0000
+++ src/surfaces/surface.cpp	2013-03-07 08:31:22 +0000
@@ -36,7 +36,8 @@
     surface_name(name),
     buffer_bundle(buffer_bundle),
     alpha_value(1.0f),
-    is_hidden(false)
+    is_hidden(false),
+    type_value(MIR_SURFACE_NORMAL)
 {
     // TODO(tvoss,kdub): Does a surface without a buffer_bundle make sense?
     assert(buffer_bundle);
@@ -128,3 +129,39 @@
 {
     return client_buffer_resource;
 }
+
+int ms::Surface::configure(MirSurfaceAttrib attrib, int value)
+{
+    int result = 0;
+
+    switch (attrib)
+    {
+    case MIR_SURFACE_TYPE:
+        set_type(static_cast<MirSurfaceType>(value));
+        result = type_value;
+        break;
+    default:
+        assert(false);
+        break;
+    }
+
+    return result;
+}
+
+MirSurfaceType ms::Surface::type() const
+{
+    return type_value;
+}
+
+bool ms::Surface::set_type(MirSurfaceType t)
+{
+    bool valid = false;
+
+    if (t >= 0 && t < MirSurfaceType_ARRAYSIZE)
+    {
+        type_value = t;
+        valid = true;
+    }
+
+    return valid;
+}

=== modified file 'tests/acceptance-tests/test_client_library.cpp'
--- tests/acceptance-tests/test_client_library.cpp	2013-02-26 16:48:46 +0000
+++ tests/acceptance-tests/test_client_library.cpp	2013-03-07 08:31:22 +0000
@@ -168,6 +168,72 @@
     launch_client_process(client_config);
 }
 
+TEST_F(DefaultDisplayServerTestFixture, surface_types)
+{
+    struct ClientConfig : ClientConfigCommon
+    {
+        void exec()
+        {
+
+            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this));
+
+            ASSERT_TRUE(connection != NULL);
+            EXPECT_TRUE(mir_connection_is_valid(connection));
+            EXPECT_STREQ(mir_connection_get_error_message(connection), "");
+
+            MirSurfaceParameters const request_params =
+            {
+                __PRETTY_FUNCTION__,
+                640, 480,
+                mir_pixel_format_abgr_8888,
+                mir_buffer_usage_hardware
+            };
+
+            mir_wait_for(mir_surface_create(connection, &request_params, create_surface_callback, this));
+
+            ASSERT_TRUE(surface != NULL);
+            EXPECT_TRUE(mir_surface_is_valid(surface));
+            EXPECT_STREQ(mir_surface_get_error_message(surface), "");
+
+            EXPECT_EQ(mir_surface_get_type(surface), MIR_SURFACE_NORMAL);
+
+            mir_wait_for(mir_surface_set_type(surface, MIR_SURFACE_FREESTYLE));
+            EXPECT_EQ(mir_surface_get_type(surface), MIR_SURFACE_FREESTYLE);
+
+            mir_wait_for(mir_surface_set_type(surface,
+                                            static_cast<MirSurfaceType>(999)));
+            EXPECT_EQ(mir_surface_get_type(surface), MIR_SURFACE_FREESTYLE);
+
+            mir_wait_for(mir_surface_set_type(surface, MIR_SURFACE_DIALOG));
+            EXPECT_EQ(mir_surface_get_type(surface), MIR_SURFACE_DIALOG);
+
+            mir_wait_for(mir_surface_set_type(surface,
+                                            static_cast<MirSurfaceType>(888)));
+            EXPECT_EQ(mir_surface_get_type(surface), MIR_SURFACE_DIALOG);
+
+            // Stress-test synchronization logic with some flooding
+            for (int i = 0; i < 1000; i++)
+            {
+                mir_surface_set_type(surface, MIR_SURFACE_NORMAL);
+                mir_surface_set_type(surface, MIR_SURFACE_UTILITY);
+                mir_surface_set_type(surface, MIR_SURFACE_DIALOG);
+                mir_surface_set_type(surface, MIR_SURFACE_OVERLAY);
+                mir_surface_set_type(surface, MIR_SURFACE_FREESTYLE);
+                mir_wait_for(mir_surface_set_type(surface,
+                                                  MIR_SURFACE_POPOVER));
+                ASSERT_EQ(mir_surface_get_type(surface), MIR_SURFACE_POPOVER);
+            }
+
+            mir_wait_for(mir_surface_release(surface, release_surface_callback,
+                                             this));
+
+            mir_connection_release(connection);
+        }
+    } client_config;
+
+    launch_client_process(client_config);
+}
+
 TEST_F(DefaultDisplayServerTestFixture, client_library_creates_multiple_surfaces)
 {
     int const n_surfaces = 13;

=== modified file 'tests/acceptance-tests/test_focus_management_api.cpp'
--- tests/acceptance-tests/test_focus_management_api.cpp	2013-03-06 12:25:34 +0000
+++ tests/acceptance-tests/test_focus_management_api.cpp	2013-03-07 08:31:22 +0000
@@ -46,6 +46,8 @@
 
 namespace mir
 {
+class Shell;
+
 namespace
 {
 struct ClientConfigCommon : TestingClientConfiguration
@@ -144,6 +146,8 @@
 
     MOCK_METHOD0(shutdown, void ());
 
+    MOCK_CONST_METHOD0(current_shell, std::shared_ptr<mir::Shell>());
+
 private:
     std::shared_ptr<shell::SessionStore> const impl;
 };

=== modified file 'tests/behavior-tests/session_management_context.cpp'
--- tests/behavior-tests/session_management_context.cpp	2013-03-06 12:54:33 +0000
+++ tests/behavior-tests/session_management_context.cpp	2013-03-07 08:31:22 +0000
@@ -75,6 +75,11 @@
     {
         return std::shared_ptr<mc::Buffer>();
     }
+
+    virtual int configure(MirSurfaceAttrib, int)
+    {
+        return 0;
+    }
 };
 
 struct SizedDummySurface : public DummySurface

=== modified file 'tests/integration-tests/cucumber/test_session_management_context.cpp'
--- tests/integration-tests/cucumber/test_session_management_context.cpp	2013-03-06 16:48:33 +0000
+++ tests/integration-tests/cucumber/test_session_management_context.cpp	2013-03-07 08:31:22 +0000
@@ -59,6 +59,8 @@
     MOCK_METHOD1(focus_session_with_lightdm_id, void(int));
 
     MOCK_METHOD0(shutdown, void());
+
+    MOCK_CONST_METHOD0(current_shell, std::shared_ptr<mir::Shell>());
 };
 
 struct MockSession : public msh::Session
@@ -72,6 +74,8 @@
     
     MOCK_METHOD0(hide, void());
     MOCK_METHOD0(show, void());
+
+    MOCK_METHOD3(configure_surface, int(msh::SurfaceId, MirSurfaceAttrib, int));
 };
 
 struct MockSurface : public msh::Surface
@@ -85,6 +89,8 @@
     MOCK_CONST_METHOD0(size, mir::geometry::Size ());
     MOCK_CONST_METHOD0(pixel_format, mir::geometry::PixelFormat ());
     MOCK_CONST_METHOD0(client_buffer, std::shared_ptr<mc::Buffer> ());
+
+    MOCK_METHOD2(configure, int(MirSurfaceAttrib, int));
 };
 
 MATCHER_P(NamedWindowWithNoGeometry, name, "")

=== modified file 'tests/unit-tests/CMakeLists.txt'
--- tests/unit-tests/CMakeLists.txt	2013-03-04 15:54:45 +0000
+++ tests/unit-tests/CMakeLists.txt	2013-03-07 08:31:22 +0000
@@ -17,6 +17,7 @@
 endif()
 add_subdirectory(surfaces/)
 add_subdirectory(draw/)
+add_subdirectory(shell/)
 
 add_executable(unit-tests ${UNIT_TEST_SOURCES})
 

=== modified file 'tests/unit-tests/client/CMakeLists.txt'
--- tests/unit-tests/client/CMakeLists.txt	2013-03-04 15:54:45 +0000
+++ tests/unit-tests/client/CMakeLists.txt	2013-03-07 08:31:22 +0000
@@ -3,6 +3,7 @@
   ${CMAKE_CURRENT_SOURCE_DIR}/test_client_platform.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_client_mir_surface.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_mir_connection.cpp
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_wait_handle.cpp
 )
 
 if(MIR_PLATFORM STREQUAL "android")

=== modified file 'tests/unit-tests/client/test_client_mir_surface.cpp'
--- tests/unit-tests/client/test_client_mir_surface.cpp	2013-02-26 16:48:46 +0000
+++ tests/unit-tests/client/test_client_mir_surface.cpp	2013-03-07 08:31:22 +0000
@@ -376,3 +376,22 @@
     EXPECT_EQ(pf, geom::PixelFormat::abgr_8888);
 }
 
+TEST_F(MirClientSurfaceTest, default_surface_type)
+{
+    using namespace testing;
+    using namespace mir::protobuf;
+
+    EXPECT_CALL(*mock_depository, deposit_package_rv(_,_,_,_))
+        .Times(1);
+
+    auto surface = std::make_shared<MirSurface> (connection.get(),
+                                                 *client_comm_channel,
+                                                 logger,
+                                                 mock_depository,
+                                                 params,
+                                                 &empty_callback,
+                                                 (void*) NULL);
+    surface->get_create_wait_handle()->wait_for_result();
+
+    EXPECT_EQ(surface->attrib(MIR_SURFACE_TYPE), MIR_SURFACE_NORMAL);
+}

=== added file 'tests/unit-tests/client/test_wait_handle.cpp'
--- tests/unit-tests/client/test_wait_handle.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/client/test_wait_handle.cpp	2013-03-07 08:31:22 +0000
@@ -0,0 +1,114 @@
+/*
+ * Copyright © 2013 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: Daniel van Vugt <[e-mail]>
+ */
+
+#include <thread>
+#include <gtest/gtest.h>
+#include "src/client/mir_wait_handle.h"
+
+using namespace mir_toolkit;
+
+TEST(WaitHandle, symmetric_synchronous)
+{
+    MirWaitHandle w;
+
+    for (int i = 0; i < 100; i++)
+    {
+        w.result_received();
+        w.wait_for_result();
+    }
+
+    EXPECT_TRUE(true);   // Failure would be hanging in the above loop
+}
+
+TEST(WaitHandle, asymmetric_synchronous)
+{
+    MirWaitHandle w;
+
+    for (int i = 1; i <= 100; i++)
+    {
+        for (int j = 1; j <= i; j++)
+            w.result_received();
+
+        w.wait_for_result();
+    }
+
+    EXPECT_TRUE(true);   // Failure would be hanging in the above loop
+}
+
+namespace
+{
+    int symmetric_result = 0;
+
+    void symmetric_thread(MirWaitHandle *in, MirWaitHandle *out, int max)
+    {
+        for (int i = 1; i <= max; i++)
+        {
+            in->wait_for_result();
+            symmetric_result = i;
+            out->result_received();
+        }
+    }
+}
+
+TEST(WaitHandle, symmetric_asynchronous)
+{
+    const int max = 100;
+    MirWaitHandle in, out;
+    std::thread t(symmetric_thread, &in, &out, max);
+
+    for (int i = 1; i <= max; i++)
+    {
+        in.result_received();
+        out.wait_for_result();
+        ASSERT_EQ(symmetric_result, i);
+    }
+    t.join();
+}
+
+namespace
+{
+    int asymmetric_result = 0;
+
+    void asymmetric_thread(MirWaitHandle *in, MirWaitHandle *out, int max)
+    {
+        for (int i = 1; i <= max; i++)
+        {
+            in->wait_for_result();
+            asymmetric_result = i;
+            for (int j = 1; j <= i; j++)
+                out->result_received();
+        }
+    }
+}
+
+TEST(WaitHandle, asymmetric_asynchronous)
+{
+    const int max = 100;
+    MirWaitHandle in, out;
+    std::thread t(asymmetric_thread, &in, &out, max);
+
+    for (int i = 1; i <= max; i++)
+    {
+        in.result_received();
+        for (int j = 1; j <= i; j++)
+            out.expect_result();
+        out.wait_for_result();
+        ASSERT_EQ(asymmetric_result, i);
+    }
+    t.join();
+}

=== modified file 'tests/unit-tests/frontend/test_application_mediator.cpp'
--- tests/unit-tests/frontend/test_application_mediator.cpp	2013-03-06 12:54:33 +0000
+++ tests/unit-tests/frontend/test_application_mediator.cpp	2013-03-07 08:31:22 +0000
@@ -112,6 +112,12 @@
     void tag_session_with_lightdm_id(std::shared_ptr<msh::Session> const&, int) {}
     void focus_session_with_lightdm_id(int) {}
 
+    std::shared_ptr<mir::Shell> current_shell() const
+    {
+        std::shared_ptr<mir::Shell> noshell;
+        return noshell;
+    }
+
     std::shared_ptr<msh::SurfaceFactory> factory;
 };
 

=== modified file 'tests/unit-tests/frontend/test_application_mediator_gbm.cpp'
--- tests/unit-tests/frontend/test_application_mediator_gbm.cpp	2013-03-06 12:54:33 +0000
+++ tests/unit-tests/frontend/test_application_mediator_gbm.cpp	2013-03-07 08:31:22 +0000
@@ -83,6 +83,12 @@
     void tag_session_with_lightdm_id(std::shared_ptr<msh::Session> const&, int) {}
     void focus_session_with_lightdm_id(int) {}
 
+    std::shared_ptr<mir::Shell> current_shell() const
+    {
+        std::shared_ptr<mir::Shell> noshell;
+        return noshell;
+    }
+
     std::shared_ptr<msh::SurfaceFactory> factory;
 };
 

=== modified file 'tests/unit-tests/sessions/test_application_session.cpp'
--- tests/unit-tests/sessions/test_application_session.cpp	2013-03-06 12:54:33 +0000
+++ tests/unit-tests/sessions/test_application_session.cpp	2013-03-07 08:31:22 +0000
@@ -64,6 +64,8 @@
     MOCK_CONST_METHOD0(pixel_format, mir::geometry::PixelFormat ());
     MOCK_CONST_METHOD0(client_buffer, std::shared_ptr<mc::Buffer> ());
 
+    MOCK_METHOD2(configure, int(MirSurfaceAttrib, int));
+
 private:
     ms::BasicProxySurface impl;
 };

=== added directory 'tests/unit-tests/shell'
=== added file 'tests/unit-tests/shell/CMakeLists.txt'
--- tests/unit-tests/shell/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/shell/CMakeLists.txt	2013-03-07 08:31:22 +0000
@@ -0,0 +1,5 @@
+list (APPEND UNIT_TEST_SOURCES
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_nullshell.cpp
+)
+
+set( UNIT_TEST_SOURCES ${UNIT_TEST_SOURCES} PARENT_SCOPE)

=== added file 'tests/unit-tests/shell/test_nullshell.cpp'
--- tests/unit-tests/shell/test_nullshell.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/shell/test_nullshell.cpp	2013-03-07 08:31:22 +0000
@@ -0,0 +1,53 @@
+/*
+ * Copyright © 2013 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: Daniel van Vugt <[e-mail]>
+ */
+
+#include "mir/shell/nullshell.h"
+#include <gtest/gtest.h>
+
+using namespace mir::shell;
+
+TEST(NullShell, surface_attribs)
+{
+    NullShell n;
+
+    EXPECT_TRUE(n.supports(MIR_SURFACE_TYPE));
+    EXPECT_FALSE(n.supports(static_cast<MirSurfaceAttrib>(-1)));
+    EXPECT_FALSE(n.supports(static_cast<MirSurfaceAttrib>(99)));
+    EXPECT_FALSE(n.supports(MirSurfaceAttrib_ARRAYSIZE));
+}
+
+TEST(NullShell, surface_types)
+{
+    NullShell n;
+
+    EXPECT_TRUE(n.supports(MIR_SURFACE_TYPE, MIR_SURFACE_NORMAL));
+    EXPECT_TRUE(n.supports(MIR_SURFACE_TYPE, MIR_SURFACE_UTILITY));
+    EXPECT_TRUE(n.supports(MIR_SURFACE_TYPE, MIR_SURFACE_DIALOG));
+    EXPECT_TRUE(n.supports(MIR_SURFACE_TYPE, MIR_SURFACE_OVERLAY));
+    EXPECT_TRUE(n.supports(MIR_SURFACE_TYPE, MIR_SURFACE_FREESTYLE));
+    EXPECT_TRUE(n.supports(MIR_SURFACE_TYPE, MIR_SURFACE_POPOVER));
+    EXPECT_FALSE(n.supports(MIR_SURFACE_TYPE, MirSurfaceType_ARRAYSIZE));
+    EXPECT_FALSE(n.supports(MIR_SURFACE_TYPE, -1));
+    EXPECT_FALSE(n.supports(MIR_SURFACE_TYPE, 123));
+
+    EXPECT_FALSE(n.supports(static_cast<MirSurfaceAttrib>(-1),
+                            MIR_SURFACE_NORMAL));
+    EXPECT_FALSE(n.supports(static_cast<MirSurfaceAttrib>(99),
+                            MIR_SURFACE_NORMAL));
+    EXPECT_FALSE(n.supports(MirSurfaceAttrib_ARRAYSIZE, MIR_SURFACE_NORMAL));
+}

=== modified file 'tests/unit-tests/surfaces/test_surface.cpp'
--- tests/unit-tests/surfaces/test_surface.cpp	2013-03-06 12:54:33 +0000
+++ tests/unit-tests/surfaces/test_surface.cpp	2013-03-07 08:31:22 +0000
@@ -306,3 +306,21 @@
 
     EXPECT_EQ(alpha, ret_alpha);
 }
+
+TEST_F(SurfaceCreation, types)
+{
+    using namespace testing;
+
+    ms::Surface surf{surface_name, mock_buffer_bundle};
+
+    EXPECT_EQ(MIR_SURFACE_NORMAL, surf.type());
+
+    EXPECT_TRUE(surf.set_type(MIR_SURFACE_UTILITY));
+    EXPECT_EQ(surf.type(), MIR_SURFACE_UTILITY);
+
+    EXPECT_FALSE(surf.set_type(static_cast<MirSurfaceType>(999)));
+    EXPECT_EQ(surf.type(), MIR_SURFACE_UTILITY);
+
+    EXPECT_TRUE(surf.set_type(MIR_SURFACE_DIALOG));
+    EXPECT_EQ(surf.type(), MIR_SURFACE_DIALOG);
+}