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

[Daniel d'Andrada]

Move prompt surfaces from MirSurface to Application and emit firstChanged signal

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

* 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

[lp:~dandrader/qtmir/detachPromptSurfaces]

489. By Daniel d'Andrada on 2016-05-17

Update unity-api versions

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

FAILED: Continuous integration, rev:488
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/223/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1650
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1603
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1603
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1596/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1596/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1596/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1596/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1596/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1596/console

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

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:489
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/224/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1653
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1605
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1605
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1598/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1598/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1598/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1598/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1598/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1598/console

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

review: Needs Fixing (continuous-integration)

[lp:~dandrader/qtmir/detachPromptSurfaces]

490. By Daniel d'Andrada on 2016-05-17

Fix code

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

FAILED: Continuous integration, rev:490
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/225/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1657
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1607
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1607
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1600/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1600/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1600/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1600/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1600/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1600/console

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

review: Needs Fixing (continuous-integration)

[Nick Dedekind (nick-dedekind)]

review: Needs Fixing

[Daniel d'Andrada (dandrader)]

Replied to inline comment

Unmerged revisions

=== modified file 'debian/control'
--- debian/control	2016-04-28 12:48:42 +0000
+++ debian/control	2016-05-17 19:58:11 +0000
@@ -22,7 +22,7 @@
                libubuntu-app-launch2-dev (>= 0.9),
                libubuntu-application-api-dev (>= 2.1.0),
                libudev-dev,
-               libunity-api-dev (>= 7.110),
+               libunity-api-dev (>= 7.112),
                liburl-dispatcher1-dev,
                libxkbcommon-dev,
                libxrender-dev,
@@ -93,7 +93,7 @@
 Conflicts: libqtmir,
            libunity-mir1,
 Provides: unity-application-impl,
-          unity-application-impl-15,
+          unity-application-impl-16,
 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	2016-04-06 18:12:31 +0000
+++ debian/gles-patches/convert-to-gles.patch	2016-05-17 19:58:11 +0000
@@ -84,7 +84,7 @@
 -Conflicts: libqtmir,
 -           libunity-mir1,
 -Provides: unity-application-impl,
--          unity-application-impl-15,
+-          unity-application-impl-16,
 -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/modules/Unity/Application/application.cpp'
--- src/modules/Unity/Application/application.cpp	2016-04-26 08:56:36 +0000
+++ src/modules/Unity/Application/application.cpp	2016-05-17 19:58:11 +0000
@@ -486,6 +486,7 @@
 
     if (m_session) {
         m_proxySurfaceList.setSourceList(nullptr);
+        m_proxyPromptSurfaceList.setSourceList(nullptr);
         m_session->disconnect(this);
         m_session->surfaceList()->disconnect(this);
         m_session->setApplication(nullptr);
@@ -527,6 +528,7 @@
             Q_EMIT fullscreenChanged(fullscreen());
 
         m_proxySurfaceList.setSourceList(m_session->surfaceList());
+        m_proxyPromptSurfaceList.setSourceList(m_session->promptSurfaceList());
     } else {
         // this can only happen after the session has stopped
         Q_ASSERT(m_state == InternalState::Stopped || m_state == InternalState::StoppedResumable
@@ -829,6 +831,11 @@
     return &m_proxySurfaceList;
 }
 
+unityapp::MirSurfaceListInterface* Application::promptSurfaceList()
+{
+    return &m_proxyPromptSurfaceList;
+}
+
 void Application::requestFocus()
 {
     if (m_proxySurfaceList.rowCount() > 0) {

=== modified file 'src/modules/Unity/Application/application.h'
--- src/modules/Unity/Application/application.h	2016-04-26 08:56:36 +0000
+++ src/modules/Unity/Application/application.h	2016-05-17 19:58:11 +0000
@@ -109,6 +109,7 @@
     QSize initialSurfaceSize() const override;
     void setInitialSurfaceSize(const QSize &size) override;
     unity::shell::application::MirSurfaceListInterface* surfaceList() override;
+    unity::shell::application::MirSurfaceListInterface* promptSurfaceList() override;
 
     ProcessState processState() const { return m_processState; }
     void setProcessState(ProcessState value);
@@ -191,6 +192,7 @@
     bool m_closing{false};
 
     ProxySurfaceListModel m_proxySurfaceList;
+    ProxySurfaceListModel m_proxyPromptSurfaceList;
 
     friend class ApplicationManager;
     friend class SessionManager;

=== modified file 'src/modules/Unity/Application/mirsurface.cpp'
--- src/modules/Unity/Application/mirsurface.cpp	2016-04-26 08:30:26 +0000
+++ src/modules/Unity/Application/mirsurface.cpp	2016-05-17 19:58:11 +0000
@@ -956,11 +956,6 @@
     return m_focused;
 }
 
-unity::shell::application::MirSurfaceListInterface* MirSurface::promptSurfaceList()
-{
-    return &m_promptSurfaceList;
-}
-
 void MirSurface::requestFocus()
 {
     DEBUG_MSG << "()";

=== modified file 'src/modules/Unity/Application/mirsurface.h'
--- src/modules/Unity/Application/mirsurface.h	2016-04-26 08:30:26 +0000
+++ src/modules/Unity/Application/mirsurface.h	2016-05-17 19:58:11 +0000
@@ -88,8 +88,6 @@
 
     bool focused() const override;
 
-    unity::shell::application::MirSurfaceListInterface* promptSurfaceList() override;
-
     Q_INVOKABLE void requestFocus() override;
     Q_INVOKABLE void close() override;
     Q_INVOKABLE void raise() override;
@@ -229,8 +227,6 @@
     };
     ClosingState m_closingState{NotClosing};
     AbstractTimer *m_closeTimer{nullptr};
-
-    MirSurfaceListModel m_promptSurfaceList;
 };
 
 } // namespace qtmir

=== modified file 'src/modules/Unity/Application/mirsurfacelistmodel.cpp'
--- src/modules/Unity/Application/mirsurfacelistmodel.cpp	2016-05-17 19:58:11 +0000
+++ src/modules/Unity/Application/mirsurfacelistmodel.cpp	2016-05-17 19:58:11 +0000
@@ -68,6 +68,7 @@
     Q_EMIT countChanged();
     if (count() == 1) {
         Q_EMIT emptyChanged();
+        Q_EMIT firstChanged();
     }
 }
 
@@ -88,6 +89,9 @@
         if (count() == 0) {
             Q_EMIT emptyChanged();
         }
+        if (i == 0) {
+            Q_EMIT firstChanged();
+        }
     }
 }
 
