QtMir

CMakeLists.txt (+1/-1)
debian/changelog (+7/-0)
debian/control (+2/-2)
src/modules/Unity/Application/application.cpp (+2/-1)
src/modules/Unity/Application/application.h (+1/-3)
src/modules/Unity/Application/application_manager.cpp (+3/-14)
src/modules/Unity/Application/application_manager.h (+1/-11)
src/modules/Unity/Application/mirsurface.cpp (+26/-7)
src/modules/Unity/Application/mirsurface.h (+6/-3)
src/modules/Unity/Application/mirsurfaceinterface.h (+2/-0)
src/modules/Unity/Application/mirsurfaceitem.cpp (+6/-0)
src/modules/Unity/Application/mirsurfaceitem.h (+1/-0)
src/modules/Unity/Application/mirsurfacemanager.cpp (+5/-6)
src/modules/Unity/Application/mirsurfacemanager.h (+2/-2)
src/modules/Unity/Application/session.cpp (+3/-1)
src/platforms/mirserver/CMakeLists.txt (+1/-1)
src/platforms/mirserver/creationhints.cpp (+28/-5)
src/platforms/mirserver/creationhints.h (+11/-7)
src/platforms/mirserver/mirwindowmanager.cpp (+6/-7)
src/platforms/mirserver/sessionlistener.cpp (+5/-5)
src/platforms/mirserver/sessionlistener.h (+4/-4)
src/platforms/mirserver/surfaceobserver.cpp (+6/-2)
src/platforms/mirserver/surfaceobserver.h (+2/-1)
tests/framework/fake_mirsurface.h (+3/-0)
tests/framework/qtmir_test.cpp (+1/-1)
tests/mirserver/WindowManager/CMakeLists.txt (+2/-0)
tests/mirserver/WindowManager/window_manager.cpp (+26/-5)
tests/modules/ApplicationManager/application_manager_test.cpp (+29/-31)
tests/modules/SurfaceManager/mirsurface_test.cpp (+4/-4)

To merge this branch:

bzr merge lp://qastaging/~unity-team/qtmir/shell_chrome

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Gerry Boland (community)		2016-02-17	Approve on 2016-03-16
Unity8 CI Bot (community)	continuous-integration	2016-02-17	Needs Fixing on 2016-03-14
Lukáš Tinkl (community)			Needs Fixing on 2016-03-13
PS Jenkins bot (community)	continuous-integration	2016-02-17	Needs Fixing on 2016-02-17
Review via email: mp+286361@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2016-02-17.

Commit message

Add support for low shell chrome.

Description of the change

* Are there any related MPs required for this MP to build/function as expected? Please list.
https://code.launchpad.net/~nick-dedekind/unity-api/shell_chrome/+merge/286309
https://code.launchpad.net/~nick-dedekind/unity8/shell_chrome/+merge/286306
https://code.launchpad.net/~nick-dedekind/qtubuntu/shell_chrome/+merge/286308

* 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

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-17: Posted in a previous version of this proposal

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-17: Posted in a previous version of this proposal

FAILED: Continuous integration, rev:443
http://jenkins.qa.ubuntu.com/job/qtmir-ci/697/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/391/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/393/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/273/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/428/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/430/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/273/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-17:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-17:

FAILED: Continuous integration, rev:444
http://jenkins.qa.ubuntu.com/job/qtmir-ci/698/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/392/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/394/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/274/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/429/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/431/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/274/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-17:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-17:

FAILED: Continuous integration, rev:445
http://jenkins.qa.ubuntu.com/job/qtmir-ci/700/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/394/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/396/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/276/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/431/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/433/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/276/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Gerry Boland (gerboland) wrote on 2016-02-26:

As I said in the unity-api MP, I don't understand what you mean by "shell chrome" and suspect there might be better terminology.

Am happy to see the ExecFlags stuff going away.

