Merge lp:~mterry/qtmir/fix-wakelocks into lp:qtmir

[Michael Terry]

Don't hold a wakelock on apps that are exempt from lifecycle management.

This supports the new exemptFromLifecycle flag for applications as well as bringing back the internal RunningInBackground state.

The tests were revived from the same branch that removed them when we removed RunningInBackground (revision 400)

Don't hold a wakelock on apps that are exempt from lifecycle management.

This supports the new exemptFromLifecycle flag for applications as well as bringing back the internal RunningInBackground state.

The tests were revived from the same branch that removed them when we removed RunningInBackground [1] (and in hindsight, I should have thought harder about the implications of simply removing a test that confirmed we were releasing wakelocks).

This branch is related to unity-api [2] and unity8 [3] MPS. All of which are in service of fixing bug 1518764.

[1] https://code.launchpad.net/~mterry/qtmir/no-touch-no-lifecycle/+merge/272791

[2] https://code.launchpad.net/~mterry/unity-api/fix-wakelocks/+merge/279476
[3] https://code.launchpad.net/~mterry/unity8/fix-wakelocks/+merge/279489

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:424
http://jenkins.qa.ubuntu.com/job/qtmir-ci/593/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/289/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/289/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/171/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/326/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/326/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/171/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtmir-ci/593/rebuild

review: Needs Fixing (continuous-integration)

[lp:~mterry/qtmir/fix-wakelocks]

425. By Michael Terry on 2015-12-03

Merge from surfaceItemFillMode

[Daniel d'Andrada (dandrader)]

In Application::setInternalState(Application::InternalState state):

"""
        case InternalState::RunningInBackground:
            releaseWakelock();
            break;
"""

could be simply:

"""
        case InternalState::RunningInBackground:

"""

as the next case does the same.

[lp:~mterry/qtmir/fix-wakelocks]

426. By Michael Terry on 2015-12-03

Collapse a switch statement

Unmerged revisions

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2015-11-25 15:38:50 +0000
+++ CMakeLists.txt	2015-12-03 17:30:39 +0000
@@ -75,7 +75,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=11)
+pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application=12)
 
 include_directories(${APPLICATION_API_INCLUDE_DIRS})
 

=== modified file 'debian/control'
--- debian/control	2015-11-25 15:38:50 +0000
+++ debian/control	2015-12-03 17:30:39 +0000
@@ -22,7 +22,7 @@
                libubuntu-app-launch2-dev,
                libubuntu-application-api-dev (>= 2.1.0),
                libudev-dev,
-               libunity-api-dev (>= 7.103),
+               libunity-api-dev (>= 7.104),
                liburl-dispatcher1-dev,
                libxkbcommon-dev,
                libxrender-dev,
@@ -93,7 +93,7 @@
 Conflicts: libqtmir,
            libunity-mir1,
 Provides: unity-application-impl,
-          unity-application-impl-11,
+          unity-application-impl-12,
 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	2015-10-01 17:20:40 +0000
+++ src/modules/Unity/Application/application.cpp	2015-12-03 17:30:39 +0000
@@ -52,6 +52,7 @@
     , m_session(nullptr)
     , m_requestedState(RequestedRunning)
     , m_processState(ProcessUnknown)
