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

[Daniel d'Andrada]

Implement MirSurface::allowClientResize

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

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

* 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

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

FAILED: Continuous integration, rev:609
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/558/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/4341/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4369
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4203/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4203/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4203/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4203/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4203/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4203/console

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

review: Needs Fixing (continuous-integration)

[Gerry Boland (gerboland)]

- CanonicalWindowManagerPolicy::handle_modify_window(windowInfo, modifications);
+ tools.modify_window(window_info, modifications);

I'd prefer you kept the reference to the CanonicalWindowManagerPolicy, for consistency. We're trying to use as much Mir code as possible (even though it is trivial)

+ auto extraWindowInfo = getExtraInfo(windowInfo);
const auto & <- to save a copy?

review: Needs Fixing

[Daniel d'Andrada (dandrader)]

On 08/03/2017 09:17, Gerry Boland wrote:
> + auto extraWindowInfo = getExtraInfo(windowInfo);
> const auto & <- to save a copy?

Are you sure? getExtraInfo() returns a std::shared_ptr<>. Won't move
semantics take care of that?

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

FAILED: Continuous integration, rev:610
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/561/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/4358/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4386
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4219/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4219/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4219/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4219/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4219/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4219/console

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

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

> On 08/03/2017 09:17, Gerry Boland wrote:
> > + auto extraWindowInfo = getExtraInfo(windowInfo);
> > const auto & <- to save a copy?
>
> Are you sure? getExtraInfo() returns a std::shared_ptr<>. Won't move
> semantics take care of that?

The times that 'const auto&' gains over 'const auto' are mostly those when the function returns a reference.

For any function that returns a type (i.e. not a reference) it is very likely that RVO "takes care of it" and invoking move semantics would actually be less efficient.

[Gerry Boland (gerboland)]

Ok, compilers can do magic

review: Approve

[Lukáš Tinkl (lukas-kde)]

- pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application=26)
+ pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application=27)

You also need to add the above to the toplevel CMakeLists.txt so that it compiles

review: Needs Fixing

[Daniel d'Andrada (dandrader)]

> - pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application=26)
> + pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application=27)
>
> You also need to add the above to the toplevel CMakeLists.txt so that it
> compiles

Done.

[lp:~dandrader/qtmir/allowClientResize]

611. By Daniel d'Andrada on 2017-03-13

bump unity-shell-application required version

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

FAILED: Continuous integration, rev:611
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/571/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/4424/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4452
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4285/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4285/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4285/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4285/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4285/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4285/console

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

review: Needs Fixing (continuous-integration)

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

PASSED: Continuous integration, rev:611
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/572/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4428
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4456
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4289
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4289/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4289
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4289/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4289
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4289/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4289
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4289/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4289
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4289/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4289
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4289/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Lukáš Tinkl (lukas-kde)]

review: Abstain

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2017-02-08 19:53:50 +0000
+++ CMakeLists.txt	2017-03-13 19:42:34 +0000
@@ -90,7 +90,7 @@
 pkg_check_modules(GSETTINGS_QT REQUIRED gsettings-qt)
 pkg_check_modules(QTDBUSTEST libqtdbustest-1 REQUIRED)
 pkg_check_modules(QTDBUSMOCK libqtdbusmock-1 REQUIRED)
-pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application=26)
+pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application=27)
 pkg_check_modules(CGMANAGER libcgmanager REQUIRED)
 pkg_check_modules(CONTENT_HUB libcontent-hub>=0.2 REQUIRED)
 

=== modified file 'debian/control'
--- debian/control	2017-03-07 23:41:44 +0000
+++ debian/control	2017-03-13 19:42:34 +0000
@@ -25,7 +25,7 @@
                libubuntu-app-launch3-dev,
                libubuntu-application-api-dev (>= 2.1.0),
                libudev-dev,
-               libunity-api-dev (>= 8.2),
+               libunity-api-dev (>= 8.5),
                liburl-dispatcher1-dev,
                libxkbcommon-dev,
                libxrender-dev,
@@ -102,7 +102,7 @@
 Conflicts: libqtmir,
            libunity-mir1,
 Provides: unity-application-impl,
-          unity-application-impl-26,
+          unity-application-impl-27,
 Description: Qt plugin for Unity specific Mir APIs
  QtMir provides Qt/QML bindings for Mir features that are exposed through the
  qtmir-desktop or qtmir-android QPA plugin such as Application management

=== modified file 'debian/gles-patches/convert-to-gles.patch'
--- debian/gles-patches/convert-to-gles.patch	2017-02-15 13:20:44 +0000
+++ debian/gles-patches/convert-to-gles.patch	2017-03-13 19:42:34 +0000
@@ -84,7 +84,7 @@
 -Conflicts: libqtmir,
 -           libunity-mir1,
 -Provides: unity-application-impl,