@@ -105,6 +109,10 @@
         m_surfaceList.move(from, to);
         endMoveRows();
     }
+
+    if ((from == 0 || to == 0) && m_surfaceList.count() > 1) {
+        Q_EMIT firstChanged();
+    }
 }
 
 unityapp::MirSurfaceInterface *MirSurfaceListModel::get(int index)
@@ -143,6 +151,7 @@
     if (wasEmpty) {
         Q_EMIT emptyChanged();
     }
+    Q_EMIT firstChanged();
 }
 
 void MirSurfaceListModel::addSurfaceList(MirSurfaceListModel *surfaceListModel)
@@ -233,6 +242,8 @@
         connect(m_sourceList, &QObject::destroyed, this, [this]() { this->setSourceList(nullptr); });
         connect(m_sourceList, &unityapp::MirSurfaceListInterface::countChanged,
                 this, &unityapp::MirSurfaceListInterface::countChanged);
+        connect(m_sourceList, &unityapp::MirSurfaceListInterface::firstChanged,
+                this, &unityapp::MirSurfaceListInterface::firstChanged);
     }
 
     endResetModel();

=== modified file 'src/modules/Unity/Application/session.cpp'
--- src/modules/Unity/Application/session.cpp	2016-05-10 20:13:37 +0000
+++ src/modules/Unity/Application/session.cpp	2016-05-17 19:58:11 +0000
@@ -137,6 +137,11 @@
     return &m_surfaceList;
 }
 
