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

[Daniel d'Andrada]

Don't let clients resize their surfaces while in staged (phone/tablet) mode

Prereq-archive: ppa:ci-train-ppa-service/2555

That doesn't fix the bug for gtk+ apps though. I think gtk-mir needs some work.

* Are there any related MPs required for this MP to build/function as expected? Please list.
https://code.launchpad.net/~dandrader/qtmir/allowClientResize/+merge/319209
https://code.launchpad.net/~dandrader/unity-api/surfaceAllowClientResize/+merge/319730

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
Not applicable

* If you changed the UI, has there been a design review?
Not applicable

[Unity8 CI Bot (unity8-ci-bot)]

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

review: Needs Fixing (continuous-integration)

[Lukáš Tinkl (lukas-kde)]

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?

review: Needs Information

[Daniel d'Andrada (dandrader)]

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

[Lukáš Tinkl (lukas-kde)]

Thx for the explanation

[Gerry Boland (gerboland)]

+{
+ 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

review: Needs Information

[Daniel d'Andrada (dandrader)]

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.

[Unity8 CI Bot (unity8-ci-bot)]

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

review: Needs Fixing (continuous-integration)

[Gerry Boland (gerboland)]

review: Approve

[Lukáš Tinkl (lukas-kde)]

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;
                  ^

review: Needs Fixing

[Gerry Boland (gerboland)]

review: Approve

[Gerry Boland (gerboland)]

review: Approve

[Unity8 CI Bot (unity8-ci-bot)]

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

review: Needs Fixing (continuous-integration)

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:2869
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3463/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4566
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2751
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2751
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4594
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4424
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4424/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4424
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4424/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4424
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4424/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4424
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4424/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4424
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4424/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4424
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4424/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

[Unity8 CI Bot (unity8-ci-bot)]

PASSED: Continuous integration, rev:2869
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3465/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4574
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2757
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2757
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4602
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4429
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4429/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4429
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4429/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4429
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4429/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4429
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4429/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4429
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4429/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4429
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4429/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

=== modified file 'debian/control'
--- debian/control	2017-03-16 12:52:57 +0000
+++ debian/control	2017-03-16 12:52:59 +0000
@@ -162,7 +162,7 @@
          qttranslations5-l10n,
          ubuntu-thumbnailer-impl-0,
          ubuntu-wallpapers,
-         unity-application-impl-26,
+         unity-application-impl-27,
          unity-notifications-impl-3,
          unity-plugin-scopes | unity-scopes-impl,
          unity-scopes-impl-12,
@@ -212,7 +212,7 @@
          ${misc:Depends},
          ${shlibs:Depends},
 Provides: unity-application-impl,
-          unity-application-impl-26,
+          unity-application-impl-27,
           unity8-fake-env,
 Replaces: unity8-autopilot (<< 8.02+15.04.20150422-0ubuntu1),
           unity8-fake-env,

=== modified file 'plugins/WindowManager/Window.cpp'
--- plugins/WindowManager/Window.cpp	2016-12-06 14:41:54 +0000
+++ plugins/WindowManager/Window.cpp	2017-03-16 12:52:59 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016 Canonical, Ltd.
+ * Copyright (C) 2016-2017 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -67,6 +67,23 @@
     }
 }
 
+bool Window::allowClientResize() const
+{
+    return m_allowClientResize;
+}
+
+void Window::setAllowClientResize(bool value)
+{
+    if (value != m_allowClientResize) {
+        DEBUG_MSG << "("<<value<<")";
+        m_allowClientResize = value;
+        if (m_surface) {
+            m_surface->setAllowClientResize(value);
+        }
+        Q_EMIT allowClientResizeChanged(m_allowClientResize);
+    }
+}
+
 Mir::State Window::state() const
 {
     return m_state;
@@ -154,6 +171,13 @@
             updateFocused();
         });
 
+        connect(surface, &unityapi::MirSurfaceInterface::allowClientResizeChanged, this, [this]() {
+            if (m_surface->allowClientResize() != m_allowClientResize) {
+                m_allowClientResize = m_surface->allowClientResize();
+                Q_EMIT allowClientResizeChanged(m_allowClientResize);
+            }
+        });
+
         // bring it up to speed
         if (m_positionRequested) {
             m_surface->setRequestedPosition(m_requestedPosition);
@@ -161,6 +185,7 @@
         if (m_stateRequested && m_surface->state() == Mir::RestoredState) {
             m_surface->requestState(m_state);
         }
+        m_surface->setAllowClientResize(m_allowClientResize);
 
         // and sync with surface
         updatePosition();

=== modified file 'plugins/WindowManager/Window.h'
--- plugins/WindowManager/Window.h	2017-01-26 11:10:01 +0000
+++ plugins/WindowManager/Window.h	2017-03-16 12:52:59 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016 Canonical, Ltd.
+ * Copyright (C) 2016-2017 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -89,6 +89,13 @@
      */
     Q_PROPERTY(unity::shell::application::MirSurfaceInterface* surface READ surface NOTIFY surfaceChanged)
 
+    /**
+     * @brief Whether to comply to resize requests coming from the client side
+     *
+     * It's true by default
+     */
+    Q_PROPERTY(bool allowClientResize READ allowClientResize WRITE setAllowClientResize NOTIFY allowClientResizeChanged)
+
 public:
     Window(int id, QObject *parent = nullptr);
     virtual ~Window();
@@ -104,6 +111,9 @@
     void setSurface(unity::shell::application::MirSurfaceInterface *surface);
     void setFocused(bool value);
 
+    bool allowClientResize() const;
+    void setAllowClientResize(bool);
+
     QString toString() const;
 
 public Q_SLOTS:
@@ -133,6 +143,7 @@
     void focusedChanged(bool value);
     void confinesMousePointerChanged(bool value);
     void surfaceChanged(unity::shell::application::MirSurfaceInterface *surface);
+    void allowClientResizeChanged(bool value);
 
     /**
      * @brief Emitted when focus for this window is requested by an external party
@@ -152,6 +163,8 @@
     Mir::State m_state{Mir::RestoredState};
     bool m_stateRequested{false};
     unity::shell::application::MirSurfaceInterface *m_surface{nullptr};
+
+    bool m_allowClientResize{true};
 };
 
 QDebug operator<<(QDebug dbg, const Window *window);

=== modified file 'qml/Stage/Stage.qml'
--- qml/Stage/Stage.qml	2017-03-08 09:51:58 +0000
+++ qml/Stage/Stage.qml	2017-03-16 12:52:59 +0000
@@ -1302,6 +1302,10 @@
                             target: stageMaths
                             animateX: !focusAnimation.running && itemIndex !== spreadItem.highlightedIndex
                         }
+                        PropertyChanges {
+                            target: appDelegate.window
+                            allowClientResize: false
+                        }
                     },
                     State {
                         name: "stagedWithSideStage"; when: root.state == "stagedWithSideStage"
@@ -1327,6 +1331,10 @@
                             target: resizeArea
                             enabled: false
                         }
+                        PropertyChanges {
+                            target: appDelegate.window
+                            allowClientResize: false
+                        }
                     },
                     State {
                         name: "maximized"; when: appDelegate.maximized && !appDelegate.minimized

=== modified file 'tests/mocks/Unity/Application/MirSurface.h'
--- tests/mocks/Unity/Application/MirSurface.h	2017-03-16 12:52:57 +0000
+++ tests/mocks/Unity/Application/MirSurface.h	2017-03-16 12:52:59 +0000
@@ -93,6 +93,9 @@
 
     bool confinesMousePointer() const override { return false; }
 
+    bool allowClientResize() const override { return true; }
+    void setAllowClientResize(bool) override {}
+
     QPoint requestedPosition() const override { return m_requestedPosition; }
     void setRequestedPosition(const QPoint &) override;
 

=== modified file 'tests/plugins/Unity/Launcher/launchermodeltest.cpp'
--- tests/plugins/Unity/Launcher/launchermodeltest.cpp	2017-03-16 12:52:57 +0000
+++ tests/plugins/Unity/Launcher/launchermodeltest.cpp	2017-03-16 12:52:59 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2016 Canonical Ltd.
+ * Copyright 2013-2017 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 as published by
@@ -68,6 +68,8 @@
     bool focused() const override { return true; }
     QRect inputBounds() const override { return QRect(); }
     bool confinesMousePointer() const override { return false; }
+    bool allowClientResize() const override { return true; }
+    void setAllowClientResize(bool) override {}
     QPoint requestedPosition() const override { return QPoint(); }
     void setRequestedPosition(const QPoint &) override {}
     MirSurfaceInterface* parentSurface() const override { return nullptr; }