+void MirSurfaceManager::onSurfaceModified(const std::shared_ptr<mir::scene::Surface> & surface,
nitpick - have the & beside the variable name.

In this method, I'm not convinced why mutex needed. This will be called on the GUI thread, since it a slot connected to via a queued connection.

You've a blank line after the if statement here too.

+++ src/modules/Unity/Application/mirsurfacemanager.h
+ const QVariant& value);
nitpick - & beside the var name please.

+++ src/platforms/mirserver/windowmanagerlistener.h
+++ src/platforms/mirserver/mirwindowmanager.h
In both files:
+ void surfaceMofidied(const std::shared_ptr<mir::scene::Surface>& surface,
Type. Plus & beside var name.

review: Needs Fixing

Revision history for this message

Gerry Boland (gerboland) wrote on 2016-03-08:

+ void surfaceMofidied(const std::shared_ptr<mir::scene::Surface>& surface,
typo

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2016-03-08:

> As I said in the unity-api MP, I don't understand what you mean by "shell
> chrome" and suspect there might be better terminology.
>
> Am happy to see the ExecFlags stuff going away.
>
>
> +void MirSurfaceManager::onSurfaceModified(const
> std::shared_ptr<mir::scene::Surface> & surface,
> nitpick - have the & beside the variable name.

done.

>
> In this method, I'm not convinced why mutex needed. This will be called on the
> GUI thread, since it a slot connected to via a queued connection.

I didn't add the mutex, but it's protecting the "surface to qml surface" hash in other methods, so just doing the same. Either keep, or remove all.

>
> You've a blank line after the if statement here too.
>

Done.

> +++ src/modules/Unity/Application/mirsurfacemanager.h
> + const QVariant& value);
> nitpick - & beside the var name please.
>

Done.

> +++ src/platforms/mirserver/windowmanagerlistener.h

Removed this class. Not used.

> +++ src/platforms/mirserver/mirwindowmanager.h
> In both files:
> + void surfaceMofidied(const std::shared_ptr<mir::scene::Surface>& surface,
> Type. Plus & beside var name.

Done.

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-08:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Gerry Boland (gerboland) wrote on 2016-03-09:

+void MirSurface::setShellChrome(Mir::ShellChrome shellChrome)
I thought this was a hint set by clients only.

review: Needs Information

Revision history for this message

Gerry Boland (gerboland) wrote on 2016-03-09:

+++ src/modules/Unity/Application/mirsurfaceitem.h
newline added, please remove.

+ if (it != m_mirSurfaceToQmlSurfaceHash.end()) {
+
+ qmlSurface = it.value();
kill the newline please

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2016-03-10:

> +void MirSurface::setShellChrome(Mir::ShellChrome shellChrome)
> I thought this was a hint set by clients only.

Which is why it's not in unity-api; but the value comes through the window manager surface modifications, so I set it from there.

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2016-03-10:

> > +void MirSurface::setShellChrome(Mir::ShellChrome shellChrome)
> > I thought this was a hint set by clients only.
>
> Which is why it's not in unity-api; but the value comes through the window
> manager surface modifications, so I set it from there.

It's updating the surface's internal state, not the server->client.

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-10:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Gerry Boland (gerboland) wrote on 2016-03-10:

It could be a private slot then. I only see it being called in mirsurface.cpp

Revision history for this message

Gerry Boland (gerboland) wrote on 2016-03-10:

Rest looks reasonable

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2016-03-11:

> > > +void MirSurface::setShellChrome(Mir::ShellChrome shellChrome)
> > > I thought this was a hint set by clients only.
> >
> > Which is why it's not in unity-api; but the value comes through the window
> > manager surface modifications, so I set it from there.
>
> It could be a private slot then. I only see it being called in mirsurface.cpp

Yeah, I changed it to private slot. It was previously being called from MirSurfaceManager directly, but now it's using the surface observer in the same way as the min/max size hints are coming through.

Revision history for this message

Gerry Boland (gerboland) wrote on 2016-03-11:

Cool, thanks for that

lp://qastaging/~unity-team/qtmir/shell_chrome updated on 2016-03-11