+    , m_exemptFromLifecycle(false)
 {
     qCDebug(QTMIR_APPLICATIONS) << "Application::Application - appId=" << desktopFileReader->appId();
 
@@ -78,6 +79,7 @@
     switch (m_state) {
     case InternalState::Starting:
     case InternalState::Running:
+    case InternalState::RunningInBackground:
     case InternalState::SuspendingWaitSession:
     case InternalState::SuspendingWaitProcess:
         wipeQMLCache();
@@ -209,6 +211,8 @@
         return "Starting";
     case InternalState::Running:
         return "Running";
+    case InternalState::RunningInBackground:
+        return "RunningInBackground";
     case InternalState::SuspendingWaitSession:
         return "SuspendingWaitSession";
     case InternalState::SuspendingWaitProcess:
@@ -277,6 +281,7 @@
     case InternalState::Starting:
         return Starting;
     case InternalState::Running:
+    case InternalState::RunningInBackground:
     case InternalState::SuspendingWaitSession:
     case InternalState::SuspendingWaitProcess:
     case InternalState::Closing:
@@ -327,6 +332,7 @@
     case InternalState::Running:
         // already where it's wanted to be
         break;
+    case InternalState::RunningInBackground:
     case InternalState::SuspendingWaitSession:
     case InternalState::Suspended:
         resume();
@@ -359,6 +365,7 @@
             Q_ASSERT(m_processState == ProcessUnknown);
         }
         break;
+    case InternalState::RunningInBackground:
     case InternalState::SuspendingWaitSession:
     case InternalState::SuspendingWaitProcess:
     case InternalState::Suspended:
@@ -408,6 +415,7 @@
     case InternalState::Running:
         setInternalState(InternalState::Closing);
         break;
+    case InternalState::RunningInBackground:
     case InternalState::SuspendingWaitSession:
     case InternalState::SuspendingWaitProcess:
     case InternalState::Suspended:
@@ -452,6 +460,7 @@
         switch (m_state) {
         case InternalState::Starting:
         case InternalState::Running:
+        case InternalState::RunningInBackground:
         case InternalState::Closing:
             m_session->resume();
             break;
@@ -510,6 +519,9 @@
         case InternalState::Running:
             acquireWakelock();
             break;
+        case InternalState::RunningInBackground:
+            releaseWakelock();
+            break;
         case InternalState::Suspended:
             releaseWakelock();
             break;
@@ -606,8 +618,15 @@
     Q_ASSERT(m_state == InternalState::Running);
     Q_ASSERT(m_session != nullptr);
 
-    setInternalState(InternalState::SuspendingWaitSession);
-    m_session->suspend();
+    if (exemptFromLifecycle()) {
+        // There's no need to keep the wakelock as the process is never suspended
+        // and thus has no cleanup to perform when (for example) the display is
+        // blanked.
+        setInternalState(InternalState::RunningInBackground);
+    } else {
+        setInternalState(InternalState::SuspendingWaitSession);
+        m_session->suspend();
+    }
 }
 
 void Application::resume()
@@ -622,6 +641,8 @@
     } else if (m_state == InternalState::SuspendingWaitSession) {
         setInternalState(InternalState::Running);
         m_session->resume();
+    } else if (m_state == InternalState::RunningInBackground) {
+        setInternalState(InternalState::Running);
     }
 }
 
@@ -639,6 +660,22 @@
     return m_desktopData->isTouchApp();
 }
 
