Merge lp:~paulliu/unity-mir/logout into lp:unity-mir

Proposed by Ying-Chun Liu
Status: Rejected
Rejected by: Daniel d'Andrada
Proposed branch: lp:~paulliu/unity-mir/logout
Merge into: lp:unity-mir
Diff against target: 243 lines (+163/-0)
5 files modified
src/modules/Unity/Application/CMakeLists.txt (+2/-0)
src/modules/Unity/Application/application_manager.cpp (+17/-0)
src/modules/Unity/Application/application_manager.h (+4/-0)
src/modules/Unity/Application/dbusunitysessionservice.cpp (+52/-0)
src/modules/Unity/Application/dbusunitysessionservice.h (+88/-0)
To merge this branch: bzr merge lp:~paulliu/unity-mir/logout
Reviewer Review Type Date Requested Status
Daniel d'Andrada (community) Disapprove
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+216336@code.launchpad.net

Commit message

Adding methods for logout

Description of the change

* Are there any related MPs required for this MP to build/function as expected?
No.

 * Did you perform an exploratory manual test run of your code change and any related functionality?
Yes.Run qdbus on phone and the dbus methods and signals are emitted.

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

 * If you changed the UI, has there been a design review?
N/A

To post a comment you must log in.
lp:~paulliu/unity-mir/logout updated
216. By Ying-Chun Liu on 2014-04-17

stop apps

217. By Ying-Chun Liu on 2014-04-17

Fix bug

PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:216
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~paulliu/unity-mir/logout/+merge/216336/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-mir-ci/326/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/189/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/190/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/189/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/326/rebuild

review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:217
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~paulliu/unity-mir/logout/+merge/216336/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-mir-ci/327/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/190
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/191
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/191/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/190

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/327/rebuild

review: Needs Fixing (continuous-integration)
lp:~paulliu/unity-mir/logout updated
218. By Ying-Chun Liu on 2014-04-17

Fix coding style

219. By Ying-Chun Liu on 2014-04-17

