Merge lp:~gerboland/unity-mir/use-upstart-app-launch2 into lp:unity-mir

Proposed by Gerry Boland
Status: Merged
Approved by: Daniel d'Andrada
Approved revision: 103
Merged at revision: 94
Proposed branch: lp:~gerboland/unity-mir/use-upstart-app-launch2
Merge into: lp:unity-mir
Diff against target: 866 lines (+299/-239)
10 files modified
debian/control (+2/-0)
src/modules/Unity/Application/Application.pro (+1/-1)
src/modules/Unity/Application/application.cpp (+28/-18)
src/modules/Unity/Application/application.h (+3/-5)
src/modules/Unity/Application/application_manager.cpp (+124/-176)
src/modules/Unity/Application/application_manager.h (+8/-6)
src/modules/Unity/Application/applicationscreenshotprovider.cpp (+7/-0)
src/modules/Unity/Application/desktopfilereader.cpp (+2/-1)
src/modules/Unity/Application/taskcontroller.cpp (+101/-27)
src/modules/Unity/Application/taskcontroller.h (+23/-5)
To merge this branch: bzr merge lp:~gerboland/unity-mir/use-upstart-app-launch2
Reviewer Review Type Date Requested Status
Daniel d'Andrada (community) Approve
PS Jenkins bot (community) continuous-integration Approve
MichaƂ Sawicz code Approve
Review via email: mp+187769@code.launchpad.net

Commit message

Use upstart-app-launch for app start/stop

Description of the change

Use upstart-app-launch for app start/stop

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
97. By Gerry Boland

If process ends not by hand of shell, clean up after it properly

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

+ DLOG("Application::Application (this=%p, appId=%p, state=%d", this, qPrintable(appId), static_cast<int>(state));

s/appId=%p/appId=%s

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
98. By Gerry Boland

s/appId=%p/appId=%s - well spotted Daniel

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

"""
|@@ -354,12 +271,14 @@ void ApplicationManager::authorizeSessio
| authorized = false; //to be proven wrong
|
| DLOG("ApplicationManager::authorizeSession (this=%p, pid=%lld)", this, pid);
|- Application* application = findApplicationWithPid(pid);
|- if (application) {
""""

