Merge lp:~unity-team/unity8/better-windowed-logic into lp:unity8

[Michał Sawicz]

Change the behavior alond with the switch in indicator-display

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

* https://code.launchpad.net/~lukas-kde/indicator-session/desktopModeSwitch/+merge/288413
* https://code.launchpad.net/~lukas-kde/unity8/sessionIndicatorForDevices/+merge/288466

 * Did you perform an exploratory manual test run of your code change and any related functionality?

yes. this has been running at MWC

 * Did you make sure that your branch does not contain spurious tags?

yes

 * 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?

yes

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:2149
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/686/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/904
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/920
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/920
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/918
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/918/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/918/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/918
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/918/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/918
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/918/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/918
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/918/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/918/console

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

review: Needs Fixing (continuous-integration)

[Unity8 CI Bot (unity8-ci-bot)]

PASSED: Continuous integration, rev:2150
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/688/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/392
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial,testname=qmluitests.sh/392
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=phone-armhf,release=vivid+overlay,testname=autopilot.sh/392
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/906
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/922
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/922
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/920
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/920/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/920
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/920/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/920
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/920/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/920
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/920/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/920
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/920/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/920
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/920/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Michael Terry (mterry)]

Inline comments. I'm especially concerned that this MP just directly modifies gsettings. Is that not a setting we expect the user to set and expect to be kept?

[Michael Terry (mterry)]

review: Needs Fixing

[Michael Zanetti (mzanetti)]

dropped some debug prints, but the overwriting of the gsetting is intentional

[lp:~unity-team/unity8/better-windowed-logic]

2151. By Michael Zanetti on 2016-03-15

drop some debug prints

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:2151
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/741/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/978
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/994
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/994
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/992/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/992/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/992/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/992/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/992/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/992/console

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

review: Needs Fixing (continuous-integration)

[Michael Terry (mterry)]

We talked on IRC about this. We think the logic needs to be more robust in future (e.g. pay attention to screen size changes too, although we get that for free right now because the phone becomes a pointer device when plugged into a new screen). etc. But that can come later.

review: Approve

=== removed file 'qml/Components/Unity8Settings.qml'
--- qml/Components/Unity8Settings.qml	2015-08-11 11:46:06 +0000
+++ qml/Components/Unity8Settings.qml	1970-01-01 00:00:00 +0000
@@ -1,50 +0,0 @@
-/*
- * Copyright (C) 2015 Canonical, Ltd.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; version 3.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-import QtQuick 2.4
-import GSettings 1.0
-
-QtObject {
-    id: root
-
-    property string usageMode: "Staged"
-
-    // FIXME: Works around a bug where if we change a Loader's source in response to a GSettings
-    // property change in the same event loop, the Loader's previously loaded component does not
-    // get destroyed and its bindings continue to operate!
-    //
-    // Shouldn't be needed after
-    // https://code.launchpad.net/~lukas-kde/gsettings-qt/queued-processing/+merge/259883 gets
-    // merged.
-    property var timer: Timer {
-        interval: 1
-        onTriggered: { root.usageMode = root.wrapped.usageMode; }
-    }
-    property var wrappedConnections: Connections {
-        target: root.wrapped
-        ignoreUnknownSignals: true // don't spam us
-        onUsageModeChanged: { root.timer.start(); }
-    }
-    property var wrapped: GSettings {
-        schema.id: "com.canonical.Unity8"
-        Component.onCompleted: {
-            // init the value. it's a dynamic prop, so we have to check first
-            if (root.usageMode) {
-                root.usageMode = root.wrapped.usageMode;
-            }
-        }
-    }
-}

=== modified file 'qml/OrientedShell.qml'
--- qml/OrientedShell.qml	2016-02-24 22:14:13 +0000
+++ qml/OrientedShell.qml	2016-03-15 14:26:25 +0000
@@ -58,7 +58,7 @@
         }
     }
         // to be overwritten by tests
-    property var unity8Settings: Unity8Settings {}
+    property var unity8Settings: GSettings { schema.id: "com.canonical.Unity8" }
     property var oskSettings: GSettings { schema.id: "com.canonical.keyboard.maliit" }
 
     property int physicalOrientation: Screen.orientation
@@ -68,11 +68,13 @@
     InputDeviceModel {
         id: miceModel
         deviceFilter: InputInfo.Mouse
+        property int oldCount: 0
     }
 
     InputDeviceModel {
         id: touchPadModel
         deviceFilter: InputInfo.TouchPad
+        property int oldCount: 0
     }
 
     InputDeviceModel {
@@ -87,6 +89,29 @@
         deviceFilter: InputInfo.TouchScreen
     }
 
+    readonly property int pointerInputDevices: miceModel.count + touchPadModel.count
+    onPointerInputDevicesChanged: {
+        console.log("Pointer input devices changed:", pointerInputDevices, "current mode:", root.unity8Settings.usageMode, "old device count", miceModel.oldCount + touchPadModel.oldCount)
+        if (root.unity8Settings.usageMode === "Windowed") {
+            if (pointerInputDevices === 0) {
+                // All pointer devices have been unplugged. Move to staged.
+                root.unity8Settings.usageMode = "Staged";
+            }
+        } else {
+            var longEdgeWidth = Math.max(root.width, root.height)
+            if (longEdgeWidth > units.gu(90)){
+                if (pointerInputDevices > 0 && pointerInputDevices > miceModel.oldCount + touchPadModel.oldCount) {
+                    root.unity8Settings.usageMode = "Windowed";
+                }
+            } else {
+                // Make sure we initialize to something sane
+                root.unity8Settings.usageMode = "Staged";
+            }
+        }
+        miceModel.oldCount = miceModel.count;
+        touchPadModel.oldCount = touchPadModel.count;
+    }
+
     /* FIXME: This exposes the NameRole as a work arround for lp:1542224.
      * When QInputInfo exposes NameRole to QML, this should be removed.
      */