+bool Application::exemptFromLifecycle() const
+{
+    return m_exemptFromLifecycle;
+}
+
+void Application::setExemptFromLifecycle(bool exemptFromLifecycle)
+{
+    if (m_exemptFromLifecycle != exemptFromLifecycle)
+    {
+        // We don't adjust current suspension state, we only care about exempt
+        // status going into a suspend.
+        m_exemptFromLifecycle = exemptFromLifecycle;
+        Q_EMIT exemptFromLifecycleChanged(m_exemptFromLifecycle);
+    }
+}
+
 QString Application::longAppId() const
 {
     return m_longAppId;

=== modified file 'src/modules/Unity/Application/application.h'
--- src/modules/Unity/Application/application.h	2015-11-17 14:16:22 +0000
+++ src/modules/Unity/Application/application.h	2015-12-03 17:30:39 +0000
@@ -68,6 +68,7 @@
     enum class InternalState {
         Starting,
         Running,
+        RunningInBackground,
         SuspendingWaitSession,
         SuspendingWaitProcess,
         Suspended,
@@ -104,6 +105,8 @@
     Qt::ScreenOrientations supportedOrientations() const override;
     bool rotatesWindowContents() const override;
     bool isTouchApp() const override;
+    bool exemptFromLifecycle() const override;
+    void setExemptFromLifecycle(bool) override;
 
     void setStage(Stage stage);
 
@@ -178,6 +181,7 @@
     SessionInterface *m_session;
     RequestedState m_requestedState;
     ProcessState m_processState;
+    bool m_exemptFromLifecycle;
 
     friend class ApplicationManager;
     friend class SessionManager;

=== modified file 'src/modules/Unity/Application/application_manager.cpp'
--- src/modules/Unity/Application/application_manager.cpp	2015-11-17 14:16:22 +0000
+++ src/modules/Unity/Application/application_manager.cpp	2015-12-03 17:30:39 +0000
@@ -229,6 +229,8 @@
                 return QVariant::fromValue(application->focused());
             case RoleIsTouchApp:
                 return QVariant::fromValue(application->isTouchApp());
+            case RoleExemptFromLifecycle:
+                return QVariant::fromValue(application->exemptFromLifecycle());
             case RoleSession:
                 return QVariant::fromValue(application->session());
             case RoleFullscreen:

=== modified file 'src/modules/Unity/Application/application_manager.h'
--- src/modules/Unity/Application/application_manager.h	2015-11-17 14:16:22 +0000
+++ src/modules/Unity/Application/application_manager.h	2015-12-03 17:30:39 +0000
@@ -68,7 +68,7 @@
 
     // FIXME: these roles should be added to unity-api and removed from here
     enum MoreRoles {
-        RoleSession = RoleIsTouchApp+1,
+        RoleSession = RoleExemptFromLifecycle+1,
         RoleFullscreen,
     };
 

=== modified file 'tests/modules/ApplicationManager/application_manager_test.cpp'
--- tests/modules/ApplicationManager/application_manager_test.cpp	2015-11-17 14:16:22 +0000
+++ tests/modules/ApplicationManager/application_manager_test.cpp	2015-12-03 17:30:39 +0000
@@ -1737,6 +1737,114 @@
     }
 }
 
