Merge lp://staging/~dandrader/unity8/miral into lp://staging/unity8

FAILED: Continuous integration, rev:2674
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2465/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3241/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3269
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3123/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3123/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3123/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3123/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3123/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3123/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3123/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3123/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3123/console

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

FAILED: Continuous integration, rev:2675
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2476/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3265/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3293
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3147/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3147/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3147/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3147/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3147/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3147/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3147/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3147/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3147/console

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

FAILED: Continuous integration, rev:2697
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2485/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3277/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3305
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3159/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3159/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3159/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3159/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3159/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3159/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3159/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3159/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3159/console

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

FAILED: Continuous integration, rev:2697
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2487/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3279/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3307
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3161/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3161
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3161/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3161
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3161/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3161/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3161
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3161/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3161
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3161/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3161/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3161
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3161/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3161
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3161/artifact/output/*zip*/output.zip

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

* See a bunch of inline comments

* I think we should not give up on the differntiation between claimFocus() and requestFocus(). One of them says the stage wants to actually focus something *now* while the other indicates that the app requested focus on its own (or perhaps u-a-l did). IMO claimFocus() should not be dropped and still do the raisId() call on the model. requestfocus() instead should end up in the shell, and if the shell is ok with doing so, it should call claimFocus() itself.

* IMO you're exaggerating with the debug messages. unity8.toplevelsurfacelist was already very annoying and this seems to have even more prints. Sure, you can argue that I could turn them off with the debug filter, which is what I do, but then I get none any more while the most important ones like raise() added() and removed() would still make sense.

This has been a code review for the code up until the mocks. Still needs review for tests/mocks and especially in depth testing.

Not yet sure if just an issue in the mocks or not, but this breaks:

- open an app
- click the minimize button in the window decoration
- click the app again in the launcher to restore it
- now click the minimize button again
=> button doesn't work any more

On 18/11/2016 12:29, Michael Zanetti wrote:
> * IMO you're exaggerating with the debug messages. unity8.toplevelsurfacelist was already very annoying and this seems to have even more prints. Sure, you can argue that I could turn them off with the debug filter, which is what I do, but then I get none any more while the most important ones like raise() added() and removed() would still make sense.

Reduced the logging output. Should be the way you prefer now.

- have 3 apps (a, b and c), make them overlap a bit for better visibility
- focus a by clicking it
- focus b by clicking it
- focus c by clicking it
- minimize c
=> a will be focused

expected: after minimizing c, b should be focused

On 18/11/2016 12:29, Michael Zanetti wrote:
> * I think we should not give up on the differntiation between claimFocus() and requestFocus(). One of them says the stage wants to actually focus something*now* while the other indicates that the app requested focus on its own (or perhaps u-a-l did). IMO claimFocus() should not be dropped and still do the raisId() call on the model. requestfocus() instead should end up in the shell, and if the shell is ok with doing so, it should call claimFocus() itself.

What you are explaining effectively is that: leave things as they are
currently with regards to focus. Which means focus lives in unity8 and
miral has no say over it.

The whole point of this miral effort is exactly to move such window
management decisions down to Mir, as dictated by our high-profile
stakeholder.

On the bright side, focus requests made by unity8 should naturally
always be sensible (since it knows the situation pretty well) and
therefore miral should always swiftly comply. So in terms of end
results, those requests will work just like imperative assignments.

Focus is a central piece of window management and if miral is to do
anything useful it has to have a say in it. And we cannot have two
entities making decisions on focus (unity8 and miral) as it will
inevitably be racy and conflictive.

As for the origin of the focus change request (shell vs. application),
it doesn't matter from miral's standpoint. It will comply to either as
long the change is valid and makes sense. Eg.: it should never allow a
window blocked by a child modal dialog to be focused. So it's mostly
sanity checking. I don't see miral ignoring requests on a whim. :)

Besides, having two separate code paths would make unity8 code more
complex. Eg: Having both code that sets surface state and code that
responds to surface state changes. "If surface state change was done in
response my your own set(), do nothing, else, ponder and respond."

FAILED: Continuous integration, rev:2698
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2512/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3317/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3345
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3197/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3197
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3197/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3197
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3197/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3197/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3197
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3197/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3197
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3197/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3197/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3197
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3197/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3197
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3197/artifact/output/*zip*/output.zip

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

