Code review comment for lp:~nick-dedekind/qtubuntu/shell_chrome

+++ src/ubuntumirclient/input.cpp
at the end of the anonymous namespace, could you please add "// namespace"
I always find
+}
+
+}
hard to parse because you're missing the hint the indent usually gives - since we don't indent namespace contents.

+++ src/ubuntumirclient/window.cpp
+const Qt::WindowType WindowHidesShellDecorations = (Qt::WindowType)0x00800000;
being a pedant, but if mir chose the name "shell chrome" I guess we should use its vocabulary too.

+ void updateSurface();
While it was private, I didn't mind, but now it is public, I do. Function name is terrible considering what it does. Please either rename, or refactor a little to avoid making it public member of UbuntuSurface.

There is a general problem in this file, it's unclear what UbuntuSurface is, and how it relates to UbuntuWindow, and what responsibilities it has. My understanding was that UbuntuSurface should wrap the mir surface api in a Qt way, but as you saw it also is saving state, and referring to state in UbuntuWindow/QWindow when working. So the separation of responsibilities here is totally unclear.

So what you've done is natural, moving some states out of UbuntuSurface into UbuntuWindow. What is annoying is that UbuntuSurface imposes some sanity checks, referring to those states. I suspect we'll try to remove those, and trust that Mir will impose correct behaviour, instead of doing that here.

But
+ MirShellChrome mShellChrome;
does we need to save this state? Can we not just query the value from the mirclient api?

+ const QRect& exposeRect = mWindowVisible ? QRect(QPoint(), geometry().size()) : QRect();
& beside the variable name plz.