--          unity-application-impl-26,
+-          unity-application-impl-27,
 -Description: Qt plugin for Unity specific Mir APIs
 - QtMir provides Qt/QML bindings for Mir features that are exposed through the
 - qtmir-desktop or qtmir-android QPA plugin such as Application management

=== modified file 'src/common/windowmodelnotifier.h'
--- src/common/windowmodelnotifier.h	2017-01-18 21:24:15 +0000
+++ src/common/windowmodelnotifier.h	2017-03-13 19:42:34 +0000
@@ -20,6 +20,7 @@
 #include <QObject>
 #include <QPoint>
 #include <QSize>
+#include <QMutex>
 
 #include <miral/window_info.h>
 
@@ -58,6 +59,10 @@
     // Mir::MaximizedBottomLeftState:
     // Mir::MaximizedBottomRightState:
     Mir::State state{Mir::UnknownState};
+
+    bool allowClientResize{true};
+
+    QMutex mutex;
 };
 
 std::shared_ptr<ExtraWindowInfo> getExtraInfo(const miral::WindowInfo &windowInfo);

=== modified file 'src/modules/Unity/Application/mirsurface.cpp'
--- src/modules/Unity/Application/mirsurface.cpp	2017-03-07 23:42:05 +0000
+++ src/modules/Unity/Application/mirsurface.cpp	2017-03-13 19:42:34 +0000
@@ -767,6 +767,7 @@
 
 void MirSurface::emitSizeChanged()
 {
+    qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << (void*)this << "," << appId() << "]::sizeChanged(" << m_size << ")";
     Q_EMIT sizeChanged(m_size);
 }
 
@@ -937,6 +938,20 @@
     return m_surface->confine_pointer_state() == mir_pointer_confined_to_window;
 }
 
+bool MirSurface::allowClientResize() const
+{
+    return m_extraInfo->allowClientResize;
+}
+
+void MirSurface::setAllowClientResize(bool value)
+{
+    if (m_extraInfo->allowClientResize != value) {
+        QMutexLocker locker(&m_extraInfo->mutex);
+        m_extraInfo->allowClientResize = value;
+        Q_EMIT allowClientResizeChanged(value);
+    }
+}
+
 void MirSurface::activate()
 {
     INFO_MSG << "()";

=== modified file 'src/modules/Unity/Application/mirsurface.h'
--- src/modules/Unity/Application/mirsurface.h	2017-02-02 09:17:48 +0000
+++ src/modules/Unity/Application/mirsurface.h	2017-03-13 19:42:34 +0000
@@ -96,6 +96,9 @@
 
     bool confinesMousePointer() const override;
 
+    bool allowClientResize() const override;
+    void setAllowClientResize(bool) override;
+
     Q_INVOKABLE void activate() override;
 
     unity::shell::application::MirSurfaceInterface *parentSurface() const override;

=== modified file 'src/platforms/mirserver/windowmanagementpolicy.cpp'
--- src/platforms/mirserver/windowmanagementpolicy.cpp	2017-02-15 13:21:50 +0000
+++ src/platforms/mirserver/windowmanagementpolicy.cpp	2017-03-13 19:42:34 +0000
@@ -90,9 +90,18 @@
 
 void WindowManagementPolicy::handle_modify_window(
     miral::WindowInfo &windowInfo,
-    const miral::WindowSpecification &modifications)
+    const miral::WindowSpecification &modificationsClient)
 {
-    // TODO this applies the default policy. Qt needs to process the request instead
+    miral::WindowSpecification modifications(modificationsClient);
+
+    if (modifications.size().is_set()) {
+        auto extraWindowInfo = getExtraInfo(windowInfo);
+        QMutexLocker locker(&extraWindowInfo->mutex);
+        if (!extraWindowInfo->allowClientResize) {
+            modifications.size().consume();
+        }
+    }
+
     CanonicalWindowManagerPolicy::handle_modify_window(windowInfo, modifications);
 
     // TODO Once Qt processes the request we probably don't want to notify from here

=== modified file 'tests/framework/fake_mirsurface.h'
--- tests/framework/fake_mirsurface.h	2017-02-02 09:17:48 +0000
+++ tests/framework/fake_mirsurface.h	2017-03-13 19:42:34 +0000
@@ -79,6 +79,9 @@
     QRect inputBounds() const override { return QRect(0,0,10,10); }
     bool confinesMousePointer() const override { return false; }
 
+    bool allowClientResize() const override { return true; }
+    void setAllowClientResize(bool) override {}
+
     unity::shell::application::MirSurfaceInterface *parentSurface() const override { return nullptr; }
     unity::shell::application::MirSurfaceListInterface *childSurfaceList() const override { return nullptr; }