On 18/11/2016 12:29, Michael Zanetti wrote:
>> +int TopLevelWindowModel::nextFreeId(int candidateId)
>> >+{
>> >+ if (candidateId > m_maxId) {
>> >+ return nextFreeId(1);
>> >+ } else {
>> >+ if (indexForId(candidateId) == -1) {
>> >+ // it's indeed free
>> >+ return candidateId;
>> >+ } else {
>> >+ return nextFreeId(candidateId + 1);
>> >+ }
>> >+ }
>> >+}
> In the very unlikely event that we'd run out of free ids, this would crash with a stack overflow. Think it makes sense to write this in a non-recursive manner and perhaps add some way to actually handle failures?
>
If we run out of free ids it's because there's already a bug somewhere.
There's no way there's a valid situation where we would have literally a
million windows lying around.

But a non-recursive version is indeed lighter-weight on resources if it
ever has to traverse a long stretch of taken ids. Done.

FAILED: Continuous integration, rev:2699
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2513/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3318/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3346
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3198/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3198
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3198/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3198
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3198/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3198/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3198
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3198/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3198
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3198/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3198/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3198
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3198/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3198
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3198/artifact/output/*zip*/output.zip

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

On 18/11/2016 12:29, Michael Zanetti wrote:
>> +QString TopLevelWindowModel::toString()
>> >+{
>> >+ QString str;
>> >+ for (int i = 0; i < m_windowModel.count(); ++i) {
>> >+ auto item = m_windowModel.at(i);
>> >+
>> >+ QString itemStr = QString("(index=%1,appId=%2,surface=0x%3,id=%4)")
>> >+ .arg(i)
>> >+ .arg(item.application->appId())
>> >+ .arg((qintptr)item.window->surface(), 0, 16)
>> >+ .arg(item.window->id());
> it would be slightly more performant if you'd use QString::arg(QString, QString, QSttring, QString) instead as it has to walk the string only once with that...
>
Done.

On 18/11/2016 12:29, Michael Zanetti wrote:
>> +public Q_SLOTS:
>> >+ /**
>> >+ * @brief Returns the surface at the given index
>> >+ *
>> >+ * It will be a nullptr if the application is still starting up and thus hasn't yet created
>> >+ * and drawn into a surface.
>> >+ *
>> >+ * Same as windowAt(index).surface()
>> >+ */
>> >+ unity::shell::application::MirSurfaceInterface *surfaceAt(int index) const;
>> >+
>> >+ /**
>> >+ * @brief Returns the window at the given index
>> >+ *
>> >+ * Will always be valid
>> >+ */
>> >+ Window *windowAt(int index) const;
>> >+
>> >+ /**
>> >+ * @brief Returns the application at the given index
>> >+ */
>> >+ unity::shell::application::ApplicationInfoInterface *applicationAt(int index) const;
>> >+
>> >+ /**
>> >+ * @brief Returns the unique id of the element at the given index
>> >+ */
>> >+ int idAt(int index) const;
>> >+
>> >+ /**
>> >+ * @brief Returns the index where the row with the given id is located
>> >+ *
>> >+ * Returns -1 if there's no row with the given id.
>> >+ */
>> >+ int indexForId(int id) const;
> slots with return value are weird. All the above should be public Q_INVOKABLE instead IMO.

Which uglifies the header file. Not great either.

Done.

>
>> >+
>> >+ /**
>> >+ * @brief Raises the row with the given id to the top of the window stack (index == count-1)
>> >+ */
>> >+ void raiseId(int id);
> this makes sense as slot indeed
>
But it's only a slot so it can be called from QML. Made it a Q_INVOKABLE
as well to match the others.

On 18/11/2016 12:29, Michael Zanetti wrote:
>> +Window::Window(int id)
>> >+ : QObject(nullptr)
>> >+ , m_id(id)
>> >+{
>> >+ DEBUG_MSG << "()";
>> >+ QQmlEngine::setObjectOwnership(this, QQmlEngine::CppOwnership);
> is there a reason why you can't use QObject's parent instead?

Laziness. Done.

>
> Would make sense in any case to parent the Window objects properly IMO
>
Yes, that's a nice safety net.

On 18/11/2016 12:29, Michael Zanetti wrote:
>> +QString Window::toString() const
>> >+{
>> >+ if (surface()) {
>> >+ return QString("Window[0x%1, id=%2, MirSurface[0x%3,\"%4\"]]")
>> >+ .arg((quintptr)this, 0, 16)
>> >+ .arg(id())
>> >+ .arg((quintptr)surface(), 0, 16)
>> >+ .arg(surface()->name());
> same here... if this is only ever executed in certain debug modes, ok with me, but if this is executed in production code too, please use the single .arg(..., ..., ...) call for better performance.
>
Done.

On 18/11/2016 12:29, Michael Zanetti wrote:
>> + Q_PROPERTY(unity::shell::application::MirSurfaceInterface* surface READ surface NOTIFY surfaceChanged)
> gah I despise this overly excessive namespacing...

Me too! Nested namespaces are an eyesore.

> IMO you could just do "using namespace unity::shell::application;" and be done with this, really... It's not like we'd ever have a conflicting MirSurfaceInterface class somewhere else...
>
Not in a header file though. I think that's bad form. A .cpp file
#including this header would inadvertently get the
unity::shell::application namespace included in their main one.

I'm also scared of doing namespace manipulations in Qt macros. That
might go wrong.

On 18/11/2016 12:29, Michael Zanetti wrote:
>> + qRegisterMetaType<Window*>("Window*");
> think it would make sense to qmlRegisterUncreatableType<Window>(...) too? Seems it would be useful API to be able to access Window from QML
>

Not sure. qmlRegisterUncreatableType() documentation says: "This is
useful where the type is only intended for providing attached properties
or enum values."
Not the case here.

On 18/11/2016 12:29, Michael Zanetti wrote:
>> - onMinimizeClicked: root.minimizeClicked();
>> >+ onMinimizeClicked: {
>> >+ if (applicationWindow.surface) {
>> >+ applicationWindow.surface.requestState(Mir.MinimizedState);
>> >+ }
>> >+ }
> this seems odd... why would you refactor DecoratedWindow to not directly operate on the data any more but instead pass everything outside with signals in a earlier branch, but now start introducing this somewhat messy behavior here again? IMO this should also be passed to outside to the Stage, and only the stage being in charge to actually execute things on the surfaces.
>
That's some old change that pre-dates that branch you mentioned.
Reverted.

On 18/11/2016 12:29, Michael Zanetti wrote:
>> - function focusNext() {
> same as I mentioned earlier. I'm still a bit scared of dropping control over this from the qml part, but lets see how it works... At least now that the model is part of unity8 again, it seems easier to change that back or modify in yet another way if we need to.
>
This is a perfect case where it makes sense to delegate to the C++
model. Choosing the next window do be focused when the current one is
minimized is a bunch of imperative code (and WM policy) that fits better
in C++ and not mixed with animation code in QML.

Particularly now that the model knows everything (who's focused and the
position and state of all surfaces).

FAILED: Continuous integration, rev:2700
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2514/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3319/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3347
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3199/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3199
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3199/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3199
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3199/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3199/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3199
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3199/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3199
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3199/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3199/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3199
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3199/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3199
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3199/artifact/output/*zip*/output.zip

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

On 18/11/2016 12:29, Michael Zanetti wrote:
>> if (!shown && priv.mainStageDelegate && !root.spreadShown) {
>> >- priv.mainStageDelegate.claimFocus();
>> >+ priv.mainStageDelegate.requestFocus();
> ok... given that requestFocus() effectively does claimFocus() now, this change totally makes sense... but we're losing the distinction between "the stage wants to focus something" and "an app requests focus by itsel (or something else than the stage)".
>
> Can you think of a way to keep that separation perhaps?
>
Heh, I actually think it is a good thing to handle them the same way (as
I said in some other comment).

On 18/11/2016 12:29, Michael Zanetti wrote:
>> + function updateQmlFocusFromMirSurfaceFocus() {
>> >+ if (model.window.focused) {
>> > claimFocus();
>> >+ priv.focusedAppDelegate = appDelegate;
>> >+ //} else if (priv.focusedAppDelegate === appDelegate && root.state != "spread") {
>> >+ // priv.focusedAppDelegate = null;
> leftover?
>
Yes, removed.

On 18/11/2016 12:29, Michael Zanetti wrote:
>> + onMaximizeHorizontallyClicked: {
>> >+ if (appDelegate.maximizedHorizontally)
>> >+ appDelegate.requestRestore();
>> >+ else
>> >+ appDelegate.requestMaximizeHorizontally();
>> >+ }
> {} missing
>
>> >+ onMaximizeVerticallyClicked: {
>> >+ if (appDelegate.maximizedVertically)
>> >+ appDelegate.requestRestore();
>> >+ else
>> >+ appDelegate.requestMaximizeVertically();
>> >+ }
> {} missing
>

Not sure if that's in our coding style but ok, added.

On 18/11/2016 14:16, Michael Zanetti wrote:
> Review: Needs Fixing
>
> Not yet sure if just an issue in the mocks or not, but this breaks:
>
> - open an app
> - click the minimize button in the window decoration
> - click the app again in the launcher to restore it
> - now click the minimize button again
> => button doesn't work any more

Bug in the mock. Works with qtmir.

FAILED: Continuous integration, rev:2705
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2515/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3320/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3348
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3200/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3200
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3200/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3200
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3200/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3200/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3200
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3200/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3200
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3200/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3200/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3200
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3200/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3200
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3200/artifact/output/*zip*/output.zip

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

FAILED: Continuous integration, rev:2706
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2516/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3321/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3349
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3201/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3201
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3201/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3201
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3201/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3201/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3201
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3201/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3201
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3201/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3201/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3201
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3201/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3201
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3201/artifact/output/*zip*/output.zip

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

>>>> - priv.mainStageDelegate.claimFocus();
>>>> >> >+ priv.mainStageDelegate.requestFocus();
>> > ok... given that requestFocus() effectively does claimFocus() now, this change totally makes sense... but we're losing the distinction between "the stage wants to focus something" and "an app requests focus by itsel (or something else than the stage)".
>> >
>> > Can you think of a way to keep that separation perhaps?
>> >
> Heh, I actually think it is a good thing to handle them the same way (as
> I said in some other comment).

How does focus stealing prevention come into play here? I believe the
plan was to only grant focus to apps that send along a ~MirCookie based
on a timestamp. I believe it's described here:

https://docs.google.com/document/d/1L85DdfDd3lDbvchYbgQ45C_lJ1IeTMG4uc7Nuq_XdAE/edit#heading=h.nlenkblnkaqq

On 18.11.2016 21:09, Daniel d'Andrada wrote:
> On 18/11/2016 12:29, Michael Zanetti wrote:
>>> + qRegisterMetaType<Window*>("Window*");
>> think it would make sense to qmlRegisterUncreatableType<Window>(...) too? Seems it would be useful API to be able to access Window from QML
>>
>
> Not sure. qmlRegisterUncreatableType() documentation says: "This is
> useful where the type is only intended for providing attached properties
> or enum values."
> Not the case here.

I'm more thinking about being able to access the Window type in QML
altogether. Not really about providing attached properties etc, but it
could make sense to do pass the Window object to somewhere in QML and
access its property there without having access to the model.

> On 18/11/2016 12:29, Michael Zanetti wrote:
> > * I think we should not give up on the differntiation between claimFocus()
> and requestFocus(). One of them says the stage wants to actually focus
> something*now* while the other indicates that the app requested focus on its
> own (or perhaps u-a-l did). IMO claimFocus() should not be dropped and still
> do the raisId() call on the model. requestfocus() instead should end up in the
> shell, and if the shell is ok with doing so, it should call claimFocus()
> itself.
>
> What you are explaining effectively is that: leave things as they are
> currently with regards to focus. Which means focus lives in unity8 and
> miral has no say over it.
>
> The whole point of this miral effort is exactly to move such window
> management decisions down to Mir, as dictated by our high-profile
> stakeholder.

There's a bunch of other stakeholders though that want to have it pixel perfect when it comes to animations and transitions. I am not saying miral should not have any say, but miral should send a requestFocus() to the shell and the shell will do it.
What I'm saying is that miral can tell unity to change the focus, and unity will follow that (unless it's in the middle of a transition or something). What you're proposing is that miral has all the say on when to change focus, and unity8 cannot do any course correction if the moment is bad.

> Focus is a central piece of window management and if miral is to do
> anything useful it has to have a say in it. And we cannot have two
> entities making decisions on focus (unity8 and miral) as it will
> inevitably be racy and conflictive.

which is exactly what we have now... unity8 thinks it is in charge, but miral can just jump in and trash it. I'm really just trying to avoid that.

>
> As for the origin of the focus change request (shell vs. application),
> it doesn't matter from miral's standpoint. It will comply to either as
> long the change is valid and makes sense. Eg.: it should never allow a
> window blocked by a child modal dialog to be focused. So it's mostly
> sanity checking. I don't see miral ignoring requests on a whim. :)
>
> Besides, having two separate code paths would make unity8 code more
> complex. Eg: Having both code that sets surface state and code that
> responds to surface state changes. "If surface state change was done in
> response my your own set(), do nothing, else, ponder and respond."

And I'm also not talking about 2 separate code paths. I'm saying the one code path should go through unity8 and not just manipulate the model at random.

Therefore I think only unity8 should manipulate the model, taking miral as consultant on how to manipulate it. This current approach is really the one that creates multiple entities fighting over the state of the model.

On 18/11/2016 14:21, Michael Zanetti wrote:
> Review: Needs Fixing
>
> - have 3 apps (a, b and c), make them overlap a bit for better visibility
> - focus a by clicking it
> - focus b by clicking it
> - focus c by clicking it
> - minimize c
> => a will be focused
>
> expected: after minimizing c, b should be focused

Can't reproduce in "make tryDesktopStage". Need more specific steps
(like exact test and apps launched).

On 21/11/2016 07:27, Michael Zanetti wrote:
>
> On 18.11.2016 21:09, Daniel d'Andrada wrote:
>> On 18/11/2016 12:29, Michael Zanetti wrote:
>>>> + qRegisterMetaType<Window*>("Window*");
>>> think it would make sense to qmlRegisterUncreatableType<Window>(...) too? Seems it would be useful API to be able to access Window from QML
>>>
>> Not sure. qmlRegisterUncreatableType() documentation says: "This is
>> useful where the type is only intended for providing attached properties
>> or enum values."
>> Not the case here.
> I'm more thinking about being able to access the Window type in QML
> altogether. Not really about providing attached properties etc, but it
> could make sense to do pass the Window object to somewhere in QML and
> access its property there without having access to the model.
>
>
This already happens in Stage.qml:

"""
x: model.window.position.x - decoratedWindow.contentX
y: model.window.position.y - decoratedWindow.contentY
"""

Looks like qmlRegisterUncreatableType() is not needed for that. If think
you only need it for Window.foo type of constructs.

On 21/11/2016 06:48, Michał Sawicz wrote:
>>>>> - priv.mainStageDelegate.claimFocus();
>>>>>>>> + priv.mainStageDelegate.requestFocus();
>>>> ok... given that requestFocus() effectively does claimFocus() now, this change totally makes sense... but we're losing the distinction between "the stage wants to focus something" and "an app requests focus by itsel (or something else than the stage)".
>>>>
>>>> Can you think of a way to keep that separation perhaps?
>>>>
>> Heh, I actually think it is a good thing to handle them the same way (as
>> I said in some other comment).
> How does focus stealing prevention come into play here? I believe the
> plan was to only grant focus to apps that send along a ~MirCookie based
> on a timestamp. I believe it's described here:
>
> https://docs.google.com/document/d/1L85DdfDd3lDbvchYbgQ45C_lJ1IeTMG4uc7Nuq_XdAE/edit#heading=h.nlenkblnkaqq
>
>
This happens at a level lower than unity8. All unity8 knows and cares
about is that some surface in the model got focus.

On 21.11.2016 12:09, Daniel d'Andrada wrote:
> On 21/11/2016 07:27, Michael Zanetti wrote:
>>
>> On 18.11.2016 21:09, Daniel d'Andrada wrote:
>>> On 18/11/2016 12:29, Michael Zanetti wrote:
>>>>> + qRegisterMetaType<Window*>("Window*");
>>>> think it would make sense to qmlRegisterUncreatableType<Window>(...) too? Seems it would be useful API to be able to access Window from QML
>>>>
>>> Not sure. qmlRegisterUncreatableType() documentation says: "This is
>>> useful where the type is only intended for providing attached properties
>>> or enum values."
>>> Not the case here.
>> I'm more thinking about being able to access the Window type in QML
>> altogether. Not really about providing attached properties etc, but it
>> could make sense to do pass the Window object to somewhere in QML and
>> access its property there without having access to the model.
>>
>>
> This already happens in Stage.qml:
>
> """
> x: model.window.position.x - decoratedWindow.contentX
> y: model.window.position.y - decoratedWindow.contentY
> """
>
> Looks like qmlRegisterUncreatableType() is not needed for that. If think
> you only need it for Window.foo type of constructs.

Hmm, ok... fine then.

FAILED: Continuous integration, rev:2707
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2517/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3322/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3350
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3202/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3202
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3202/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3202
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3202/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3202/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3202
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3202/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3202
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3202/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3202/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3202
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3202/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3202
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3202/artifact/output/*zip*/output.zip

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

> On 18/11/2016 14:21, Michael Zanetti wrote:
> > Review: Needs Fixing
> >
> > - have 3 apps (a, b and c), make them overlap a bit for better visibility
> > - focus a by clicking it
> > - focus b by clicking it
> > - focus c by clicking it
> > - minimize c
> > => a will be focused
> >
> > expected: after minimizing c, b should be focused
>
>
> Can't reproduce in "make tryDesktopStage". Need more specific steps
> (like exact test and apps launched).

Hmm... I can't reproduce it any more either after your latest changes.

> On 18/11/2016 14:16, Michael Zanetti wrote:
> > Review: Needs Fixing
> >
> > Not yet sure if just an issue in the mocks or not, but this breaks:
> >
> > - open an app
> > - click the minimize button in the window decoration
> > - click the app again in the launcher to restore it
> > - now click the minimize button again
> > => button doesn't work any more
>
> Bug in the mock. Works with qtmir.

This is really why I think we need to move all those models to unity8, put unity8 in control of them and mock things at a lower level. All our tests regarding window management are just useless right now... they can perfectly pass while the real thing can be broken really badly.

On 21/11/2016 10:08, Michael Zanetti wrote:
>> On 18/11/2016 14:16, Michael Zanetti wrote:
>>> Review: Needs Fixing
>>>
>>> Not yet sure if just an issue in the mocks or not, but this breaks:
>>>
>>> - open an app
>>> - click the minimize button in the window decoration
>>> - click the app again in the launcher to restore it
>>> - now click the minimize button again
>>> => button doesn't work any more
>> Bug in the mock. Works with qtmir.
> This is really why I think we need to move all those models to unity8, put unity8 in control of them and mock things at a lower level. All our tests regarding window management are just useless right now... they can perfectly pass while the real thing can be broken really badly.

Window management policy things like "if currently focused window gets
minimized, focus the most recently focused window that is not hidden or
minimized" will now belong to a lower level (miral) and as such they get
mock implementations in our tests.

Thus qml tests that strictly check window management policy will indeed
get redundant. Test of this nature now belong to miral's own test suite.

> On 21/11/2016 10:08, Michael Zanetti wrote:
> >> On 18/11/2016 14:16, Michael Zanetti wrote:
> >>> Review: Needs Fixing
> >>>
> >>> Not yet sure if just an issue in the mocks or not, but this breaks:
> >>>
> >>> - open an app
> >>> - click the minimize button in the window decoration
> >>> - click the app again in the launcher to restore it
> >>> - now click the minimize button again
> >>> => button doesn't work any more
> >> Bug in the mock. Works with qtmir.
> > This is really why I think we need to move all those models to unity8, put
> unity8 in control of them and mock things at a lower level. All our tests
> regarding window management are just useless right now... they can perfectly
> pass while the real thing can be broken really badly.
>
>
> Window management policy things like "if currently focused window gets
> minimized, focus the most recently focused window that is not hidden or
> minimized" will now belong to a lower level (miral) and as such they get
> mock implementations in our tests.
>
> Thus qml tests that strictly check window management policy will indeed
> get redundant. Test of this nature now belong to miral's own test suite.

Right, but we would still need tests that make sure that the QML part works together with the model as we expect it. And right now we don't have such tests... Anyhow, this is a different discussion than this particular merge proposal. It just once again popped up here. Also this isn't a new problem, we already had this since forever, but as long as we only had the one ApplicationManager model which would not even do much except moving things when unity8 said so, it was still easy enough to keep the mock close enough to the real thing. Now this seems to grow out of a manageable size.

> > On 18/11/2016 12:29, Michael Zanetti wrote:
> > > * I think we should not give up on the differntiation between claimFocus()
> > and requestFocus(). One of them says the stage wants to actually focus
> > something*now* while the other indicates that the app requested focus on
> its
> > own (or perhaps u-a-l did). IMO claimFocus() should not be dropped and still
> > do the raisId() call on the model. requestfocus() instead should end up in
> the
> > shell, and if the shell is ok with doing so, it should call claimFocus()
> > itself.
> >
> > What you are explaining effectively is that: leave things as they are
> > currently with regards to focus. Which means focus lives in unity8 and
> > miral has no say over it.
> >
> > The whole point of this miral effort is exactly to move such window
> > management decisions down to Mir, as dictated by our high-profile
> > stakeholder.
>
> There's a bunch of other stakeholders though that want to have it pixel
> perfect when it comes to animations and transitions. I am not saying miral
> should not have any say, but miral should send a requestFocus() to the shell
> and the shell will do it.
> What I'm saying is that miral can tell unity to change the focus, and unity
> will follow that (unless it's in the middle of a transition or something).
> What you're proposing is that miral has all the say on when to change focus,
> and unity8 cannot do any course correction if the moment is bad.
>
> > Focus is a central piece of window management and if miral is to do
> > anything useful it has to have a say in it. And we cannot have two
> > entities making decisions on focus (unity8 and miral) as it will
> > inevitably be racy and conflictive.
>
> which is exactly what we have now... unity8 thinks it is in charge, but miral
> can just jump in and trash it. I'm really just trying to avoid that.
>
> >
> > As for the origin of the focus change request (shell vs. application),
> > it doesn't matter from miral's standpoint. It will comply to either as
> > long the change is valid and makes sense. Eg.: it should never allow a
> > window blocked by a child modal dialog to be focused. So it's mostly
> > sanity checking. I don't see miral ignoring requests on a whim. :)
> >
> > Besides, having two separate code paths would make unity8 code more
> > complex. Eg: Having both code that sets surface state and code that
> > responds to surface state changes. "If surface state change was done in
> > response my your own set(), do nothing, else, ponder and respond."
>
> And I'm also not talking about 2 separate code paths. I'm saying the one code
> path should go through unity8 and not just manipulate the model at random.
>
> Therefore I think only unity8 should manipulate the model, taking miral as
> consultant on how to manipulate it. This current approach is really the one
> that creates multiple entities fighting over the state of the model.

This is a valid concern. The model must only have one owner. I think it safest that Unity8 has total ownership of the model and uses MirAL's recommendations only when it can.

MirAL has no concept of intermediary-states (like transitions/animations cause), so cannot advise Unit...

FAILED: Continuous integration, rev:2708
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2522/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3327/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3355
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3207/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3207/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3207/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3207/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3207/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3207/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3207/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3207/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3207/console

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

FAILED: Continuous integration, rev:2709
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2523/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3328/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3356
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3208/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3208/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3208/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3208/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3208/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3208/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3208/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3208/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3208/console

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

FAILED: Continuous integration, rev:2711
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2525/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3330/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3358
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3210/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3210/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3210/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3210/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3210/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3210/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3210/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3210/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3210/console

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

Thanks a lot for the activate() thing. Makes me feel much more comfy with the whole thing.

One minor thing (see inline comments) which I missed last time.

I'll give it another test with the latest changes, but it looks like we're getting close to an approval now.

On 23/11/2016 09:24, Michael Zanetti wrote:
>> readonly property Item clientAreaItem: applicationWindow
>> >
>> >+ readonly property real contentX: applicationWindow.x
>> >+ readonly property real contentY: applicationWindow.y
> afaict, you're only using that for knowing how much to add/remove for the decoration height. There is already a property "decorationHeight" you can use so this would be not needed, esp as applicationWindow.y will always be 0 at this point too.
>
Actually I can just use the existing clientAreaItem property right above
them. Fixed.

On 23/11/2016 09:24, Michael Zanetti wrote:
> Qt.point(appDelegate.requestedX + decoratedWindow.decorationHeight, appDelegate.requestedY)
>
> should be enough

I would rather not assume that decorations only affect the Y coordinate.

FAILED: Continuous integration, rev:2712
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2527/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3333/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3361
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3213/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3213/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3213/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3213/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3213/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3213/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3213/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3213/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3213/console

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

FAILED: Continuous integration, rev:2712
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2529/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3335/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3363
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3215/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3215/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3215/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3215/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3215/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3215/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3215/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3215/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3215/console

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

FAILED: Continuous integration, rev:2713
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2540/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3346/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3374
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3226/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3226/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3226/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3226/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3226/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3226/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3226/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3226/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3226/console

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

FAILED: Continuous integration, rev:2714
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2544/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3351/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3379
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3230/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3230/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3230/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3230/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3230/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3230/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3230/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3230/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3230/console

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

FAILED: Continuous integration, rev:2715
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2558/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3369/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3397
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3248/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3248/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3248/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3248/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3248
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3248/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3248
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3248/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3248/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3248/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3248/console

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

New instances of the same app do not cascade:

0. make tryShell
1. Launch dialer
2. Click the "+" button to launch a second instance
3. The new window doesn't cascade

If you add a trust prompt to a window and launch a second instance of the same app, the newly created window is not focused

Windows are not restored properly:

1. Maximize dash
2. Minimize it
3. Click the bfb icon
4. The dash window is restored to "normal" size, not to the maximized from step 1.

On 29/11/2016 15:35, Lukáš Tinkl wrote:
> Review: Needs Fixing
>
> New instances of the same app do not cascade:
>
> 0. make tryShell
> 1. Launch dialer
> 2. Click the "+" button to launch a second instance
> 3. The new window doesn't cascade

Same happens in trunk

Double click to maximize breaks:

1. Double click the dash decoration to maximize the window
2. Drag it down from the panel to restore it
3. Double click again to maximize -> no longer works

In tablet mode, if you put a fullscreen app into side stage (like camera), the main app (dash usually) has its decoration cut at the top.

> In tablet mode, if you put a fullscreen app into side stage (like camera), the
> main app (dash usually) has its decoration cut at the top.

This happens in trunk too

On 29/11/2016 15:37, Lukáš Tinkl wrote:
> Review: Needs Fixing
>
> If you add a trust prompt to a window and launch a second instance of the same app, the newly created window is not focused

Fixed. Bug in the Unity.Application mock.

On 29/11/2016 15:39, Lukáš Tinkl wrote:
> Review: Needs Fixing
>
> Windows are not restored properly:
>
> 1. Maximize dash
> 2. Minimize it
> 3. Click the bfb icon
> 4. The dash window is restored to "normal" size, not to the maximized from step 1.

That's due to the simplicity of the Unity.Application mock. It doesn't
have a full blown implementation of the window management bits/policy
that have been now moved to a lower level, behind
Unity.Application/qtmir API. This use case works with the real
Unity.Application/qtmir.

Unfortunately this means that right now we can no longer test everything
window management related in our qmltests. For some things untiy8 just
sits back and expects to get the correct changes to the model for free,
which it will then respond to by updating the qml items and animating
the qml scene (the view).

To be more specific, in response to the click on step 3 unity8 just
calls MirSurface->activate(). qtmir/miral will take care of noticing
that this surface is minimized and will restore it to its previous state
before finally focusing and raising it.

On 29/11/2016 15:50, Lukáš Tinkl wrote:
> Review: Needs Fixing
>
> Double click to maximize breaks:
>
> 1. Double click the dash decoration to maximize the window
> 2. Drag it down from the panel to restore it
> 3. Double click again to maximize -> no longer works

Fixed.