Merge lp:~aacid/qtmir/remove_non_interface_things into lp:qtmir

Proposed by Albert Astals Cid
Status: Merged
Approved by: Gerry Boland
Approved revision: 619
Merged at revision: 627
Proposed branch: lp:~aacid/qtmir/remove_non_interface_things
Merge into: lp:qtmir
Diff against target: 123 lines (+1/-54)
2 files modified
src/modules/Unity/Application/application_manager.cpp (+0/-40)
src/modules/Unity/Application/application_manager.h (+1/-14)
To merge this branch: bzr merge lp:~aacid/qtmir/remove_non_interface_things
Reviewer Review Type Date Requested Status
Unity8 CI Bot (community) continuous-integration Approve
Gerry Boland (community) Approve
Review via email: mp+320600@code.launchpad.net

Commit message

Remove empty property and move invokable

They are not part of the interface and not used by unity8 either

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

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

To post a comment you must log in.
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:619
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/606/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4604
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4632
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4459
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4459/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4459
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4459/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4459
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4459/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4459
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4459/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4459
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4459/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4459
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4459/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/606/rebuild

review: Approve (continuous-integration)
Revision history for this message
Gerry Boland (gerboland) wrote :

- qtmir::Application* findApplicationWithPid(const pid_t pid) const;
run this by Dednick, my memory suggests DBus menus require the pid. Though probably not since it's not Q_INVOKABLE

Rest looks good!

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

> - qtmir::Application* findApplicationWithPid(const pid_t pid) const;
> run this by Dednick, my memory suggests DBus menus require the pid. Though
> probably not since it's not Q_INVOKABLE

We dont use it for menus. We only use the "Surface Menus" (which use surfaceId)
But we "may" need something in the future when we start using "Application Menus" (which use pid)

>
> Rest looks good!

Revision history for this message
Albert Astals Cid (aacid) wrote :

> > - qtmir::Application* findApplicationWithPid(const pid_t pid) const;
> > run this by Dednick, my memory suggests DBus menus require the pid. Though
> > probably not since it's not Q_INVOKABLE
>
> We dont use it for menus. We only use the "Surface Menus" (which use
> surfaceId)
> But we "may" need something in the future when we start using "Application
> Menus" (which use pid)

We can add it later if needed, in this branch i'm just removing functions we don't need to close the entry points for the branch that adds the mutex handling to ApplicationManager.

>
> >
> > Rest looks good!

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

Yep, happy to remove it not being used ATM

review: Approve
620. By Albert Astals Cid

