Merge lp:~gerboland/unity-mir/use-upstart-app-launch2 into lp:unity-mir
- use-upstart-app-launch2
- Merge into trunk
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 | ||||||||||||||||
Related bugs: |
|
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
PS Jenkins bot (ps-jenkins) wrote : | # |
- 97. By Gerry Boland
-
If process ends not by hand of shell, clean up after it properly
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:97
http://
Executed test runs:
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Daniel d'Andrada (dandrader) wrote : | # |
+ DLOG("Applicati
s/appId=%p/appId=%s
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:97
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 98. By Gerry Boland
-
s/appId=%p/appId=%s - well spotted Daniel
Daniel d'Andrada (dandrader) wrote : | # |
"""
|@@ -354,12 +271,14 @@ void ApplicationMana
| authorized = false; //to be proven wrong
|
| DLOG("Applicati
|- Application* application = findApplication
|- 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=%
Daniel d'Andrada (dandrader) wrote : | # |
"""
@@ -428,7 +347,10 @@ void ApplicationMana
QString argStr(
QStringList arguments(
- application = new Application(
+ application = new Application(
+ delete desktopData;
+ application-
+ application-
"""
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?
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:98
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 99. By Gerry Boland
-
Support respawning apps again
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.
Daniel d'Andrada (dandrader) wrote : | # |
Code-wise, my only complaint is the unnecessary recreation of the DesktopFileReader I explained above.
- 100. By Gerry Boland
-
Some handy extra debug info
- 101. By Gerry Boland
-
Fix for quick focus->
unfocus- >focus caused app to still suspend
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:99
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:101
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
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.
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
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:102
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 103. By Gerry Boland
-
Add todo for screenshot
MichaĆ Sawicz (saviq) wrote : | # |
Code looks good, didn't test though.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:103
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Daniel d'Andrada (dandrader) wrote : | # |
Looks good now.
Preview Diff
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 |
PASSED: Continuous integration, rev:96 jenkins. qa.ubuntu. com/job/ unity-mir- ci/74/ jenkins. qa.ubuntu. com/job/ unity-mir- saucy-amd64- ci/16 jenkins. qa.ubuntu. com/job/ unity-mir- saucy-armhf- ci/74 jenkins. qa.ubuntu. com/job/ unity-mir- saucy-armhf- ci/74/artifact/ work/output/ *zip*/output. zip jenkins. qa.ubuntu. com/job/ unity-mir- saucy-i386- ci/74
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild: s-jenkins: 8080/job/ unity-mir- ci/74/rebuild
http://