Code review comment for lp:~vanvugt/mir/frontend-server

I think the problem is that frontend::Shell doesn't represent a clear abstraction. If it did, then we would probably have a good name for that abstraction.

Three of the four functions are actually to do with manipulating the data model. Vis: open_session(), close_session() and create_surface_for(). I agree that as Session is part of the data model it ought to understand creating a surface and the latter is misplaced.

That comes down to a poor interface design around scene, Session and Surface. It ought to be easy for a shell to decorate the data model required by frontend and add its own logic. The fact that we've made that hard for ourselves (and for unity8) is the underlying cause for the awkward handle_surface_created().

Note that this interface is a part of a larger mess: We've got an awkward coupling between creating surfaces and them being added to the scene. (Creating the surface implicitly adds it to the scene *before* the client has had a chance to paint a buffer.)

The interface should probably be something like:

class frontend::XXXXX
{
public:
    virtual std::shared_ptr<Session> create_session(...) = 0;
    virtual void activate(std::shared_ptr<Session> a_session) = 0;
    virtual void deactivate(std::shared_ptr<Session> a_session) = 0;
};

class frontend::Session
{
public:
    virtual std::shared_ptr<Surface> create_session(...) = 0;
    virtual void activate(std::shared_ptr<Surface> a_surface) = 0;
    virtual void deactivate(std::shared_ptr<Surface> a_surface) = 0;
    ...
};

We ought to provide utility classes for decorating implementations of these that can be used by shells to wrap the implementations provided by scene.

PS I still think that Scene is a better name than Server