Merge

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:620
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/642/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4749
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4777
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4599
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4599/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4599
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4599/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4599
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4599/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4599
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4599/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4599
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4599/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4599
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4599/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/642/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/modules/Unity/Application/application_manager.cpp'
--- src/modules/Unity/Application/application_manager.cpp 2017-02-21 18:46:30 +0000
+++ src/modules/Unity/Application/application_manager.cpp 2017-03-24 08:44:31 +0000
@@ -658,9 +658,6 @@
658 m_applications.append(application);658 m_applications.append(application);
659 endInsertRows();659 endInsertRows();
660 Q_EMIT countChanged();660 Q_EMIT countChanged();
661 if (m_applications.size() == 1) {
662 Q_EMIT emptyChanged();
663 }
664661
665 m_modelUnderChange = false;662 m_modelUnderChange = false;
666663
@@ -686,9 +683,6 @@
686 m_applications.removeAt(index);683 m_applications.removeAt(index);
687 endRemoveRows();684 endRemoveRows();
688 Q_EMIT countChanged();685 Q_EMIT countChanged();
689 if (index == 0) {
690 Q_EMIT emptyChanged();
691 }
692686
693 disconnect(application, &Application::fullscreenChanged, this, 0);687 disconnect(application, &Application::fullscreenChanged, this, 0);
694 disconnect(application, &Application::focusedChanged, this, 0);688 disconnect(application, &Application::focusedChanged, this, 0);
@@ -705,29 +699,6 @@
705 DEBUG_MSG << "(appId=" << application->appId() << ") - after " << toString();699 DEBUG_MSG << "(appId=" << application->appId() << ") - after " << toString();
706}700}
707701
708void ApplicationManager::move(int from, int to) {
709 qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::move - from=" << from << "to=" << to;
710 if (from == to) return;
711
712 Q_ASSERT(!m_modelUnderChange);
713 m_modelUnderChange = true;
714
715 if (from >= 0 && from < m_applications.size() && to >= 0 && to < m_applications.size()) {
716 QModelIndex parent;
717 /* When moving an item down, the destination index needs to be incremented
718 by one, as explained in the documentation:
719 http://qt-project.org/doc/qt-5.0/qtcore/qabstractitemmodel.html#beginMoveRows */
720
721 beginMoveRows(parent, from, from, parent, to + (to > from ? 1 : 0));
722 m_applications.move(from, to);
723 endMoveRows();
724 }
725
726 m_modelUnderChange = false;
727
728 qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::move after " << toString();
729}
730
731QModelIndex ApplicationManager::findIndex(Application* application)702QModelIndex ApplicationManager::findIndex(Application* application)
732{703{
733 for (int i = 0; i < m_applications.size(); ++i) {704 for (int i = 0; i < m_applications.size(); ++i) {
@@ -763,17 +734,6 @@
763 return nullptr;734 return nullptr;
764}735}
765736
766Application *ApplicationManager::findApplication(qtmir::MirSurfaceInterface* surface)
767{
768 Q_FOREACH (Application *app, m_applications) {
769 if (app->session() == surface->session()) {
770 return app;
771 }
772 }
773 return nullptr;
774}
775
776
777void ApplicationManager::onSessionStarting(SessionInterface *qmlSession)737void ApplicationManager::onSessionStarting(SessionInterface *qmlSession)
778{738{
779 Application* application = findApplicationWithSession(qmlSession->session());739 Application* application = findApplicationWithSession(qmlSession->session());
780740
=== modified file 'src/modules/Unity/Application/application_manager.h'
--- src/modules/Unity/Application/application_manager.h 2017-03-20 15:56:22 +0000
+++ src/modules/Unity/Application/application_manager.h 2017-03-24 08:44:31 +0000
@@ -61,9 +61,6 @@
61{61{
62 Q_OBJECT62 Q_OBJECT
6363
64 // TODO: Move to unity::shell::application::ApplicationManagerInterface
65 Q_PROPERTY(bool empty READ isEmpty NOTIFY emptyChanged)
66
67public:64public:
68 static ApplicationManager* create();65 static ApplicationManager* create();
69 static ApplicationManager* singleton();66 static ApplicationManager* singleton();
@@ -89,13 +86,6 @@
89 int rowCount(const QModelIndex & parent = QModelIndex()) const override;86 int rowCount(const QModelIndex & parent = QModelIndex()) const override;
90 QVariant data(const QModelIndex & index, int role) const override;87 QVariant data(const QModelIndex & index, int role) const override;
9188
92 Q_INVOKABLE void move(int from, int to);
93
94 bool isEmpty() const { return rowCount() == 0; }
95
96 const QList<Application*> &list() const { return m_applications; }
97 qtmir::Application* findApplicationWithPid(const pid_t pid) const;
98
99 SessionInterface *findSession(const mir::scene::Session* session) const override;89 SessionInterface *findSession(const mir::scene::Session* session) const override;
10090
101public Q_SLOTS:91public Q_SLOTS:
@@ -109,14 +99,12 @@
109 void onResumeRequested(const QString& appId);99 void onResumeRequested(const QString& appId);
110 void onSessionStarting(SessionInterface *session);100 void onSessionStarting(SessionInterface *session);
111101
112Q_SIGNALS:
113 void emptyChanged();
114
115private Q_SLOTS:102private Q_SLOTS:
116 void onAppDataChanged(const int role);103 void onAppDataChanged(const int role);
117 void onApplicationClosing(Application *application);104 void onApplicationClosing(Application *application);
118105
119private:106private:
107 qtmir::Application* findApplicationWithPid(const pid_t pid) const;
120 Application* findApplicationWithSession(const std::shared_ptr<mir::scene::Session> &session);108 Application* findApplicationWithSession(const std::shared_ptr<mir::scene::Session> &session);
121 void setFocused(Application *application);109 void setFocused(Application *application);
122 void add(Application *application);110 void add(Application *application);
@@ -128,7 +116,6 @@
128116
129 Application* findApplicationWithPromptSession(const mir::scene::PromptSession* promptSession);117 Application* findApplicationWithPromptSession(const mir::scene::PromptSession* promptSession);
130 Application *findClosingApplication(const QString &inputAppId) const;118 Application *findClosingApplication(const QString &inputAppId) const;
131 Application *findApplication(MirSurfaceInterface* surface);
132119
133 QList<Application*> m_applications;120 QList<Application*> m_applications;
134 DBusFocusInfo *m_dbusFocusInfo;121 DBusFocusInfo *m_dbusFocusInfo;

Subscribers

People subscribed via source and target branches