Merge lp:~gerboland/qtmir/duplicate-open-apps into lp:qtmir

[Gerry Boland]

TaskController may call processStarted synchronously, so check for that in startApplication before adding

TaskController may call processStarted synchronously, so check for that in startApplication before adding

* 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?
Y
 * 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:251
http://jenkins.qa.ubuntu.com/job/qtmir-ci/72/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-utopic-amd64-ci/72
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-utopic-armhf-ci/72
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-utopic-armhf-ci/72/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Michael Zanetti (mzanetti)]

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

yes

 * Did CI run pass? If not, please explain why.

waiting...

review: Approve

=== modified file 'src/modules/Unity/Application/application.cpp'
--- src/modules/Unity/Application/application.cpp	2014-09-11 16:18:40 +0000
+++ src/modules/Unity/Application/application.cpp	2014-09-18 12:02:01 +0000
@@ -162,6 +162,11 @@
     m_pid = pid;
 }
 
+void Application::setArguments(const QStringList arguments)
+{
+    m_arguments = arguments;
+}
+
 void Application::setSession(Session *newSession)
 {
     qCDebug(QTMIR_APPLICATIONS) << "Application::setSession - appId=" << appId() << "session=" << newSession;

=== modified file 'src/modules/Unity/Application/application.h'
--- src/modules/Unity/Application/application.h	2014-08-29 11:01:20 +0000
+++ src/modules/Unity/Application/application.h	2014-09-18 12:02:01 +0000
@@ -116,6 +116,7 @@
     QString longAppId() const;
     pid_t pid() const;
     void setPid(pid_t pid);
+    void setArguments(const QStringList arguments);
     void setFocused(bool focus);
     void setSession(Session *session);
 

=== modified file 'src/modules/Unity/Application/application_manager.cpp'
--- src/modules/Unity/Application/application_manager.cpp	2014-09-04 11:11:26 +0000
+++ src/modules/Unity/Application/application_manager.cpp	2014-09-18 12:02:01 +0000
@@ -439,24 +439,30 @@
         return nullptr;
     }
 
-    application = new Application(
-                m_taskController,
-                m_desktopFileReaderFactory->createInstance(appId, m_taskController->findDesktopFileForAppId(appId)),
-                Application::Starting,
-                arguments,
-                this);
-
-    if (!application->isValid()) {
-        qWarning() << "Unable to instantiate application with appId" << appId;
-        return nullptr;
-    }
-
-    // override stage if necessary
-    if (application->stage() == Application::SideStage && flags.testFlag(ApplicationManager::ForceMainStage)) {
-        application->setStage(Application::MainStage);
-    }
-
-    add(application);
+    // The TaskController may synchroneously callback onProcessStarting, so check if application already added
+    application = findApplication(appId);
+    if (application) {
+        application->setArguments(arguments);
+    } else {
+        application = new Application(
+                    m_taskController,
+                    m_desktopFileReaderFactory->createInstance(appId, m_taskController->findDesktopFileForAppId(appId)),
+                    Application::Starting,
+                    arguments,
+                    this);
+
+        if (!application->isValid()) {
+            qWarning() << "Unable to instantiate application with appId" << appId;
+            return nullptr;
+        }
+
+        // override stage if necessary
+        if (application->stage() == Application::SideStage && flags.testFlag(ApplicationManager::ForceMainStage)) {
+            application->setStage(Application::MainStage);
+        }
+
+        add(application);
+    }
     return application;
 }
 

=== modified file 'tests/modules/ApplicationManager/application_manager_test.cpp'
--- tests/modules/ApplicationManager/application_manager_test.cpp	2014-09-11 16:18:40 +0000
+++ tests/modules/ApplicationManager/application_manager_test.cpp	2014-09-18 12:02:01 +0000
@@ -687,6 +687,49 @@
 }
 
 /*
+ * Test that if TaskController synchronously calls back processStarted, that ApplicationManager
+ * does not add the app to the model twice.
+ */
+TEST_F(ApplicationManagerTests,synchronousProcessStartedCallDoesNotDuplicateEntryInModel)
+{
+    using namespace ::testing;
+    const QString appId("testAppId");
+    const QString name("Test App");
+
+    // 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(*mockDesktopFileReader, name()).WillByDefault(Return(name));
+
+    ON_CALL(desktopFileReaderFactory, createInstance(appId, _)).WillByDefault(Return(mockDesktopFileReader));
+
+    ON_CALL(appController, startApplicationWithAppIdAndArgs(appId, _))
+        .WillByDefault(Invoke(
+                        [&](const QString &appId, Unused) {
+                            applicationManager.onProcessStarting(appId);
+                            return true;
+                        }
+                      ));
+
+    // start the application
+    Application *theApp = applicationManager.startApplication(appId, ApplicationManager::NoFlag);
+
+    // check application data
+    EXPECT_EQ(theApp->state(), Application::Starting);
+    EXPECT_EQ(theApp->appId(), appId);
+    EXPECT_EQ(theApp->name(), name);
+    EXPECT_EQ(theApp->canBeResumed(), true);
+
+    // check only once instance in the model
+    EXPECT_EQ(applicationManager.count(), 1);
+
+    // check application in list of apps
+    Application *theAppAgain = applicationManager.findApplication(appId);
+    EXPECT_EQ(theAppAgain, theApp);
+}
+
+/*
  * Test that the child sessions of a webapp process are accepted
  */
 TEST_F(ApplicationManagerTests,webAppSecondarySessionsAccepted)