@@ -222,30 +247,15 @@
         oskEnabled: (keyboardsModel.count === 0 && screens.count === 1) ||
                     forceOSKEnabled
 
-        // TODO: Factor in the connected input devices (eg: physical keyboard, mouse, touchscreen),
-        //       what's the output device (eg: big TV, desktop monitor, phone display), etc.
         usageScenario: {
             if (root.unity8Settings.usageMode === "Windowed") {
                 return "desktop";
-            } else if (root.unity8Settings.usageMode === "Staged") {
+            } else {
                 if (deviceConfiguration.category === "phone") {
                     return "phone";
                 } else {
                     return "tablet";
                 }
-            } else { // automatic
-                var longEdgeWidth = Math.max(root.width, root.height)
-                if (longEdgeWidth > units.gu(120)) {
-                    if (keyboardsModel.count + miceModel.count + touchPadModel.count > 0) {
-                        return "desktop";
-                    }
-                } else if (longEdgeWidth > units.gu(90)){
-                    if (miceModel.count + touchPadModel.count > 0) {
-                        return "desktop";
-                    }
-                }
-
-                return deviceConfiguration.category;
             }
         }
 

=== modified file 'tests/qmltests/tst_OrientedShell.qml'
--- tests/qmltests/tst_OrientedShell.qml	2016-02-15 19:47:34 +0000
+++ tests/qmltests/tst_OrientedShell.qml	2016-03-15 14:26:25 +0000
@@ -47,6 +47,9 @@
     QtObject {
         id: mockUnity8Settings
         property string usageMode: usageModeSelector.model[usageModeSelector.selectedIndex]
+        onUsageModeChanged: {
+            usageModeSelector.selectedIndex = usageModeSelector.model.indexOf(usageMode)
+        }
     }
 
     QtObject{
@@ -295,6 +298,9 @@
                 function selectStaged() {selectedIndex = 0;}
                 function selectWindowed() {selectedIndex = 1;}
                 function selectAutomatic() {selectedIndex = 2;}
+                onSelectedIndexChanged: {
+                    mockUnity8Settings.usageMode = usageModeSelector.model[usageModeSelector.selectedIndex]
+                }
             }
             MouseTouchEmulationCheckbox {
                 checked: true
@@ -1011,7 +1017,7 @@
                 { tag: "big screen, mouse", screenWidth: units.gu(200), mouse: true, kbd: false, expectedMode: "desktop", oskExpected: true },
                 { tag: "small screen, kbd", screenWidth: units.gu(50), mouse: false, kbd: true, expectedMode: "phone", oskExpected: false },
                 { tag: "medium screen, kbd", screenWidth: units.gu(100), mouse: false, kbd: true, expectedMode: "phone", oskExpected: false },
-                { tag: "big screen, kbd", screenWidth: units.gu(200), mouse: false, kbd: true, expectedMode: "desktop", oskExpected: false },
+                { tag: "big screen, kbd", screenWidth: units.gu(200), mouse: false, kbd: true, expectedMode: "phone", oskExpected: false },
                 { tag: "small screen, mouse & kbd", screenWidth: units.gu(50), mouse: true, kbd: true, expectedMode: "phone", oskExpected: false },
                 { tag: "medium screen, mouse & kbd", screenWidth: units.gu(100), mouse: true, kbd: true, expectedMode: "desktop", oskExpected: false },
                 { tag: "big screen, mouse & kbd", screenWidth: units.gu(200), mouse: true, kbd: true, expectedMode: "desktop", oskExpected: false },
@@ -1019,9 +1025,6 @@
         }
 
         function test_attachRemoveInputDevices(data) {
-            usageModeSelector.selectAutomatic();
-            tryCompare(mockUnity8Settings, "usageMode", "Automatic")
-
             loadShell("mako")
             var shell = findChild(orientedShell, "shell");
             var inputMethod = findChild(shell, "inputMethod");
@@ -1055,6 +1058,67 @@
             orientedShellLoader.width = oldWidth;
         }
 