Merged upstream.
[ Ted Gould ]
Ensure the last item of the array is NULL (LP: #1304315)

PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:219
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~paulliu/unity-mir/logout/+merge/216336/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-mir-ci/328/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/191
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/192
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/192/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/191

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/328/rebuild

review: Needs Fixing (continuous-integration)
Daniel d'Andrada (dandrader) wrote :

113 + * Copyright (C) 2013 Canonical, Ltd.

s/2013/2014

Daniel d'Andrada (dandrader) wrote :

213 + Q_SCRIPTABLE void LogoutRequested(bool b);
214 + void LogoutReady();

Signals should not start with a capital letter.

Daniel d'Andrada (dandrader) wrote :

144 +DBusLogout::DBusLogout(ApplicationManager *parent) : QObject(parent)
145 +{
146 + QDBusConnection connection = QDBusConnection::sessionBus();
147 + m_applicationManager = parent;
148 +
149 + QObject::connect(this, &DBusLogout::LogoutReady,
150 + parent, &ApplicationManager::onLogoutReady);

DBusLogout gets an ApplicationManager pointer just to make a signal/slot connection between the two!?

That's bad design. You're unnecessarily adding a circular dependency between DBusLogout and ApplicationManager. Just make ApplicationManager code create such connection.

Actually I'm not even sure this should be in unity-mir in the first place. As there's nothing "mir" about it.

review: Needs Fixing
Daniel d'Andrada (dandrader) wrote :

> [...]
> Actually I'm not even sure this should be in unity-mir in the first place. As
> there's nothing "mir" about it.

Meaning that it should probably be in unity8

Michał Sawicz (saviq) wrote :

Re: Mir or not, there's a lot of non-mir in unity-mir already. But this here is about app management. We talked about this and decided this is as good a place as any.

lp:~paulliu/unity-mir/logout updated
220. By Ying-Chun Liu on 2014-04-18

Move signal connect to application manager.
Fix signal shouldn't start with capital letter.

Ying-Chun Liu (paulliu) wrote :

> [...]
> Actually I'm not even sure this should be in unity-mir in the first place. As
> there's nothing "mir" about it.

Currently unity7 provides the same dbus interface.
To provide the same for unity8, I think both unity8 and unity-mir have to be patched anyway.
We indeed need to decide which part to put into which branch.

1. For unity-mir (application manager), it needs to receive a signal to kill all the apps.
2. And we need a place to provide the dbus interface for Logout. (either unity8 or unity-mir)
3. And unity8 have to show the dialog and/or kill itself when received a signal.

Daniel d'Andrada (dandrader) wrote :

In src/modules/Unity/Application/dbuslogout.cpp

"""
140 +#include "application_manager.h"
"""

Should be removed

-----------------------------------------

In src/modules/Unity/Application/dbuslogout.h:

"""
208 +class ApplicationManager;
"""

Should be removed.

"""
227 +private:
228 + ApplicationManager *m_applicationManager;
"""

Should be removed.

------------------------------------------

Is this com.canonical.Unity.Session inteface documented elsewhere? If not I would like to see it documented in src/modules/Unity/Application/dbuslogout.h. It's not clear to me how it's supposed to be used.

"""
Q_SCRIPTABLE void logoutRequested(bool b);
"""

What's that boolean for?

What's the workflow here? RequestLogout() emits logoutRequested() and Logout() emits logoutReady(). Naming is a bit incosistent. What's the meaning and difference between RequestLogout() and Logout()? ApplicationManager is using only of the the two...

---------------------------------------------

ApplicationManager knowing about the logout concept feels out of place. As it name says, it should know only about managing apps. I believe it should be unity8 code doing the connection between the two.

AppliationManger::unityLogout()

Poorly named to say the least (check naming conventions for signals). I believe a clearer separation of tasks and responsibilities would be having something like the following in unity8 code:

DBusLogout {
    onLogoutRequested {
       ApplicationManager.stopAllApplications()
       Qt.quit();
    }
}

Which brings to mind the name of this class: DBusLogout. As it's implementing the com.canonical.Unity.Session service, I would clearer if it was named after it. After all, com.canonical.Unity.Session should be about more than just a Logout method, right? Thus:

UnitySessionDBusService {
    onLogoutRequested {
       ApplicationManager.stopAllApplications()
       Qt.quit();
    }
}

review: Needs Fixing
Michał Sawicz (saviq) wrote :

The problem with your last suggestion Daniel would be that we'd (have to) block the UI thread during stopAllApplications, as we want to shut them down gracefully and only then shut Unity/Mir down. That's why I think the dual-signal approach will serve us better, even if we think the responsibilities should shift between unity8 and unity-mir.

I'm not entirely sure we want to move it up to the shell, though. If we think unity-mir is meant to facilitate building shells, the more we move up there, the more the shell needs to do. I can be convinced, though.

lp:~paulliu/unity-mir/logout updated
221. By Ying-Chun Liu on 2014-04-23

minor changes

lp:~paulliu/unity-mir/logout updated
222. By Ying-Chun Liu on 2014-04-23

fix the boolean variable.

Ying-Chun Liu (paulliu) wrote :

After discussion with some people, I found that we actually don't have any specs or documents about it.
All of it is being defined "while coding" it.
Here's the interface in unity7.
http://pastebin.ubuntu.com/7231097/
It is in shutdown/SessionDBusManager.cpp in lp:unity.

So the thing is "indicator-session" will call the method "RequesetLogout". And the ApplicationManager will see if we have any important apps that inhibits the logout action ( while I don't think there's such app in Phone right now ) and then send out the signal "LogoutRequested" with a boolean to indicate if there's such apps.
And then Unity shell should receive that signal and display a dialog for the user to decide to logout or not.
If the user decide to logout, then unity shell call the method "Logout" and we have to shutdown all the app. After that, we signal the shell to shut itself.

I've fix some unclear naming. Not sure if we have a better name for logoutReady and unityLogout.
In unity7, When Logout is called, it forwards this method to org.gnome.SessionManager and some fallbacks

Daniel d'Andrada (dandrader) wrote :

> The problem with your last suggestion Daniel would be that we'd (have to)
> block the UI thread during stopAllApplications, as we want to shut them down
> gracefully and only then shut Unity/Mir down. That's why I think the dual-
> signal approach will serve us better, even if we think the responsibilities
> should shift between unity8 and unity-mir.

Ok.

> I'm not entirely sure we want to move it up to the shell, though. If we think
> unity-mir is meant to facilitate building shells, the more we move up there,
> the more the shell needs to do. I can be convinced, though.

hm!? Come on, unity-mir has "unity" in its name. So it's an integral part of the unity shell. But it seems to be treated like a unity8-cpp (where we stuff all the CPP code of unity8) whereas unity8 proper is like a unity8-qml (i.e., we want unity8 to have only QML code).

Also what's being added here is the com.canonical.Unity.Session service, which I suppose will have other methods and signals in the future that unity8 qml will be interested on and not only ApplicationManager.

Daniel d'Andrada (dandrader) wrote :

> So the thing is "indicator-session" will call the method "RequesetLogout". And
> the ApplicationManager will see if we have any important apps that inhibits
> the logout action ( while I don't think there's such app in Phone right now )
> and then send out the signal "LogoutRequested" with a boolean to indicate if
> there's such apps.
> And then Unity shell should receive that signal and display a dialog for the
> user to decide to logout or not.
> If the user decide to logout, then unity shell call the method "Logout" and we
> have to shutdown all the app. After that, we signal the shell to shut itself.

1- I asked you to document com.canonical.Unity.Session in its header, not in a merge request comment :D But thanks for the info. Now things are getting clearer.

2- So the D-Bus method Logout() from the com.canonical.Unity.Session service, which is provided by the unity8 process, is called by the unity8 process itself!?

3- What Unity shell is supposed to do with this "have_inhibitors" boolean? Display a different message in the logout dialog? I would refrain from adding such boolean until we actually have use for it. Development might change direction and this boolean could end up just as cruft.

lp:~paulliu/unity-mir/logout updated
223. By Ying-Chun Liu on 2014-04-24

Rename the class.
Add some doxygen.

Ying-Chun Liu (paulliu) wrote :

> 1- I asked you to document com.canonical.Unity.Session in its header, not in a
> merge request comment :D But thanks for the info. Now things are getting
> clearer.

Added.

>
> 2- So the D-Bus method Logout() from the com.canonical.Unity.Session service,
> which is provided by the unity8 process, is called by the unity8 process
> itself!?
>

Yes. It should be called by unity8 process.
But other apps if they want to do urgent logout, they can call this method too.

> 3- What Unity shell is supposed to do with this "have_inhibitors" boolean?
> Display a different message in the logout dialog? I would refrain from adding
> such boolean until we actually have use for it. Development might change
> direction and this boolean could end up just as cruft.

Yes, a different message in the logout dialog. I remember in unity7 it shows a dialog to tell users the logout is blocked and cancelled. I think we can keep this boolean for a while to be compatible to unity? But seems ok to me to remove it now as we don't have many historical burdens.

Daniel d'Andrada (dandrader) wrote :

Cosmetics: Please remove the trailing whitespace and the unneeded #include

=== modified file 'src/modules/Unity/Application/dbusunitysessionservice.cpp'
--- src/modules/Unity/Application/dbusunitysessionservice.cpp 2014-04-24 13:38:59 +0000
+++ src/modules/Unity/Application/dbusunitysessionservice.cpp 2014-04-24 17:29:55 +0000
@@ -23,16 +23,13 @@
 #include <QDBusMetaType>
 #include <QCoreApplication>

-// std
-#include <csignal>
-
 namespace unitymir
 {

 DBusUnitySessionService::DBusUnitySessionService(QObject *parent) : QObject(parent)
 {
     QDBusConnection connection = QDBusConnection::sessionBus();
-
+
     connection.registerService("com.canonical.Unity.Session");
     connection.registerObject("/com/canonical/Unity/Session", this, QDBusConnection::ExportAllSignals | QDBusConnection::ExportScriptableSlots | QDBusConnection::ExportScriptableInvokables);
 }

review: Needs Fixing
Daniel d'Andrada (dandrader) wrote :

Another cosmetic change: That line is waaaaay too long. Please break it.

=== modified file 'src/modules/Unity/Application/dbusunitysessionservice.cpp'
--- src/modules/Unity/Application/dbusunitysessionservice.cpp 2014-04-24 17:31:43 +0000
+++ src/modules/Unity/Application/dbusunitysessionservice.cpp 2014-04-24 17:32:22 +0000
@@ -31,7 +31,10 @@ DBusUnitySessionService::DBusUnitySessio
     QDBusConnection connection = QDBusConnection::sessionBus();

     connection.registerService("com.canonical.Unity.Session");
- connection.registerObject("/com/canonical/Unity/Session", this, QDBusConnection::ExportAllSignals | QDBusConnection::ExportScriptableSlots | QDBusConnection::ExportScriptableInvokables);
+ connection.registerObject("/com/canonical/Unity/Session", this,
+ QDBusConnection::ExportAllSignals
+ | QDBusConnection::ExportScriptableSlots
+ | QDBusConnection::ExportScriptableInvokables);
 }

 DBusUnitySessionService::~DBusUnitySessionService()

review: Needs Fixing
Daniel d'Andrada (dandrader) wrote :

Thanks for the documentation! Looking good!

Daniel d'Andrada (dandrader) wrote :

We also don't need those #includes

=== modified file 'src/modules/Unity/Application/dbusunitysessionservice.cpp'
--- src/modules/Unity/Application/dbusunitysessionservice.cpp 2014-04-24 17:34:26 +0000
+++ src/modules/Unity/Application/dbusunitysessionservice.cpp 2014-04-24 17:41:10 +0000
@@ -20,8 +20,6 @@

 // Qt
 #include <QDBusConnection>
-#include <QDBusMetaType>
-#include <QCoreApplication>

 namespace unitymir
 {

=== modified file 'src/modules/Unity/Application/dbusunitysessionservice.h'
--- src/modules/Unity/Application/dbusunitysessionservice.h 2014-04-24 13:38:59 +0000
+++ src/modules/Unity/Application/dbusunitysessionservice.h 2014-04-24 17:40:25 +0000
@@ -18,7 +18,6 @@
 #define DBUSUNITYSESSIONSERVICE_H_1397562297

 #include <QObject>
-#include <QDBusArgument>

 namespace unitymir
 {

review: Needs Fixing
Daniel d'Andrada (dandrader) wrote :

You're generating all that stuff but you don't seem to even use it. And I don't we needed it at all when using registerService() & registerObject(). It can just be removed.

=== modified file 'src/modules/Unity/Application/CMakeLists.txt'
--- src/modules/Unity/Application/CMakeLists.txt 2014-04-24 13:38:59 +0000
+++ src/modules/Unity/Application/CMakeLists.txt 2014-04-24 18:16:50 +0000
@@ -15,23 +15,9 @@ include_directories(
   ${CMAKE_SOURCE_DIR}/src/unity-mir
 )

-qt5_generate_dbus_interface(
- dbusunitysessionservice.h
- ${CMAKE_CURRENT_BINARY_DIR}/com.canonical.Unity8.Session.xml
- OPTIONS -a
-)
-
-qt5_add_dbus_adaptor(
- dbusunitysessionservice_SRCS
- ${CMAKE_CURRENT_BINARY_DIR}/com.canonical.Unity8.Session.xml
- dbusunitysessionservice.h unitymir::DBusUnitySessionService
- dbusunitysessionserviceadaptor DBusUnitySessionServiceAdaptor
-)
-
 add_library(
   unityapplicationplugin SHARED

- ${CMAKE_CURRENT_BINARY_DIR}/com.canonical.Unity8.Session.xml
   application_manager.cpp
   application.cpp
   desktopfilereader.cpp
@@ -50,7 +36,6 @@ add_library(
   ubuntukeyboardinfo.cpp
   dbusunitysessionservice.cpp
   dbusunitysessionservice.h
- ${dbusunitysessionservice_SRCS}

   upstart/applicationcontroller.h
   upstart/applicationcontroller.cpp

=== modified file 'src/modules/Unity/Application/dbusunitysessionservice.cpp'
--- src/modules/Unity/Application/dbusunitysessionservice.cpp 2014-04-24 17:45:48 +0000
+++ src/modules/Unity/Application/dbusunitysessionservice.cpp 2014-04-24 18:17:23 +0000
@@ -16,7 +16,6 @@

 // local
 #include "dbusunitysessionservice.h"
-#include "dbusunitysessionserviceadaptor.h"

 // Qt
 #include <QDBusConnection>

review: Needs Fixing
Daniel d'Andrada (dandrader) wrote :

If I understood it correctly the signals logoutRequested and logoutReady are used internally by unity8, right? So no need to expose them in D-Bus.

=== modified file 'src/modules/Unity/Application/dbusunitysessionservice.cpp'
--- src/modules/Unity/Application/dbusunitysessionservice.cpp 2014-04-24 18:27:05 +0000
+++ src/modules/Unity/Application/dbusunitysessionservice.cpp 2014-04-24 18:27:22 +0000
@@ -29,8 +29,7 @@ DBusUnitySessionService::DBusUnitySessio

     connection.registerService("com.canonical.Unity.Session");
     connection.registerObject("/com/canonical/Unity/Session", this,
- QDBusConnection::ExportAllSignals
- | QDBusConnection::ExportScriptableSlots
+ QDBusConnection::ExportScriptableSlots
             | QDBusConnection::ExportScriptableInvokables);
 }

=== modified file 'src/modules/Unity/Application/dbusunitysessionservice.h'
--- src/modules/Unity/Application/dbusunitysessionservice.h 2014-04-24 17:45:48 +0000
+++ src/modules/Unity/Application/dbusunitysessionservice.h 2014-04-24 18:28:40 +0000
@@ -47,7 +47,7 @@ Q_SIGNALS:
      * @param have_inhibitors if there are any special running applications
      * which inhibit the logout.
      */
- Q_SCRIPTABLE void logoutRequested(bool have_inhibitors);
+ void logoutRequested(bool have_inhibitors);

     /**
      * logoutReady signal

review: Needs Fixing
Daniel d'Andrada (dandrader) wrote :

If I understood it correctly, Q_SCRIPTABLE is used to mark the signals you want exposed on D-Bus. In which case, it makes no sense here.

=== modified file 'src/modules/Unity/Application/application_manager.h'
--- src/modules/Unity/Application/application_manager.h 2014-04-24 13:38:59 +0000
+++ src/modules/Unity/Application/application_manager.h 2014-04-24 18:45:21 +0000
@@ -132,7 +132,7 @@ public Q_SLOTS:

 Q_SIGNALS:
     void focusRequested(const QString &appId);
- Q_SCRIPTABLE void unityLogout();
+ void unityLogout();

 private Q_SLOTS:
     void screenshotUpdated();

review: Needs Fixing
lp:~paulliu/unity-mir/logout updated
224. By Ying-Chun Liu on 2014-04-25

Fix signals/slots publicity.
Remove qdbus tools generating stuff.

Daniel d'Andrada (dandrader) wrote :

97 +++ src/modules/Unity/Application/dbusunitysessionservice.cpp 2014-04-25 13:37:06 +0000
98 @@ -0,0 +1,52 @@
99 +/* -*- mode: c++; indent-tabs-mode: nil; tab-width: 4 -*- */
100 +/*

I don't think unity-mir or unity8 has this stuff in its coding style. I believe you copy-and-pasted that from some other project?

---------------

Looking at it now, there's a clear discrepancy between what the DBusUnitySessionService API says and what it actually does (or is used for).

"""
    /**
     * logoutReady signal
     *
     * This signal is emitted when all the apps are closed. And the system
     * is safe to logout.
     */
    void logoutReady();
"""

No, this signal is emitted when Logout() is called. That's all. The one that gets emitted when all the apps are closed is ApplicationManager::unityLogout.

Naming it logoutOrdered or logoutCommanded would be more appropriate. Specially considering the existing logoutRequested.

"""
    /**
     * Logout the system.
     *
     * This method directly logout the system without user's confirmation.
     * Ordinary applications should avoid calling this method. Please call
     * RequestLogout() to ask the user to decide logout or not.
     * This method will stop all the running applications and then signal
     * logoutReady when all the apps stopped.
     */
    Q_SCRIPTABLE void Logout();
"""

So the last sentence is wrong (about logoutReady)

Just remembered that as you want to change ApplicationManager, you should have the changes also in its interface: unity::shell::application::ApplicationManagerInterface from unity-api. And don't forget to update its mock in unity8.

I still don't like ApplicationManager::unityLogout. To me it looks more like a new ApplicationManager state.

ApplicationManagerInterface::suspended could be expanded from a boolean to a proper state enum comprising the following values:
 - "running" or "normal" (the equivalent of the current suspended==false behaviour)
 - suspended
 - stopped (all running apps will be stopped/closed and no new apps will be allowed to run).

Because, during logout, you could order AppMan to close all apps but afterwards, still during the logout process, a new app launch might be triggered (you never know) and AppMan should not allow it to run. That's why I think the logout thing, from AppMan's point of view, looks more like a state than a simple, stateless, command.

And I still think DBusUnitySessionService should be moved to unity8.

Would like to hear from mzanetti and Saviq.

review: Needs Fixing
Daniel d'Andrada (dandrader) wrote :

Results from the discussion with Saviq and mzanetti in #ubuntu-unity:

- This is a "quick & dirty" interim implementation to get things going as currently there's not grand plan or architecture for the logout story. This story need more thought.
- We don't want to pollute or complicate AppMan API with "quick & dirty" changes.

Thus:
Leave ApplicationManager as it is. You can achieve the same result with the existing API. Just move the loop to close all apps to unity8. And move DBusUnitySessionService along with it. Check unity8/plugins sub dir (LightDm and Utils also register dbus services).

review: Disapprove

Unmerged revisions

224. By Ying-Chun Liu on 2014-04-25

Fix signals/slots publicity.
Remove qdbus tools generating stuff.

223. By Ying-Chun Liu on 2014-04-24

Rename the class.
Add some doxygen.

222. By Ying-Chun Liu on 2014-04-23

fix the boolean variable.

221. By Ying-Chun Liu on 2014-04-23

minor changes

220. By Ying-Chun Liu on 2014-04-18

Move signal connect to application manager.
Fix signal shouldn't start with capital letter.

219. By Ying-Chun Liu on 2014-04-17

Merged upstream.
[ Ted Gould ]
Ensure the last item of the array is NULL (LP: #1304315)

218. By Ying-Chun Liu on 2014-04-17

Fix coding style

217. By Ying-Chun Liu on 2014-04-17

Fix bug

216. By Ying-Chun Liu on 2014-04-17

stop apps

215. By Ying-Chun Liu on 2014-04-17

fix bugs

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/modules/Unity/Application/CMakeLists.txt'
2--- src/modules/Unity/Application/CMakeLists.txt 2014-03-28 16:41:02 +0000
3+++ src/modules/Unity/Application/CMakeLists.txt 2014-04-25 13:37:06 +0000
4@@ -34,6 +34,8 @@
5 proc_info.cpp
6 shellinputarea.cpp
7 ubuntukeyboardinfo.cpp
8+ dbusunitysessionservice.cpp
9+ dbusunitysessionservice.h
10
11 upstart/applicationcontroller.h
12 upstart/applicationcontroller.cpp
13
14=== modified file 'src/modules/Unity/Application/application_manager.cpp'
15--- src/modules/Unity/Application/application_manager.cpp 2014-04-14 12:56:28 +0000
16+++ src/modules/Unity/Application/application_manager.cpp 2014-04-25 13:37:06 +0000
17@@ -20,6 +20,7 @@
18 #include "application.h"
19 #include "desktopfilereader.h"
20 #include "dbuswindowstack.h"
21+#include "dbusunitysessionservice.h"
22 #include "taskcontroller.h"
23 #include "upstart/applicationcontroller.h"
24
25@@ -183,6 +184,7 @@
26 , m_ssApplicationToBeFocused(nullptr)
27 , m_lifecycleExceptions(QStringList() << "com.ubuntu.music")
28 , m_focusController(controller)
29+ , m_dbusUnitySessionService(new DBusUnitySessionService(this))
30 , m_dbusWindowStack(new DBusWindowStack(this))
31 , m_taskController(taskController)
32 , m_desktopFileReaderFactory(desktopFileReaderFactory)
33@@ -207,6 +209,11 @@
34 int densityPixelPx = qFloor( (float)m_gridUnitPx / 8 );
35
36 m_panelHeight = 3 * m_gridUnitPx + 2 * densityPixelPx;
37+
38+ QObject::connect(m_dbusUnitySessionService,
39+ &unitymir::DBusUnitySessionService::logoutReady,
40+ this, &ApplicationManager::onLogoutReady);
41+
42 }
43
44 ApplicationManager::~ApplicationManager()
45@@ -612,6 +619,16 @@
46 }
47 }
48
49+void ApplicationManager::onLogoutReady()
50+{
51+ // shut all of the apps down here.
52+ for (Application *app : m_applications) {
53+ stopApplication(app->appId());
54+ }
55+
56+ Q_EMIT unityLogout();
57+}
58+
59 void ApplicationManager::screenshotUpdated()
60 {
61 if (sender()) {
62
63=== modified file 'src/modules/Unity/Application/application_manager.h'
64--- src/modules/Unity/Application/application_manager.h 2014-03-12 13:47:18 +0000
65+++ src/modules/Unity/Application/application_manager.h 2014-04-25 13:37:06 +0000
66@@ -49,6 +49,7 @@
67
68 namespace unitymir
69 {
70+class DBusUnitySessionService;
71 class DBusWindowStack;
72 class MirSurfaceManager;
73 class TaskController;
74@@ -127,9 +128,11 @@
75 void onProcessStopped(const QString& appId, const bool unexpected);
76 void onFocusRequested(const QString& appId);
77 void onResumeRequested(const QString& appId);
78+ void onLogoutReady();
79
80 Q_SIGNALS:
81 void focusRequested(const QString &appId);
82+ void unityLogout();
83
84 private Q_SLOTS:
85 void screenshotUpdated();
86@@ -154,6 +157,7 @@
87 Application* m_ssApplicationToBeFocused; // placeholder store for async focusing
88 QStringList m_lifecycleExceptions;
89 std::shared_ptr<mir::shell::FocusController> m_focusController;
90+ DBusUnitySessionService* m_dbusUnitySessionService;
91 DBusWindowStack* m_dbusWindowStack;
92 QSharedPointer<TaskController> m_taskController;
93 QSharedPointer<DesktopFileReader::Factory> m_desktopFileReaderFactory;
94
95=== added file 'src/modules/Unity/Application/dbusunitysessionservice.cpp'
96--- src/modules/Unity/Application/dbusunitysessionservice.cpp 1970-01-01 00:00:00 +0000
97+++ src/modules/Unity/Application/dbusunitysessionservice.cpp 2014-04-25 13:37:06 +0000
98@@ -0,0 +1,52 @@
99+/* -*- mode: c++; indent-tabs-mode: nil; tab-width: 4 -*- */
100+/*
101+ * Copyright (C) 2014 Canonical, Ltd.
102+ *
103+ * This program is free software: you can redistribute it and/or modify it under
104+ * the terms of the GNU Lesser General Public License version 3, as published by
105+ * the Free Software Foundation.
106+ *
107+ * This program is distributed in the hope that it will be useful, but WITHOUT
108+ * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
109+ * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
110+ * Lesser General Public License for more details.
111+ *
112+ * You should have received a copy of the GNU Lesser General Public License
113+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
114+ */
115+
116+// local
117+#include "dbusunitysessionservice.h"
118+
119+// Qt
120+#include <QDBusConnection>
121+
122+namespace unitymir
123+{
124+
125+DBusUnitySessionService::DBusUnitySessionService(QObject *parent) : QObject(parent)
126+{
127+ QDBusConnection connection = QDBusConnection::sessionBus();
128+
129+ connection.registerService("com.canonical.Unity.Session");
130+ connection.registerObject("/com/canonical/Unity/Session", this,
131+ QDBusConnection::ExportScriptableSignals
132+ | QDBusConnection::ExportScriptableSlots
133+ | QDBusConnection::ExportScriptableInvokables);
134+}
135+
136+DBusUnitySessionService::~DBusUnitySessionService()
137+{
138+}
139+
140+void DBusUnitySessionService::Logout()
141+{
142+ Q_EMIT logoutReady();
143+}
144+
145+void DBusUnitySessionService::RequestLogout()
146+{
147+ Q_EMIT logoutRequested(false);
148+}
149+
150+} // namespace unitymir
151
152=== added file 'src/modules/Unity/Application/dbusunitysessionservice.h'
153--- src/modules/Unity/Application/dbusunitysessionservice.h 1970-01-01 00:00:00 +0000
154+++ src/modules/Unity/Application/dbusunitysessionservice.h 2014-04-25 13:37:06 +0000
155@@ -0,0 +1,88 @@
156+/* -*- mode: c++; indent-tabs-mode: nil; tab-width: 4 -*- */
157+/*
158+ * Copyright (C) 2014 Canonical, Ltd.
159+ *
160+ * This program is free software: you can redistribute it and/or modify it under
161+ * the terms of the GNU Lesser General Public License version 3, as published by
162+ * the Free Software Foundation.
163+ *
164+ * This program is distributed in the hope that it will be useful, but WITHOUT
165+ * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
166+ * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
167+ * Lesser General Public License for more details.
168+ *
169+ * You should have received a copy of the GNU Lesser General Public License
170+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
171+ */
172+
173+#ifndef DBUSUNITYSESSIONSERVICE_H_1397562297
174+#define DBUSUNITYSESSIONSERVICE_H_1397562297
175+
176+#include <QObject>
177+
178+namespace unitymir
179+{
180+
181+/**
182+ * DBusUnitySessionService provides com.canonical.Unity.Session dbus
183+ * interface.
184+ *
185+ * com.canonical.Unity.Session interface provides public methods
186+ * and signals to handle Logout/Reboot/Shutdown.
187+ */
188+class DBusUnitySessionService : public QObject
189+{
190+ Q_OBJECT
191+ Q_CLASSINFO("D-Bus Interface", "com.canonical.Unity.Session")
192+
193+public:
194+ explicit DBusUnitySessionService(QObject* parent);
195+ ~DBusUnitySessionService();
196+
197+Q_SIGNALS:
198+ /**
199+ * logoutRequested signal
200+ *
201+ * This signal is emitted when some applications request the system to
202+ * logout.
203+ * @param have_inhibitors if there are any special running applications
204+ * which inhibit the logout.
205+ */
206+ void logoutRequested(bool have_inhibitors);
207+
208+ /**
209+ * logoutReady signal
210+ *
211+ * This signal is emitted when all the apps are closed. And the system
212+ * is safe to logout.
213+ */
214+ void logoutReady();
215+
216+public Q_SLOTS:
217+ /**
218+ * Logout the system.
219+ *
220+ * This method directly logout the system without user's confirmation.
221+ * Ordinary applications should avoid calling this method. Please call
222+ * RequestLogout() to ask the user to decide logout or not.
223+ * This method will stop all the running applications and then signal
224+ * logoutReady when all the apps stopped.
225+ */
226+ Q_SCRIPTABLE void Logout();
227+
228+ /**
229+ * Issue a logout request.
230+ *
231+ * This method emit the logoutRequested signal to the shell with a boolean
232+ * which indicates if there's any inhibitors. The shell should receive
233+ * this signal and display a dialog to ask the user to confirm the logout
234+ * action. If confirmed, shell can call Logout() method to kill the apps
235+ * and then logout
236+ */
237+ Q_SCRIPTABLE void RequestLogout();
238+
239+};
240+
241+} // namespace unitymir
242+
243+#endif // DBUSUNITYSESSIONSERVICE_H_1397562297

Subscribers

People subscribed via source and target branches