Speaking of log messages, I know this code is not touched by your patch but for extra bonus points it would be nice if
s/pid=%lld)"/pid=%"PRIu64")" for architecture independence (it's also an unsigned type, so lld is is wrong in more than one way).

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

"""
@@ -428,7 +347,10 @@ void ApplicationManager::authorizeSessio

     QString argStr(command.data());
     QStringList arguments(argStr.split(' '));
- application = new Application(desktopData, pid, stage, Application::Starting, arguments, m_taskController);
+ application = new Application(desktopData->appId(), Application::Starting, arguments, this);
+ delete desktopData;
+ application->setPid(pid);
+ application->setStage(stage);
"""

It's rather wasteful to parse a desktop file twice in this situation. Can't we easily avoid it by having a second constructor that takes a DesktopFileReader instead of a appId as an argument?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
99. By Gerry Boland

Support respawning apps again

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

I wonder if we can fix the mess that a pid is represented as a pid_t, a qint64 and a quint64.

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

Code-wise, my only complaint is the unnecessary recreation of the DesktopFileReader I explained above.

review: Needs Fixing
100. By Gerry Boland

Some handy extra debug info

101. By Gerry Boland

Fix for quick focus->unfocus->focus caused app to still suspend

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Gerry Boland (gerboland) wrote :

> I wonder if we can fix the mess that a pid is represented as a pid_t, a qint64
> and a quint64.
Yep, it's a mess, and on my TODO list to fix in another MR.

Revision history for this message
Gerry Boland (gerboland) wrote :

> It's rather wasteful to parse a desktop file twice in this situation. Can't we
> easily avoid it by having a second constructor that takes a DesktopFileReader
> instead of a appId as an argument?
You raise a fair point. I wanted to avoid having to parse desktop files in ApplicationManager at all, delegating it to Application alone, but the authorizer forced my hand.

I did the duplicate as I hope much of that authorizer code will go away soon. But I'll try to improve it now.

102. By Gerry Boland

Application constructor can take both appId or DesktopFileReader

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
103. By Gerry Boland

Add todo for screenshot

Revision history for this message
MichaƂ Sawicz (saviq) wrote :

Code looks good, didn't test though.

review: Approve (code)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

Looks good now.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/control'
2--- debian/control 2013-09-25 21:06:22 +0000
3+++ debian/control 2013-09-27 15:13:07 +0000
4@@ -8,6 +8,7 @@
5 libmirserver-dev (>= 0.0.12),
6 libmirclient-dev (>= 0.0.12),
7 libunity-api-dev,
8+ libupstart-app-launch1-dev,
9 qt5-default,
10 qtbase5-dev,
11 qtdeclarative5-dev,
12@@ -39,6 +40,7 @@
13 Depends: ${misc:Depends},
14 ${shlibs:Depends},
15 libunity-mir1 (= ${binary:Version}),
16+ libupstart-app-launch1-dev,
17 libmirserver-dev (>= 0.0.11),
18 libmirclient-dev (>= 0.0.11),
19 libplatform-api1-dev,
20
21=== modified file 'src/modules/Unity/Application/Application.pro'
22--- src/modules/Unity/Application/Application.pro 2013-09-11 13:20:08 +0000
23+++ src/modules/Unity/Application/Application.pro 2013-09-27 15:13:07 +0000
24@@ -9,7 +9,7 @@
25 QMAKE_CXXFLAGS_RELEASE += -Werror # so no stop on warning in debug builds
26 QMAKE_LFLAGS = -std=c++11 -Wl,-no-undefined
27
28-PKGCONFIG += mircommon mirserver ubuntu-platform-api
29+PKGCONFIG += mircommon mirserver ubuntu-platform-api glib-2.0 upstart-app-launch-1
30
31 INCLUDEPATH += ../../../unity-mir
32 LIBS += -L../../../unity-mir -lunity-mir \
33
34=== modified file 'src/modules/Unity/Application/application.cpp'
35--- src/modules/Unity/Application/application.cpp 2013-09-12 17:29:51 +0000
36+++ src/modules/Unity/Application/application.cpp 2013-09-27 15:13:07 +0000
37@@ -18,6 +18,7 @@
38 #include "application.h"
39 #include "application_manager.h"
40 #include "desktopfilereader.h"
41+#include "taskcontroller.h"
42
43 // unity-mir
44 #include "logging.h"
45@@ -25,24 +26,26 @@
46 // mir
47 #include <mir/shell/application_session.h>
48
49-Application::Application(DesktopFileReader* desktopData, qint64 pid,
50- Application::Stage stage, Application::State state,
51- const QStringList& arguments, TaskController* taskController,
52- QObject* parent)
53- : ApplicationInfoInterface(desktopData->appId(), parent)
54- , m_desktopData(desktopData)
55- , m_pid(pid)
56- , m_stage(stage)
57+Application::Application(const QString &appId, Application::State state,
58+ const QStringList &arguments, QObject *parent)
59+ : Application(new DesktopFileReader(appId), state, arguments, parent)
60+{
61+}
62+
63+Application::Application(DesktopFileReader *desktopFileReader, State state,
64+ const QStringList &arguments, QObject *parent)
65+ : ApplicationInfoInterface(desktopFileReader->appId(), parent)
66+ , m_desktopData(desktopFileReader)
67+ , m_pid(0)
68+ , m_stage((m_desktopData->stageHint() == "SideStage") ? Application::SideStage : Application::MainStage)
69 , m_state(state)
70 , m_focused(false)
71 , m_fullscreen(false)
72 , m_arguments(arguments)
73- , m_taskController(taskController)
74 , m_suspendTimer(new QTimer(this))
75 {
76- DASSERT(desktopData != NULL);
77- DLOG("Application::Application (this=%p, desktopData=%p, pid=%lld, stage=%d, state=%d",
78- this, desktopData, pid, static_cast<int>(stage), static_cast<int>(state));
79+ DLOG("Application::Application (this=%p, appId=%s, state=%d", this, qPrintable(desktopFileReader->appId()),
80+ static_cast<int>(state));
81
82 m_suspendTimer->setSingleShot(true);
83 connect(m_suspendTimer, SIGNAL(timeout()), this, SLOT(suspend()));
84@@ -54,6 +57,11 @@
85 delete m_desktopData;
86 }
87
88+bool Application::isValid() const
89+{
90+ return m_desktopData->loaded();
91+}
92+
93 QString Application::desktopFile() const
94 {
95 return m_desktopData->file();
96@@ -130,6 +138,8 @@
97
98 void Application::setSession(const std::shared_ptr<mir::shell::ApplicationSession>& session)
99 {
100+ DLOG("Application::setSession (this=%p, session=%p)", this, session.get());
101+
102 // TODO(greyback) what if called with new surface?
103 m_session = session;
104 }
105@@ -166,6 +176,9 @@
106 }
107 break;
108 case Application::Running:
109+ if (m_suspendTimer->isActive())
110+ m_suspendTimer->stop();
111+
112 if (m_state == Application::Suspended) {
113 resume();
114 session()->set_lifecycle_state(mir_lifecycle_state_resumed);
115@@ -207,20 +220,17 @@
116 void Application::suspend()
117 {
118 DLOG("Application::suspend (this=%p)", this);
119-
120- m_taskController->do_suspend(this);
121+ TaskController::singleton()->suspend(appId());
122 }
123
124 void Application::resume()
125 {
126 DLOG("Application::resume (this=%p)", this);
127-
128- m_taskController->do_resume(this);
129+ TaskController::singleton()->resume(appId());
130 }
131
132 void Application::respawn()
133 {
134 DLOG("Application::respawn (this=%p)", this);
135-
136- m_taskController->do_respawn(this);
137+ TaskController::singleton()->start(appId(), m_arguments);
138 }
139
140=== modified file 'src/modules/Unity/Application/application.h'
141--- src/modules/Unity/Application/application.h 2013-09-11 13:20:08 +0000
142+++ src/modules/Unity/Application/application.h 2013-09-27 15:13:07 +0000
143@@ -42,9 +42,8 @@
144 Q_PROPERTY(bool fullscreen READ fullscreen NOTIFY fullscreenChanged)
145
146 public:
147- Application(DesktopFileReader* desktopData, qint64 pid, Stage stage, State state,
148- const QStringList& arguments, TaskController* taskController,
149- QObject* parent = 0);
150+ Application(const QString &appId, State state, const QStringList &arguments, QObject *parent = 0);
151+ Application(DesktopFileReader *desktopFileReader, State state, const QStringList &arguments, QObject *parent = 0);
152 virtual ~Application();
153
154 // ApplicationInfoInterface
155@@ -56,6 +55,7 @@
156 State state() const override;
157 bool focused() const override;
158
159+ bool isValid() const;
160 QString desktopFile() const;
161 QString exec() const;
162 bool fullscreen() const;
163@@ -88,10 +88,8 @@
164 std::shared_ptr<mir::shell::ApplicationSession> m_session;
165 QString m_sessionName;
166 QStringList m_arguments;
167- TaskController* m_taskController;
168 QTimer* m_suspendTimer;
169
170- friend class TaskController;
171 friend class ApplicationManager;
172 friend class ApplicationListModel;
173 friend class MirSurfaceManager;
174
175=== modified file 'src/modules/Unity/Application/application_manager.cpp'
176--- src/modules/Unity/Application/application_manager.cpp 2013-09-12 17:27:47 +0000
177+++ src/modules/Unity/Application/application_manager.cpp 2013-09-27 15:13:07 +0000
178@@ -25,6 +25,7 @@
179 #include "shellserverconfiguration.h"
180 #include "sessionlistener.h"
181 #include "sessionauthorizer.h"
182+#include "taskcontroller.h"
183 #include "logging.h"
184
185 // mir
186@@ -33,13 +34,6 @@
187
188 // Qt
189 #include <QCoreApplication>
190-#include <QProcess>
191-
192-#define USE_UPSTART 0
193-
194-#if !USE_UPSTART
195- #include <pwd.h>
196-#endif
197
198 namespace msh = mir::shell;
199
200@@ -58,6 +52,7 @@
201 ApplicationManager::ApplicationManager(QObject *parent)
202 : ApplicationManagerInterface(parent)
203 , m_focusedApplication(NULL)
204+, m_taskController(TaskController::singleton())
205 {
206 DLOG("ApplicationManager::ApplicationManager (this=%p)", this);
207
208@@ -82,11 +77,12 @@
209 QObject::connect(m_mirServer->sessionAuthorizer(), &SessionAuthorizer::requestAuthorizationForSession,
210 this, &ApplicationManager::authorizeSession, Qt::BlockingQueuedConnection);
211
212- // we use random numbers, so seed the generator
213- qsrand(QTime::currentTime().msec());
214+ QObject::connect(m_taskController.data(), &TaskController::processStarted,
215+ this, &ApplicationManager::onProcessStarted);
216+ QObject::connect(m_taskController.data(), &TaskController::processStopped,
217+ this, &ApplicationManager::onProcessStopped);
218
219 m_dbusWindowStack = new DBusWindowStack(this);
220- m_taskController = new TaskController(this);
221 }
222
223 ApplicationManager::~ApplicationManager()
224@@ -152,10 +148,23 @@
225
226 bool ApplicationManager::focusApplication(const QString &appId)
227 {
228- DLOG("ApplicationManager::focusApplication (this=%p, appId=%s)", this, appId.toLatin1().data());
229+ DLOG("ApplicationManager::focusApplication (this=%p, appId=%s)", this, qPrintable(appId));
230 Application *application = findApplication(appId);
231
232- m_mirServer->the_session_manager()->set_focus_to(application->session());
233+ if (!application) {
234+ DLOG("No such running application '%s'", qPrintable(appId));
235+ return false;
236+ }
237+
238+ if (application->state() == Application::Stopped) {
239+ // Respawning this app, move to end of application list so onSessionStarting works ok
240+ // FIXME: this happens pretty late, shell could request respawn earlier
241+ application->setState(Application::Running);
242+ int from = m_applications.indexOf(application);
243+ move(from, m_applications.length()-1);
244+ } else {
245+ m_mirServer->the_session_manager()->set_focus_to(application->session());
246+ }
247
248 // FIXME(dandrader): lying here. The operation is async. So we will only know whether
249 // the focusing was successful once the server replies. Maybe the API in unity-api should
250@@ -170,40 +179,6 @@
251 m_mirServer->the_session_manager()->set_focus_to(NULL); //FIXME(greyback)
252 }
253
254-Application* ApplicationManager::respawnApplication(Application* application)
255-{
256- DLOG("ApplicationManager::respawnApplication(this=%p, application=%p)", this, application);
257-
258- // Start process - set correct environment
259- setenv("QT_QPA_PLATFORM", "ubuntumirclient", 1);
260-
261- bool result;
262- qint64 pid = 0;
263- struct passwd* passwd = getpwuid(getuid());
264- DLOG("current working directory: '%s' - args='%s'", passwd ? passwd->pw_dir : "/", application->m_arguments.join(' ').toLatin1().data());
265- QProcess builder;
266- builder.setProcessChannelMode(QProcess::ForwardedChannels);
267- QString exec(application->m_arguments[0]);
268- application->m_arguments.removeFirst();
269- result = builder.startDetached(exec, application->m_arguments, QString(passwd ? passwd->pw_dir : "/"), &pid);
270- application->m_arguments.prepend(exec);
271- DLOG_IF(result == false, "process failed to start");
272- if (result) {
273- // Set existing application's pid to new instance
274- application->setPid(pid);
275-
276- // Push to front
277- if (m_applications.count() > 1)
278- move(m_applications.indexOf(application), 0);
279-
280- DLOG("builder '%s' respawned with pid %lld", application->name().toLatin1().data(), pid);
281- return application;
282- } else {
283- DLOG("builder '%s' failed to respawn", application->name().toLatin1().data());
284- return NULL;
285- }
286-}
287-
288 Application* ApplicationManager::startApplication(const QString &appId,
289 const QStringList &arguments)
290 {
291@@ -211,102 +186,41 @@
292 }
293
294 Application *ApplicationManager::startApplication(const QString &appId, ExecFlags flags,
295- const QStringList &constArgs)
296-{
297- DLOG("ApplicationManager::startApplication (this=%p, appId=%s)", this, appId.toLatin1().data());
298-
299- QStringList arguments = constArgs;
300-
301- // Load desktop file.
302- DesktopFileReader* desktopData = new DesktopFileReader(appId);
303- if (!desktopData->loaded()) {
304- delete desktopData;
305- return NULL;
306- }
307-
308-#if USE_UPSTART
309- /* launch via upstart */
310- arguments.prepend("APP_ID=" + desktopData->appId());
311- arguments.prepend("application");
312-
313- // Start process.
314- //FIXME(greyback) use upstart via DBUS
315- QProcess process;
316- process.start("start", arguments);
317- //FIXME(greyback) this is blocking, instead listen for finished() signal
318- process.waitForFinished(1000);
319-
320- DLOG_IF(process.error(), "process 'start application' failed to start with error: %s", process.errorString().toLatin1().data());
321- if (!process.error()) {
322- // fetch pid
323- QString output = process.readAllStandardOutput();
324- int lastSpace = output.lastIndexOf(' ');
325- qint64 pid = output.remove(0, lastSpace).toInt();
326-#else
327- /* Launch manually */
328-
329- // Format arguments.
330- // FIXME(loicm) Special field codes are simply ignored for now.
331- // http://standards.freedesktop.org/desktop-entry-spec/latest/ar01s06.html
332- QStringList execArguments = desktopData->exec().split(" ", QString::SkipEmptyParts);
333- DASSERT(execArguments.size() > 0);
334- QString exec(execArguments[0]);
335- const int kSize = execArguments.size();
336- for (int i = kSize - 1; i > 0; i--) {
337- if ((execArguments[i].size() == 2) && (execArguments[i][0].toLatin1() == '%')) {
338- const char kChar = execArguments[i][1].toLatin1();
339- if (kChar == 'F' || kChar == 'u' || kChar == 'U' || kChar == 'd' || kChar == 'D'
340- || kChar == 'n' || kChar == 'N' || kChar == 'i' || kChar == 'c' || kChar == 'k'
341- || kChar == 'v' || kChar == 'm') {
342- continue;
343- }
344+ const QStringList &arguments)
345+{
346+ DLOG("ApplicationManager::startApplication (this=%p, appId=%s)", this, qPrintable(appId));
347+
348+ if (!m_taskController->start(appId, arguments)) {
349+ LOG("Asking Upstart to start application '%s' failed", qPrintable(appId));
350+ return nullptr;
351+ }
352+
353+ Application* application = new Application(appId, Application::Starting, arguments, this);
354+ if (!application->isValid()) {
355+ DLOG("Unable to instantiate application with appId '%s'", qPrintable(appId));
356+ return nullptr;
357+ }
358+
359+ // override stage if necessary
360+ if (application->stage() == Application::SideStage && flags.testFlag(ApplicationManager::ForceMainStage)) {
361+ application->setStage(Application::MainStage);
362+ }
363+
364+ add(application);
365+ return application;
366+}
367+
368+void ApplicationManager::onProcessStarted(const QString &appId)
369+{
370+ Application *application = findApplication(appId);
371+
372+ if (!application) { // if shell did not start this application, but upstart did
373+ application = new Application(appId, Application::Starting, QStringList(), this);
374+ if (!application->isValid()) {
375+ DLOG("Unable to instantiate application with appId '%s'", qPrintable(appId));
376+ return;
377 }
378- arguments.prepend(execArguments[i]);
379- }
380- arguments.append(QString("--desktop_file_hint=") + desktopData->file());
381- if (flags.testFlag(ApplicationManager::ForceMainStage))
382- arguments.append(QString("--stage_hint=main_stage"));
383- else if (desktopData->stageHint() == "SideStage")
384- arguments.append(QString("--stage_hint=side_stage"));
385-
386- // Start process - set correct environment
387- setenv("QT_QPA_PLATFORM", "ubuntumirclient", 1);
388-
389- bool result;
390- qint64 pid = 0;
391- QString path = "/";
392- // respect Path from .desktop file
393- if (desktopData->path() != "") {
394- path = desktopData->path();
395- } else {
396- struct passwd* passwd = getpwuid(getuid());
397- if (passwd)
398- path = passwd->pw_dir;
399- }
400- DLOG("current working directory: '%s'", path.toLatin1().data());
401- QProcess builder;
402- builder.setProcessChannelMode(QProcess::ForwardedChannels);
403- result = builder.startDetached(exec, arguments, path, &pid);
404-
405- DLOG_IF(result == false, "process failed to start");
406- if (result) {
407-#endif
408- DLOG("process started with pid %lld, adding '%s' to application lists", pid, desktopData->name().toLatin1().data());
409- arguments.prepend(exec);
410- Application* application = new Application(
411- desktopData, pid,
412- (flags.testFlag(ApplicationManager::ForceMainStage)
413- || desktopData->stageHint() != "SideStage")
414- ? Application::MainStage : Application::SideStage,
415- Application::Starting, //FIXME(greyback): assuming running immediately
416- arguments,
417- m_taskController
418- );
419-
420 add(application);
421- return application;
422- } else {
423- return NULL;
424 }
425 }
426
427@@ -316,35 +230,62 @@
428
429 Application *application = findApplication(appId);
430
431- if (application != NULL) {
432- if (application == m_focusedApplication) {
433- // TODO(greyback) What to do?? Focus next app, or unfocus everything??
434- m_focusedApplication = NULL;
435- Q_EMIT focusedApplicationIdChanged();
436- }
437- remove(application);
438- m_dbusWindowStack->WindowDestroyed(0, application->name());
439-
440- // Start process.
441- bool result;
442-
443-#if USE_UPSTART
444- result = QProcess::startDetached("stop", QStringList() << "application"
445- << ("APP_ID=" + application->appId()));
446- DLOG_IF(result == false, "process 'stop application' failed to execute");
447-#else
448- result = QProcess::startDetached("/bin/kill", QStringList() << "-9" << QString::number(application->m_pid));
449- DLOG_IF(result == false, "process '/bin/kill -9 %lld' failed to execute", application->m_pid);
450-#endif
451- LOG_IF(!result, "ApplicationManager: Unable to stop process '%s'",
452- application->name().toLatin1().constData());
453- delete application;
454- }
455+ if (!application) {
456+ DLOG("No such running application '%s'", qPrintable(appId));
457+ return false;
458+ }
459+
460+ if (application == m_focusedApplication) {
461+ // TODO(greyback) What to do?? Focus next app, or unfocus everything??
462+ m_focusedApplication = NULL;
463+ Q_EMIT focusedApplicationIdChanged();
464+ }
465+
466+ remove(application);
467+ m_dbusWindowStack->WindowDestroyed(0, application->name());
468+
469+ bool result = m_taskController->stop(application->appId());
470+
471+ LOG_IF(result == false, "FAILED to ask Upstart to stop application '%s'", qPrintable(application->appId()));
472+ delete application;
473
474 // FIXME(dandrader): lying here. The operation is async. So we will only know whether
475 // the focusing was successful once the server replies. Maybe the API in unity-api should
476 // reflect that?
477- return true;
478+ return result;
479+}
480+
481+void ApplicationManager::onProcessStopped(const QString &appId)
482+{
483+ Application *application = findApplication(appId);
484+
485+ // if shell did not stop the application, but upstart says it died, we assume the process has been
486+ // killed, so it can be respawned later. Only exception is if that application is focused or running
487+ // as then it most likely crashed. Update this logic when upstart gives some failure info.
488+ if (application) {
489+ bool removeApplication = false;
490+
491+ if (application == m_focusedApplication) {
492+ // Very bad case where focused application dies. Remove from list. Should give error message
493+ m_focusedApplication = nullptr;
494+ Q_EMIT focusedApplicationIdChanged();
495+ removeApplication = true;
496+ }
497+
498+ if (application->state() == Application::Running) {
499+ // Application probably crashed, else OOM killer struck. Either way state wasn't saved
500+ // so just remove application
501+ removeApplication = true;
502+ } else if (application->state() == Application::Suspended) {
503+ application->setState(Application::Stopped);
504+ }
505+
506+ if (removeApplication) {
507+ remove(application);
508+ m_dbusWindowStack->WindowDestroyed(0, application->name());
509+ delete application;
510+ }
511+ }
512 }
513
514 /************************************* Mir-side methods *************************************/
515@@ -354,12 +295,14 @@
516 authorized = false; //to be proven wrong
517
518 DLOG("ApplicationManager::authorizeSession (this=%p, pid=%lld)", this, pid);
519- Application* application = findApplicationWithPid(pid);
520- if (application) {
521- QString name = application->appId() + QString::number(qrand());
522- application->setSessionName(name);
523- authorized = true;
524- return;
525+
526+ for (Application *app : m_applications) {
527+ if (app->state() == Application::Starting
528+ && m_taskController->appIdHasProcessId(app->appId(), pid)) {
529+ app->setPid(pid);
530+ authorized = true;
531+ return;
532+ }
533 }
534
535 /*
536@@ -403,13 +346,13 @@
537
538 // some naughty applications use a script to launch the actual application. Check for the
539 // case where shell actually launched the script.
540- application = findApplication(desktopData->appId());
541+ Application *application = findApplication(desktopData->appId());
542 if (application && application->state() == Application::Starting) {
543 DLOG("Process with pid %lld appeared, attached to existing entry '%s' in application lists",
544 pid, application->appId().toLatin1().data());
545 delete desktopData;
546- QString name = application->appId() + QString::number(qrand());
547- application->setSessionName(name);
548+ application->setSessionName(application->appId());
549+ application->setPid(pid);
550 authorized = true;
551 return;
552 }
553@@ -428,7 +371,9 @@
554
555 QString argStr(command.data());
556 QStringList arguments(argStr.split(' '));
557- application = new Application(desktopData, pid, stage, Application::Starting, arguments, m_taskController);
558+ application = new Application(desktopData, Application::Starting, arguments, this);
559+ application->setPid(pid);
560+ application->setStage(stage);
561 add(application);
562 authorized = true;
563 }
564@@ -538,8 +483,11 @@
565 return nullptr;
566 }
567
568-Application* ApplicationManager::findApplicationWithPid(const int pid)
569+Application* ApplicationManager::findApplicationWithPid(const qint64 pid)
570 {
571+ if (pid <= 0)
572+ return nullptr;
573+
574 for (Application *app : m_applications) {
575 if (app->m_pid == pid) {
576 return app;
577
578=== modified file 'src/modules/Unity/Application/application_manager.h'
579--- src/modules/Unity/Application/application_manager.h 2013-09-11 13:20:08 +0000
580+++ src/modules/Unity/Application/application_manager.h 2013-09-27 15:13:07 +0000
581@@ -27,12 +27,13 @@
582 // Unity API
583 #include <unity/shell/application/ApplicationManagerInterface.h>
584
585-#include "taskcontroller.h"
586+// local
587+#include "application.h"
588
589-class Application;
590 class ShellServerConfiguration;
591 class DBusWindowStack;
592 class MirSurfaceManager;
593+class TaskController;
594
595 namespace mir {
596 namespace shell {
597@@ -82,7 +83,7 @@
598 Q_INVOKABLE void move(int from, int to);
599
600 const QList<Application*> &list() const { return m_applications; }
601- Application* findApplicationWithPid(const int pid);
602+ Application* findApplicationWithPid(const qint64 pid);
603
604 public Q_SLOTS:
605 void authorizeSession(const quint64 pid, bool &authorized);
606@@ -94,6 +95,9 @@
607
608 void onSessionCreatedSurface(mir::shell::ApplicationSession const*, std::shared_ptr<mir::shell::Surface> const&);
609
610+ void onProcessStarted(const QString& appId);
611+ void onProcessStopped(const QString& appId);
612+
613 Q_SIGNALS:
614 void focusRequested(FavoriteApplication favoriteApplication);
615
616@@ -101,7 +105,6 @@
617 void setFocused(Application *application);
618 void add(Application *application);
619 void remove(Application* application);
620- Application* respawnApplication(Application* application);
621 Application* findApplicationWithSession(const std::shared_ptr<mir::shell::Session> &session);
622 Application* findApplicationWithSession(const mir::shell::Session *session);
623 Application* findLastExecutedApplication();
624@@ -111,11 +114,10 @@
625 Application* m_focusedApplication; // remove as Mir has API for this
626 ShellServerConfiguration* m_mirServer;
627 DBusWindowStack* m_dbusWindowStack;
628- TaskController* m_taskController;
629+ QScopedPointer<TaskController> m_taskController;
630 static ApplicationManager* the_application_manager;
631
632 friend class DBusWindowStack;
633- friend class TaskController;
634 friend class MirSurfaceManager;
635 };
636
637
638=== modified file 'src/modules/Unity/Application/applicationscreenshotprovider.cpp'
639--- src/modules/Unity/Application/applicationscreenshotprovider.cpp 2013-09-24 18:48:11 +0000
640+++ src/modules/Unity/Application/applicationscreenshotprovider.cpp 2013-09-27 15:13:07 +0000
641@@ -67,11 +67,18 @@
642 return QImage();
643 }
644
645+ // TODO: if app not ready, return an app-provided splash image. If app has been stopped with saved state
646+ // return the screenshot that was saved to disk.
647 if (!app->session() || !app->session()->default_surface()) {
648 LOG("ApplicationScreenshotProvider - app session not found - taking screenshot too early");
649 return QImage();
650 }
651
652+ if (app->state() == Application::Stopped || app->state() == Application::Starting) {
653+ LOG("ApplicationScreenshotProvider - unable to take screenshot of stopped/not-ready applications");
654+ return QImage();
655+ }
656+
657 /* Workaround for bug https://bugs.launchpad.net/qtubuntu/+bug/1209216 - currently all qtubuntu
658 * based applications are allocated a fullscreen Mir surface, but draw in a geometry excluding
659 * the panel's rectangle. Mir snapshots thus have a white rectangle which the panel hides.
660
661=== modified file 'src/modules/Unity/Application/desktopfilereader.cpp'
662--- src/modules/Unity/Application/desktopfilereader.cpp 2013-09-17 17:52:54 +0000
663+++ src/modules/Unity/Application/desktopfilereader.cpp 2013-09-27 15:13:07 +0000
664@@ -98,7 +98,8 @@
665 const unsigned int kAllEntriesMask =
666 (1 << DesktopFileReader::kNameIndex) | (1 << DesktopFileReader::kCommentIndex)
667 | (1 << DesktopFileReader::kIconIndex) | (1 << DesktopFileReader::kExecIndex)
668- | (1 << DesktopFileReader::kPathIndex) | (1 << DesktopFileReader::kStageHintIndex); const unsigned int kMandatoryEntriesMask =
669+ | (1 << DesktopFileReader::kPathIndex) | (1 << DesktopFileReader::kStageHintIndex);
670+ const unsigned int kMandatoryEntriesMask =
671 (1 << DesktopFileReader::kNameIndex) | (1 << DesktopFileReader::kIconIndex)
672 | (1 << DesktopFileReader::kExecIndex);
673 const int kEntriesCount = ARRAY_SIZE(kEntryNames);
674
675=== modified file 'src/modules/Unity/Application/taskcontroller.cpp'
676--- src/modules/Unity/Application/taskcontroller.cpp 2013-08-27 13:31:20 +0000
677+++ src/modules/Unity/Application/taskcontroller.cpp 2013-09-27 15:13:07 +0000
678@@ -16,42 +16,116 @@
679 * Authored by: Ricardo Mendoza <ricardo.mendoza@canonical.com>
680 */
681
682-#include "logging.h"
683+// local
684 #include "taskcontroller.h"
685-#include "application_manager.h"
686+
687+// unity-mir
688+#include <logging.h>
689+
690+// Qt
691+#include <QStringList>
692+
693+// glib
694+#include <glib.h>
695+
696+// std
697 #include <signal.h>
698 #include <unistd.h>
699
700-TaskController::TaskController(ApplicationManager *parent) :
701+
702+TaskController* TaskController::m_theTaskController = nullptr;
703+
704+TaskController* TaskController::singleton()
705+{
706+ if (!m_theTaskController) {
707+ m_theTaskController = new TaskController();
708+ }
709+ return m_theTaskController;
710+}
711+
712+TaskController::TaskController(QObject *parent) :
713 QObject(parent)
714 {
715- DLOG("TaskController::TaskController (this=%p)", this);
716+ startCallback = [](const gchar * appId, gpointer userData) {
717+ Q_UNUSED(userData)
718+ Q_EMIT TaskController::singleton()->processStarted(QString(appId));
719+ };
720+
721+ stopCallback = [](const gchar * appId, gpointer userData) {
722+ Q_UNUSED(userData)
723+ Q_EMIT TaskController::singleton()->processStopped(QString(appId));
724+ };
725+
726+ upstart_app_launch_observer_add_app_start(startCallback, nullptr);
727+ upstart_app_launch_observer_add_app_stop(stopCallback, nullptr);
728 }
729
730 TaskController::~TaskController()
731 {
732- DLOG("TaskController::~TaskController (this=%p)", this);
733-}
734-
735-void TaskController::do_suspend(Application* application)
736-{
737- DLOG("TaskController::do_suspend (this=%p, application=%p)", this, application);
738- if (application->state() == Application::Suspended)
739- kill(application->pid(), SIGSTOP);
740-}
741-
742-void TaskController::do_resume(Application* application)
743-{
744- DLOG("TaskController::do_resume (this=%p, application=%p)", this, application);
745- if (application->state() == Application::Suspended)
746- kill(application->pid(), SIGCONT);
747-}
748-
749-void TaskController::do_respawn(Application* application)
750-{
751- DLOG("TaskController::do_respwn (this=%p, application=%p)", this, application);
752- if (application->state() == Application::Stopped) {
753- ApplicationManager *appMgr = static_cast<ApplicationManager*>(parent());
754- appMgr->respawnApplication(application);
755+ upstart_app_launch_observer_delete_app_start(startCallback, nullptr);
756+ upstart_app_launch_observer_delete_app_stop(stopCallback, nullptr);
757+}
758+
759+bool TaskController::start(const QString& appId, const QStringList& arguments)
760+{
761+ DLOG("TaskController::start appId='%s'", qPrintable(appId));
762+ gchar ** upstartArgs = nullptr;
763+ bool result = false;
764+
765+ // Convert arguments QStringList into format suitable for upstart-app-launch
766+ upstartArgs = g_new0(gchar *, arguments.length());
767+
768+ for (int i=0; i<arguments.length(); i++) {
769+ upstartArgs[i] = arguments.at(i).toLatin1().data();
770+ }
771+
772+ result = upstart_app_launch_start_application(appId.toLatin1().constData(),
773+ static_cast<const gchar * const *>(upstartArgs));
774+ g_free(upstartArgs);
775+
776+ DLOG_IF(!result, "TaskController::startApplication appId='%s' FAILED", qPrintable(appId));
777+ return result;
778+}
779+
780+bool TaskController::stop(const QString& appId)
781+{
782+ DLOG("TaskController::stop appId='%s'", qPrintable(appId));
783+ bool result = false;
784+
785+ result = upstart_app_launch_stop_application(appId.toLatin1().constData());
786+
787+ DLOG_IF(!result, "TaskController::stopApplication appId='%s' FAILED", qPrintable(appId));
788+ return result;
789+}
790+
791+bool TaskController::appIdHasProcessId(const QString& appId, const quint64 pid)
792+{
793+ DLOG("TaskController::isApplicationPid appId='%s', pid=%lld", qPrintable(appId), pid);
794+ return upstart_app_launch_pid_in_app_id(pid, appId.toLatin1().constData());
795+}
796+
797+bool TaskController::suspend(const QString& appId)
798+{
799+ DLOG("TaskController::suspend (this=%p, application=%p)", this, qPrintable(appId));
800+ pid_t pid = upstart_app_launch_get_primary_pid(appId.toLatin1().constData());
801+
802+ if (pid) {
803+ kill(pid, SIGSTOP);
804+ return true;
805+ } else {
806+ return false;
807+ }
808+}
809+
810+bool TaskController::resume(const QString& appId)
811+{
812+ DLOG("TaskController::resume (this=%p, application=%p)", this, qPrintable(appId));
813+ pid_t pid = upstart_app_launch_get_primary_pid(appId.toLatin1().constData());
814+
815+ if (pid) {
816+ kill(pid, SIGCONT);
817+ return true;
818+ } else {
819+ return false;
820 }
821 }
822
823=== modified file 'src/modules/Unity/Application/taskcontroller.h'
824--- src/modules/Unity/Application/taskcontroller.h 2013-08-27 13:31:20 +0000
825+++ src/modules/Unity/Application/taskcontroller.h 2013-09-27 15:13:07 +0000
826@@ -23,17 +23,35 @@
827
828 #include "application.h"
829
830-class ApplicationManager;
831+// upstart
832+extern "C" {
833+ #include "upstart-app-launch.h"
834+}
835+
836 class TaskController : public QObject
837 {
838 Q_OBJECT
839 public:
840- explicit TaskController(ApplicationManager* parent);
841+ static TaskController* singleton();
842 ~TaskController();
843
844- void do_resume(Application* application);
845- void do_suspend(Application* application);
846- void do_respawn(Application* application);
847+ bool start(const QString& appId, const QStringList& args);
848+ bool stop(const QString& appId);
849+
850+ bool suspend(const QString& appId);
851+ bool resume(const QString& appId);
852+
853+ bool appIdHasProcessId(const QString& appId, const quint64 pid);
854+
855+Q_SIGNALS:
856+ void processStarted(const QString& appId);
857+ void processStopped(const QString& appId);
858+
859+private:
860+ TaskController(QObject *parent = 0);
861+
862+ static TaskController* m_theTaskController;
863+ upstart_app_launch_app_observer_t startCallback, stopCallback;
864 };
865
866 #endif // TASKCONTROLLER_H

Subscribers

People subscribed via source and target branches