+        function test_overrideStaged() {
+            loadShell("mako")
+
+            // make sure we're big enough so that the automatism starts working
+            var oldWidth = orientedShellLoader.width;
+            orientedShellLoader.width = units.gu(100);
+
+            // start off by plugging a mouse, we should switch to windowed
+            MockInputDeviceBackend.addMockDevice("/mouse0", InputInfo.Mouse);
+            tryCompare(shell, "usageScenario", "desktop");
+
+            // Use the toggle to go back to Staged
+            usageModeSelector.selectStaged();
+            tryCompare(shell, "usageScenario", "phone");
+
+            // attach a second mouse, we should switch again
+            MockInputDeviceBackend.addMockDevice("/mouse1", InputInfo.Mouse);
+            tryCompare(shell, "usageScenario", "desktop");
+
+            // Remove one mouse again, stay in windowed as there is another
+            MockInputDeviceBackend.removeDevice("/mouse1");
+            tryCompare(shell, "usageScenario", "desktop");
+
+            // use the toggle again
+            usageModeSelector.selectStaged();
+            tryCompare(shell, "usageScenario", "phone");
+
+            // Remove the other mouse again, stay in staged
+            MockInputDeviceBackend.removeDevice("/mouse0");
+            tryCompare(shell, "usageScenario", "phone");
+
+            // Restore width
+            orientedShellLoader.width = oldWidth;
+        }
+
+        function test_overrideWindowed() {
+            loadShell("mako")
+
+            // make sure we're big enough so that the automatism starts working
+            var oldWidth = orientedShellLoader.width;
+            orientedShellLoader.width = units.gu(100);
+
+            // No mouse attached... we should be in staged
+            tryCompare(shell, "usageScenario", "phone");
+
+            // use the toggle to go to windowed
+            usageModeSelector.selectWindowed();
+            tryCompare(shell, "usageScenario", "desktop");
+
+            // Connect a mouse, stay in windowed
+            MockInputDeviceBackend.addMockDevice("/mouse0", InputInfo.Mouse);
+            tryCompare(shell, "usageScenario", "desktop");
+
+            // Remove the mouse again, we should go to staged
+            MockInputDeviceBackend.removeDevice("/mouse0");
+            tryCompare(shell, "usageScenario", "phone");
+
+            // Restore width
+            orientedShellLoader.width = oldWidth;
+        }
+
         /*
             Regression test for https://bugs.launchpad.net/ubuntu/+source/unity8/+bug/1471609