+MirSurfaceListModel* Session::promptSurfaceList()
+{
+    return &m_promptSurfaceList;
+}
+
 Session::State Session::state() const
 {
     return m_state;
@@ -199,14 +204,8 @@
     if (newSurface->isFirstFrameDrawn()) {
         prependSurface(newSurface);
     } else {
-        m_blankSurfaces.append(newSurface);
-        connect(newSurface, &QObject::destroyed, this, [this, newSurface]()
-            {
-                this->m_blankSurfaces.removeAll(newSurface);
-            });
         connect(newSurface, &MirSurfaceInterface::firstFrameDrawn, this, [this, newSurface]()
             {
-                this->m_blankSurfaces.removeAll(newSurface);
                 newSurface->disconnect(this);
                 this->prependSurface(newSurface);
             });
@@ -393,32 +392,19 @@
 void Session::addChildSession(SessionInterface* session)
 {
     insertChildSession(m_children->rowCount(), session);
-
-    if (m_surfaceList.count() > 0) {
-        // we assume that the top-most surface is the one that caused the prompt session to show up.
-        auto promptSurfaceList = static_cast<MirSurfaceListModel*>(m_surfaceList.get(0)->promptSurfaceList());
-        promptSurfaceList->addSurfaceList(session->surfaceList());
-    } else if (m_blankSurfaces.count() > 0) {
-        // Prompt session came in too early.
-        // Parent session might be blocked until the prompt session is dismissed.
-        // So we cannot wait anymore and must put that blank surface on the surface list so that the
-        // user can see the prompt surface on top of that it and interact.
-        auto blankSurface = m_blankSurfaces.takeFirst();
-
-        auto promptSurfaceList = static_cast<MirSurfaceListModel*>(blankSurface->promptSurfaceList());
-        promptSurfaceList->addSurfaceList(session->surfaceList());
-
-        blankSurface->disconnect(this);
-        prependSurface(blankSurface);
-    }
 }
 
 void Session::insertChildSession(uint index, SessionInterface* session)
 {
     qCDebug(QTMIR_SESSIONS) << "Session::insertChildSession - " << session->name() << " to " << name() << " @  " << index;
+    Q_ASSERT(!m_children->contains(session));
 
     m_children->insert(index, session);
 
+    // Flatten the list of prompt surfaces
+    m_promptSurfaceList.addSurfaceList(session->surfaceList());
+    m_promptSurfaceList.addSurfaceList(session->promptSurfaceList());
+
     connect(session, &QObject::destroyed, this, [this, session]() { this->removeChildSession(session); });
 
     switch (m_state) {
@@ -444,6 +430,8 @@
 
     if (m_children->contains(session)) {
         m_children->remove(session);
+        m_promptSurfaceList.removeSurfaceList(session->surfaceList());
+        m_promptSurfaceList.removeSurfaceList(session->promptSurfaceList());
     }
 
     deleteIfZombieAndEmpty();

=== modified file 'src/modules/Unity/Application/session.h'
--- src/modules/Unity/Application/session.h	2016-05-10 20:13:37 +0000
+++ src/modules/Unity/Application/session.h	2016-05-17 19:58:11 +0000
@@ -51,6 +51,7 @@
     QString name() const override;
     unity::shell::application::ApplicationInfoInterface* application() const override;
     MirSurfaceListModel* surfaceList() override;
+    MirSurfaceListModel* promptSurfaceList() override;
     State state() const override;
     bool fullscreen() const override;
     bool live() const override;
@@ -107,11 +108,7 @@
     std::shared_ptr<mir::scene::Session> m_session;
     Application* m_application;
     MirSurfaceListModel m_surfaceList;
-
-    // Registered surfaces that haven't yet drawn their first frames.
-    // Once they do, they get moved to m_surfaceList.
-    // So this list is just a temporary holder and most of the time it's gonna be empty.
-    QList<MirSurfaceInterface*> m_blankSurfaces;
+    MirSurfaceListModel m_promptSurfaceList;
 
     SessionModel* m_children;
     bool m_fullscreen;

=== modified file 'src/modules/Unity/Application/session_interface.h'
--- src/modules/Unity/Application/session_interface.h	2016-04-06 16:52:49 +0000
+++ src/modules/Unity/Application/session_interface.h	2016-05-17 19:58:11 +0000
@@ -68,6 +68,9 @@
     // List of surfaces in this session. Surfaces that are being forcibly closed are not present in this list
     virtual MirSurfaceListModel* surfaceList() = 0;
 
+    // List of prompt surfaces under this session (and its children)
+    virtual MirSurfaceListModel* promptSurfaceList() = 0;
+
     virtual SessionModel* childSessions() const = 0;
     virtual State state() const = 0;
     virtual bool fullscreen() const = 0;

=== modified file 'tests/framework/fake_mirsurface.h'
--- tests/framework/fake_mirsurface.h	2016-04-26 08:30:26 +0000
+++ tests/framework/fake_mirsurface.h	2016-05-17 19:58:11 +0000
@@ -77,8 +77,6 @@
 
     bool focused() const override { return false; }
 
-    unity::shell::application::MirSurfaceListInterface* promptSurfaceList() override { return &m_promptSurfaceList;}
-
     void requestFocus() override {
         Q_EMIT focusRequested();
     }
@@ -175,8 +173,6 @@
 
     QList<TouchEvent> m_touchesReceived;
 
-    MirSurfaceListModel m_promptSurfaceList;
-
     SessionInterface *m_session{nullptr};
 };
 

=== modified file 'tests/framework/fake_session.cpp'
--- tests/framework/fake_session.cpp	2016-03-28 18:02:26 +0000
+++ tests/framework/fake_session.cpp	2016-05-17 19:58:11 +0000
@@ -36,6 +36,8 @@
 
 MirSurfaceListModel* FakeSession::surfaceList() { return &m_surfaceList; }
 
+MirSurfaceListModel* FakeSession::promptSurfaceList() { return &m_promptSurfaceList; }
+
 SessionModel *FakeSession::childSessions() const { return nullptr; }
 
 SessionInterface::State FakeSession::state() const { return m_state; }

=== modified file 'tests/framework/fake_session.h'
--- tests/framework/fake_session.h	2016-03-28 18:02:26 +0000
+++ tests/framework/fake_session.h	2016-05-17 19:58:11 +0000
@@ -34,6 +34,7 @@
     QString name() const override;
     unity::shell::application::ApplicationInfoInterface* application() const override;
     MirSurfaceListModel* surfaceList() override;
+    MirSurfaceListModel* promptSurfaceList() override;
     SessionModel* childSessions() const override;
     State state() const override;
     bool fullscreen() const override;
@@ -81,6 +82,7 @@
     State m_state;
     std::shared_ptr<mir::scene::Session> m_session;
     MirSurfaceListModel m_surfaceList;
+    MirSurfaceListModel m_promptSurfaceList;
 };
 
 } // namespace qtmi

=== modified file 'tests/modules/SessionManager/session_test.cpp'
--- tests/modules/SessionManager/session_test.cpp	2016-05-10 20:13:37 +0000
+++ tests/modules/SessionManager/session_test.cpp	2016-05-17 19:58:11 +0000
@@ -311,46 +311,3 @@
     EXPECT_EQ(Session::Stopped, session->state());
     EXPECT_FALSE(session->m_suspendTimer->isRunning());
 }
