Merge lp://staging/~dandrader/unity8/allowClientResize into lp://staging/unity8

FAILED: Continuous integration, rev:2840
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3299/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/4342/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4370
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4204/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4204/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4204/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4204/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4204/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4204/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3299/rebuild

The code looks good, except why use this lambda?

connect(surface, &unityapi::MirSurfaceInterface::allowClientResizeChanged, this, [this]() {...}

Can't you just connect to the &Window::setAllowClientResize slot?

On 07/03/2017 11:27, Lukáš Tinkl wrote:
> Review: Needs Information
>
> The code looks good, except why use this lambda?
>
> connect(surface, &unityapi::MirSurfaceInterface::allowClientResizeChanged, this, [this]() {...}
>
> Can't you just connect to the &Window::setAllowClientResize slot?

No, they are different.

Window::setAllowClientResize is called from unity8. It sets the Window
property and then the Window forwards that to its surface, if any. So
the direction is Window -> Surface

That lambda comes in response to a change in the surface property.
Window then updates its value appropriately. So the direction is Surface
-> Window

Thx for the explanation

+{
+ if (value != m_allowClientResize) {
+ DEBUG_MSG << "("<<value<<")";
+ m_allowClientResize = value;
+ Q_EMIT allowClientResizeChanged(m_allowClientResize);
+ if (m_surface) {
+ m_surface->setAllowClientResize(value);
+ }
+ }
Why emit the signal before doing the operation? Typically a signal is emitted after the new state has been applied.

Rest looks fine

On 08/03/2017 09:20, Gerry Boland wrote:
> Review: Needs Information
>
> +{
> + if (value != m_allowClientResize) {
> + DEBUG_MSG << "("<<value<<")";
> + m_allowClientResize = value;
> + Q_EMIT allowClientResizeChanged(m_allowClientResize);
> + if (m_surface) {
> + m_surface->setAllowClientResize(value);
> + }
> + }
> Why emit the signal before doing the operation? Typically a signal is emitted after the new state has been applied.
>

Because m_allowClientResize is already changed, which is what the getter
uses. Anyway, moved the signal emission to the bottom.

FAILED: Continuous integration, rev:2841
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3310/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/4357/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4385
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4218/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4218/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4218/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4218/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4218/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4218/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3310/rebuild

You forgot to update the other mocks, it doesn't compile:

/<<BUILDDIR>>/unity8-8.15+16.04.20170314/tests/plugins/Unity/Launcher/launchermodeltest.cpp: In member function ‘void LauncherModelTest::testSurfaceCountUpdates()’:
/<<BUILDDIR>>/unity8-8.15+16.04.20170314/tests/plugins/Unity/Launcher/launchermodeltest.cpp:814:60: error: invalid new-expression of abstract class type ‘MockSurface’
         surfaces->append(new MockSurface("foobar", surfaces));
                                                            ^
/<<BUILDDIR>>/unity8-8.15+16.04.20170314/tests/plugins/Unity/Launcher/launchermodeltest.cpp:41:7: note: because the following virtual functions are pure within ‘MockSurface’:
 class MockSurface: public unity::shell::application::MirSurfaceInterface
       ^
In file included from /<<BUILDDIR>>/unity8-8.15+16.04.20170314/plugins/Unity/Launcher/launcheritem.h:26:0,
                 from /<<BUILDDIR>>/unity8-8.15+16.04.20170314/tests/plugins/Unity/Launcher/launchermodeltest.cpp:22:
/usr/include/unity/shell/application/MirSurfaceInterface.h:231:18: note: virtual bool unity::shell::application::MirSurfaceInterface::allowClientResize() const
     virtual bool allowClientResize() const = 0;
                  ^
/usr/include/unity/shell/application/MirSurfaceInterface.h:232:18: note: virtual void unity::shell::application::MirSurfaceInterface::setAllowClientResize(bool)
     virtual void setAllowClientResize(bool) = 0;
                  ^

FAILED: Continuous integration, rev:2841
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3378/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/4452/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4480/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3378/rebuild