Merge lp:~unity-team/qtmir/dash-killed-less-likely into lp:qtmir

[Michał Sawicz]

Ensure unity8-dash is less likely to be killed than other background apps.

 * Are there any related MPs required for this MP to build/function as expected? Please list.
N
 * Did you perform an exploratory manual test run of your code change and any related functionality?
Not yet, need to build.
 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:267
http://jenkins.qa.ubuntu.com/job/qtmir-ci/134/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-utopic-amd64-ci/134
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-utopic-armhf-ci/134
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-utopic-armhf-ci/134/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Oliver Grawert (ogra)]

tested and seems to work fine, the dash is not restarted anymore no matter how many apps i open

[Gerry Boland (gerboland)]

LGTM

review: Approve

[Gerry Boland (gerboland)]

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Y
 * Did CI run pass? If not, please explain why.
Fine

=== modified file 'src/modules/Unity/Application/processcontroller.cpp'
--- src/modules/Unity/Application/processcontroller.cpp	2014-07-18 12:14:30 +0000
+++ src/modules/Unity/Application/processcontroller.cpp	2014-10-10 08:40:09 +0000
@@ -115,6 +115,43 @@
 }
 
 /*!
+ * \brief ProcessController::OomController::ensureProcessLessLikelyToBeKilled
+ * \param pid
+ * Set the process OOM-killer score so that, in a low memory situation, the process
+ * is _more_ likely to be killed than the foreground application, but less likely
+ * than the background applications
+ */
+void ProcessController::OomController::ensureProcessLessLikelyToBeKilled(pid_t pid)
+{
+    // Set it to 50% of the total available range.
+    static const float defaultPercentage = 0.5;
+
+    core::posix::Process process(pid);
+
+    try {
+        plpp::OomScoreAdj shellScore;
+        core::posix::this_process::instance() >> shellScore;
+
+        plpp::OomScoreAdj processScore
+        {
+            static_cast<int>((plpp::OomScoreAdj::max_value() - shellScore.value) * defaultPercentage) + shellScore.value
+        };
+
+        process << processScore;
+    } catch(...) {
+        // Accessing OomScoreAdj resulted in an exception being thrown.
+        // Trying with the deprecated OomAdj now as a last resort.
+        try
+        {
+            process << plpp::OomAdj{plpp::OomAdj::max_value()};
+        } catch(...)
+        {
+            qDebug() << "ensureProcessIsLessLikelyToBeKilled failed for pid=" << pid;
+        }
+    }
+}
+
+/*!
  * \brief ProcessController::OomController::ensureProcessUnlikelyToBeKilled
  * \param pid
  * Set the process OOM-killer weighting so that the process is _less_ likely to be killed

=== modified file 'src/modules/Unity/Application/processcontroller.h'
--- src/modules/Unity/Application/processcontroller.h	2014-07-01 13:38:06 +0000
+++ src/modules/Unity/Application/processcontroller.h	2014-10-10 08:40:09 +0000
@@ -38,6 +38,7 @@
         OomController& operator=(const OomController&) = delete;
 
         virtual void ensureProcessLikelyToBeKilled(pid_t);
+        virtual void ensureProcessLessLikelyToBeKilled(pid_t);
         virtual void ensureProcessUnlikelyToBeKilled(pid_t);
     };
 

=== modified file 'src/modules/Unity/Application/taskcontroller.cpp'
--- src/modules/Unity/Application/taskcontroller.cpp	2014-09-18 09:39:48 +0000
+++ src/modules/Unity/Application/taskcontroller.cpp	2014-10-10 08:40:09 +0000
@@ -116,7 +116,14 @@
     if (pid == 0) {
         pid = app->pid();
     }
-    m_processController->oomController()->ensureProcessLikelyToBeKilled(pid);
+
+    // FIXME This harcodes the unity8-dash name, we should consider a different approach,
+    // some privileged way to define which apps should be less likely to be killed.
+    if (app->appId() == "unity8-dash") {
+        m_processController->oomController()->ensureProcessLessLikelyToBeKilled(pid);
+    } else {
+        m_processController->oomController()->ensureProcessLikelyToBeKilled(pid);
+    }
 
     if (pid) {
         // We do assume that the app was launched by ubuntu-app-launch and with that,

=== modified file 'tests/modules/ApplicationManager/application_manager_test.cpp'
--- tests/modules/ApplicationManager/application_manager_test.cpp	2014-10-06 11:59:39 +0000
+++ tests/modules/ApplicationManager/application_manager_test.cpp	2014-10-10 08:40:09 +0000
@@ -60,6 +60,7 @@
     EXPECT_CALL(processController, sigContinueProcessGroupForPid(_)).Times(1);
     EXPECT_CALL(oomController, ensureProcessUnlikelyToBeKilled(_)).Times(1);
     EXPECT_CALL(oomController, ensureProcessLikelyToBeKilled(_)).Times(1);
+    EXPECT_CALL(oomController, ensureProcessLessLikelyToBeKilled(_)).Times(0);
 
     auto application = applicationManager.startApplication(
                 appId,
@@ -72,6 +73,41 @@
     QMetaObject::invokeMethod(application, "onSessionResumed");
 }
 
+TEST_F(ApplicationManagerTests, SuspendingAndResumingDashResultsInOomScoreAdjustment)
+{
+    using namespace ::testing;
+
+    quint64 procId = 5921;
+    std::shared_ptr<mir::scene::Surface> aSurface(nullptr);
+    QByteArray cmdLine( "/usr/bin/app1 --desktop_file_hint=unity8-dash");
+
+    EXPECT_CALL(procInfo,command_line(procId))
+        .Times(1)
+        .WillOnce(Return(cmdLine));
+
+    ON_CALL(appController,appIdHasProcessId(_,_)).WillByDefault(Return(false));
+
+    EXPECT_CALL(processController, sigStopProcessGroupForPid(_)).Times(1);
+    EXPECT_CALL(processController, sigContinueProcessGroupForPid(_)).Times(1);
+    EXPECT_CALL(oomController, ensureProcessUnlikelyToBeKilled(_)).Times(1);
+    EXPECT_CALL(oomController, ensureProcessLikelyToBeKilled(_)).Times(0);
+    EXPECT_CALL(oomController, ensureProcessLessLikelyToBeKilled(_)).Times(1);
+
+    bool authed = true;
+
+    std::shared_ptr<mir::scene::Session> session = std::make_shared<MockSession>("Oo", procId);
+    applicationManager.authorizeSession(procId, authed);
+    onSessionStarting(session);
+
+    auto application = applicationManager.findApplication("unity8-dash");
+    applicationManager.onSessionCreatedSurface(session.get(), aSurface);
+
+    // FIXME - this is doesn't really excerise the actualt behaviour since suspend/resume should be
+    // controlled by state changes. Requires using suspend timer.
+    QMetaObject::invokeMethod(application, "onSessionSuspended");
+    QMetaObject::invokeMethod(application, "onSessionResumed");
+}
+
 // Currently disabled as we need to make sure that we have a corresponding mir session, too.
 TEST_F(ApplicationManagerTests, DISABLED_FocusingRunningApplicationResultsInOomScoreAdjustment)
 {

=== modified file 'tests/modules/common/mock_oom_controller.h'
--- tests/modules/common/mock_oom_controller.h	2014-07-01 13:38:06 +0000
+++ tests/modules/common/mock_oom_controller.h	2014-10-10 08:40:09 +0000
@@ -26,6 +26,7 @@
 struct MockOomController : public qtmir::ProcessController::OomController
 {
     MOCK_METHOD1(ensureProcessLikelyToBeKilled, void(pid_t));
+    MOCK_METHOD1(ensureProcessLessLikelyToBeKilled, void(pid_t));
     MOCK_METHOD1(ensureProcessUnlikelyToBeKilled, void(pid_t));
 };
 }