-
-/*
-    Regression test for https://bugs.launchpad.net/ubuntu/+source/qtmir/+bug/1578665
-
-    A child prompt session was arriving before the application surface got a chance to draw
-    its first frame. And on top of that, the application was blocked from drawing anything
-    until that prompt session gone away.
- */
-TEST_F(SessionTests, ChildSessionArrivesBeforeSurfaceDrawnFirstFrame)
-{
-    using namespace testing;
-
-    const QString appId("test-app");
-    const pid_t procId = 5551;
-
-    auto mirSession = std::make_shared<MockSession>(appId.toStdString(), procId);
-
-    auto session = std::make_shared<qtmir::Session>(mirSession, promptSessionManager);
-
-    FakeMirSurface *blankSurface = new FakeMirSurface;
-    session->registerSurface(blankSurface);
-
-    // Surface doesn't appear in this list as it has not drawn its first frame yet
-    EXPECT_EQ(0, session->surfaceList()->count());
-
-    Session* childSession = new Session(mirSession, promptSessionManager);
-    session->addChildSession(childSession);
-
-    FakeMirSurface *promptSurface = new FakeMirSurface;
-    childSession->registerSurface(promptSurface);
-    promptSurface->drawFirstFrame();
-
-    // Even though blankSurface still hasn't drawn its first frame (ie, it's still blank),
-    // it should now appear in this list as it now has a child prompt surface, which the
-    // user must interact with.
-    auto sessionSurfaceList = static_cast<MirSurfaceListModel*>(session->surfaceList());
-    EXPECT_EQ(1, sessionSurfaceList->count());
-    EXPECT_TRUE(sessionSurfaceList->contains(blankSurface));
-
-    auto promptSurfaceList = static_cast<MirSurfaceListModel*>(blankSurface->promptSurfaceList());
-    EXPECT_EQ(1, promptSurfaceList->count());
-    EXPECT_TRUE(promptSurfaceList->contains(promptSurface));
-}