+TEST_F(ApplicationManagerTests, lifecycleExemptAppIsNotSuspended)
+{
+    using namespace ::testing;
+
+    const QString appId("testAppId");
+    const quint64 procId = 12345;
+
+    // Set up Mocks & signal watcher
+    auto mockDesktopFileReader = new NiceMock<MockDesktopFileReader>(appId, QFileInfo());
+    ON_CALL(*mockDesktopFileReader, loaded()).WillByDefault(Return(true));
+    ON_CALL(*mockDesktopFileReader, appId()).WillByDefault(Return(appId));
+
+    ON_CALL(desktopFileReaderFactory, createInstance(appId, _)).WillByDefault(Return(mockDesktopFileReader));
+
+    EXPECT_CALL(appController, startApplicationWithAppIdAndArgs(appId, _))
+            .Times(1)
+            .WillOnce(Return(true));
+
+    auto the_app = applicationManager.startApplication(appId, ApplicationManager::NoFlag);
+    applicationManager.onProcessStarting(appId);
+    std::shared_ptr<mir::scene::Session> session = std::make_shared<MockSession>("", procId);
+    bool authed = true;
+    applicationManager.authorizeSession(procId, authed);
+    onSessionStarting(session);
+
+    // App creates surface, focuses it so state is running
+    FakeMirSurface *surface = new FakeMirSurface;
+    onSessionCreatedSurface(session.get(), surface);
+    surface->drawFirstFrame();
+
+    // Test normal lifecycle management as a control group
+    ASSERT_EQ(Application::InternalState::Running, the_app->internalState());
+    ASSERT_EQ(Application::ProcessState::ProcessRunning, the_app->processState());
+
+    EXPECT_CALL(*(mir::scene::MockSession*)session.get(), set_lifecycle_state(mir_lifecycle_state_will_suspend));
+    the_app->setRequestedState(Application::RequestedSuspended);
+    ASSERT_EQ(Application::InternalState::SuspendingWaitSession, the_app->internalState());
+
+    static_cast<qtmir::Session*>(the_app->session())->doSuspend();
+    ASSERT_EQ(Application::InternalState::SuspendingWaitProcess, the_app->internalState());
+    applicationManager.onProcessSuspended(the_app->appId());
+    ASSERT_EQ(Application::InternalState::Suspended, the_app->internalState());
+
+    EXPECT_CALL(*(mir::scene::MockSession*)session.get(), set_lifecycle_state(mir_lifecycle_state_resumed));
+    the_app->setRequestedState(Application::RequestedRunning);
+
+    EXPECT_EQ(Application::Running, the_app->state());
+
+    // Now mark the app as exempt from lifecycle management and retest
+    the_app->setExemptFromLifecycle(true);
+
+    EXPECT_EQ(Application::Running, the_app->state());
+
+    EXPECT_CALL(*(mir::scene::MockSession*)session.get(), set_lifecycle_state(_)).Times(0);
+    the_app->setRequestedState(Application::RequestedSuspended);
+
+    // And expect it to be running still
+    ASSERT_EQ(Application::InternalState::RunningInBackground, the_app->internalState());
+
+    the_app->setRequestedState(Application::RequestedRunning);
+
+    EXPECT_EQ(Application::Running, the_app->state());
+    ASSERT_EQ(Application::InternalState::Running, the_app->internalState());
+}
+
+/*
+ * Test lifecycle exempt applications have their wakelocks released when shell tries to suspend them
+ */
+TEST_F(ApplicationManagerTests, lifecycleExemptAppHasWakelockReleasedOnAttemptedSuspend)
+{
+    using namespace ::testing;
+
+    const QString appId("testAppId");
+    const quint64 procId = 12345;
+
+    // Set up Mocks & signal watcher
+    auto mockDesktopFileReader = new NiceMock<MockDesktopFileReader>(appId, QFileInfo());
+    ON_CALL(*mockDesktopFileReader, loaded()).WillByDefault(Return(true));
+    ON_CALL(*mockDesktopFileReader, appId()).WillByDefault(Return(appId));
+
+    ON_CALL(desktopFileReaderFactory, createInstance(appId, _)).WillByDefault(Return(mockDesktopFileReader));
+
+    EXPECT_CALL(appController, startApplicationWithAppIdAndArgs(appId, _))
+            .Times(1)
+            .WillOnce(Return(true));
+
+    auto application = applicationManager.startApplication(appId, ApplicationManager::NoFlag);
+    applicationManager.onProcessStarting(appId);
+    std::shared_ptr<mir::scene::Session> session = std::make_shared<MockSession>("", procId);
+    bool authed = true;
+    applicationManager.authorizeSession(procId, authed);
+    onSessionStarting(session);
+
+    // App creates surface, focuses it so state is running
+    FakeMirSurface *surface = new FakeMirSurface;
+    onSessionCreatedSurface(session.get(), surface);
+    surface->drawFirstFrame();
+
+    // Mark app as exempt
+    application->setExemptFromLifecycle(true);
+
+    application->setRequestedState(Application::RequestedSuspended);
+
+    EXPECT_FALSE(sharedWakelock.enabled());
+    ASSERT_EQ(Application::InternalState::RunningInBackground, application->internalState());
+    EXPECT_EQ(Application::Running, application->state());
+}
+
 /*
  * Test that when user stops an application, application does not delete QML cache
  */

=== modified file 'tests/modules/common/qtmir_test.cpp'
--- tests/modules/common/qtmir_test.cpp	2015-10-01 16:48:38 +0000
+++ tests/modules/common/qtmir_test.cpp	2015-12-03 17:30:39 +0000
@@ -26,6 +26,9 @@
     case Application::InternalState::Running:
         *os << "Running";
         break;
+    case Application::InternalState::RunningInBackground:
+        *os << "RunningInBackground";
+        break;
     case Application::InternalState::SuspendingWaitSession:
         *os << "SuspendingWaitSession";
         break;