Code review comment for lp:~dandrader/qtmir/multiSurfaceApp

> Please merge trunk, many conflicts have appeared.

Done.

> +void Session::appendSurface
> +void Session::reallyAppendSurface
> these names confuse me. Perhaps rename to something like:
> appendSurface -> registerSurface
> reallyAppendSurface -> appendSurface

Done.

> qDebug() << "Application::suspend - no surface to call stopFrameDropper()
> on!";
> I think we can move this to category logging

Done.

> + return const_cast<ObjectListModel<MirSurfaceInterface>*>(&m_surfaces);
> const_cast needed here? Can't the pointer be a pointer to a const object?

Right. I was just blindly following the signature of childSessions. Fixed.

> === modified file 'src/modules/Unity/Application/session.h'
> + QList<MirSurfaceInterface*> m_blankSurfaces;
> What use is this? I only see you append to and remove from this list, but
> never use it for anything.

Originally it was meant to keep track of the surfaces that were registered but not yet appended. But later it turned out that this wasn't needed after all. Removed.

> + Q_PROPERTY(MirSurfaceInterface* lastSurface READ lastSurface NOTIFY
> lastSurfaceChanged CONSTANT);
> Can you add a "REMOVEME" comment to this, this is not an API I want to keep.
> It also not very correct, as what if the surface pointed to is removed, but
> other surfaces exist - should it not be updated?

This will be removed once unity8 starts using Session.surfaces. Ie, once it actually starts supporting multiple surfaces per application. It was just way too awkward to have untiy8 fetching the last surface from Session.surfaces in QML. Hence this property.

Added the FIXME comment to the property declaration.

lastSurface is being updated correctly as the surfaces model changes.