Merge lp:~mterry/unity8/tutorial-reappears into lp:unity8

[Michael Terry]

Make sure the edge tutorial is destroyed when we receive a call during the wizard.

Make sure the edge tutorial is destroyed when we receive a call during the wizard.

Now... This bug doesn't make sense to me. What's happening is that the Tutorial.qml Loader is correctly marked as "active=false". But the loaded object is not actually released/destroyed.

I couldn't figure why this was happening. And I couldn't reproduce it via qmltests. In all cases I saw, the object was correctly destroyed. But I could reproduce the bug on the device. And adding log printfs indicated that yup, the object was staying around after Loader.active was false.

But on a hunch, cleaning up the object hierarchy helped. Before, each tutorial page would have a visual parent of "shell.stages" but a QtObject parent of the tutorial loader item. This is a leftover from before the latest tutorial redesign. Now visual parents *shouldn't* affect object lifecycle, I think...? But it seems like it does.

Because when I made the tutorial page's visual parents be the same as their QtObject parents (and just moved the whole Tutorial object underneath shell.stages), it worked fine. The tutorial objects were destroyed when the loader went inactive.

Do any Qml-heads know why this happens? I would guess some refcounting somewhere, but I don't know why I couldn't reproduce in qmltests.

I added some sanity checks at the end of the current test for wizard/tutorial early exit, which don't hurt. But even without my qml changes, they still pass (because I could never reproduce this bug in laboratory conditions).

== Checklist ==

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

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

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

 * If you changed the UI, has there been a design review?
 NA

[Michael Terry (mterry)]

(Oh yeah, and I cleared out a couple unused vestigial properties: stages and overlay)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1699
http://jenkins.qa.ubuntu.com/job/unity8-ci/5490/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/1993
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/654
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/655
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/655
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/1755
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1991
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1991/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19200

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

review: Needs Fixing (continuous-integration)

[Albert Astals Cid (aacid)]

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

 * Did CI run pass?
It passed up to the expected passing standards

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

review: Approve

[lp:~mterry/unity8/tutorial-reappears]

1700. By Michael Terry on 2015-04-03

Merge in lp:~mterry/unity8/skip-spread-tutorial-on-desktop

Unmerged revisions

=== modified file 'qml/Shell.qml'
--- qml/Shell.qml	2015-03-10 14:25:12 +0000
+++ qml/Shell.qml	2015-04-03 14:23:20 +0000
@@ -283,6 +283,31 @@
                 value: shell.background
             }
         }
+
+        Tutorial {
+            id: tutorial
+            objectName: "tutorial"
+            anchors.fill: parent
+            active: AccountsService.demoEdges
+            paused: LightDM.Greeter.active
+            launcher: launcher
+            panel: panel
+            edgeSize: shell.edgeSize
+
+            // EdgeDragAreas don't work with mice.  So to avoid trapping the user,
+            // we'll tell the tutorial to avoid using them on the Desktop.  The
+            // Desktop doesn't use the same spread design anyway.  The tutorial is
+            // all a bit of a placeholder on non-phone form factors right now.
+            // When the design team gives us more guidance, we can do something
+            // more clever here.
+            // TODO: use DeviceConfiguration instead of checking source
+            useEdgeDragArea: applicationsDisplayLoader.source != Qt.resolvedUrl("Stages/DesktopStage.qml")
+
+            onFinished: {
+                AccountsService.demoEdges = false;
+                active = false; // for immediate response / if AS is having problems
+            }
+        }
     }
 
     InputMethod {
@@ -580,23 +605,6 @@
         }
     }
 
-    Tutorial {
-        id: tutorial
-        objectName: "tutorial"
-        active: AccountsService.demoEdges
-        paused: LightDM.Greeter.active
-        launcher: launcher
-        panel: panel
-        stages: stages
-        overlay: overlay
-        edgeSize: shell.edgeSize
-
-        onFinished: {
-            AccountsService.demoEdges = false;
-            active = false; // for immediate response / if AS is having problems
-        }
-    }
-
     Connections {
         target: SessionBroadcast
         onShowHome: showHome()

=== modified file 'qml/Tutorial/Tutorial.qml'
--- qml/Tutorial/Tutorial.qml	2015-01-06 19:50:14 +0000
+++ qml/Tutorial/Tutorial.qml	2015-04-03 14:23:20 +0000
@@ -23,11 +23,10 @@
     property alias active: loader.active
     property bool paused
     property real edgeSize
+    property bool useEdgeDragArea
 
     property Item launcher
     property Item panel
-    property Item stages
-    property Item overlay
 
     readonly property bool launcherEnabled: loader.item ? loader.item.launcherEnabled : true
     readonly property bool spreadEnabled: loader.item ? loader.item.spreadEnabled : true
@@ -62,6 +61,12 @@
 
         Binding {
             target: loader.item
+            property: "useEdgeDragArea"
+            value: root.useEdgeDragArea
+        }
+
+        Binding {
+            target: loader.item
             property: "launcher"
             value: root.launcher
         }
@@ -72,18 +77,6 @@
             value: root.panel
         }
 
-        Binding {
-            target: loader.item
-            property: "stages"
-            value: root.stages
-        }
-
-        Binding {
-            target: loader.item
-            property: "overlay"
-            value: root.overlay
-        }
-
         Connections {
             target: loader.item
             onFinished: root.finished()

=== modified file 'qml/Tutorial/TutorialContent.qml'
--- qml/Tutorial/TutorialContent.qml	2015-01-06 19:13:58 +0000
+++ qml/Tutorial/TutorialContent.qml	2015-04-03 14:23:20 +0000
@@ -22,8 +22,6 @@
 
     property Item launcher
     property Item panel
-    property Item stages
-    property Item overlay
 
     readonly property bool launcherEnabled: !running ||
                                             (!paused && tutorialLeft.shown)
@@ -34,6 +32,7 @@
 
     property bool paused: false
     property real edgeSize
+    property bool useEdgeDragArea
 
     signal finished()
 
@@ -66,7 +65,6 @@
     TutorialLeft {
         id: tutorialLeft
         objectName: "tutorialLeft"
-        parent: root.stages
         anchors.fill: parent
         launcher: root.launcher
         paused: !shown || root.paused
@@ -77,21 +75,26 @@
     TutorialLeftFinish {
         id: tutorialLeftFinish
         objectName: "tutorialLeftFinish"
-        parent: root.stages
         anchors.fill: parent
         textXOffset: root.launcher.panelWidth
         paused: !shown || root.paused
+        text: root.useEdgeDragArea ? i18n.tr("Tap here to continue.") : i18n.tr("Click here to finish.")
 
         onFinished: {
             root.launcher.hide();
-            tutorialRight.show();
+            if (root.useEdgeDragArea) {
+                tutorialRight.show();
+            } else {
+                // Last two screens use EdgeDragArea (right edge spread and
+                // bottom edge).  So just end tutorial here.
+                root.finish();
+            }
         }
     }
 
     TutorialRight {
         id: tutorialRight
         objectName: "tutorialRight"
-        parent: root.stages
         anchors.fill: parent
         edgeSize: root.edgeSize
         panel: root.panel
@@ -103,7 +106,6 @@
     TutorialBottom {
         id: tutorialBottom
         objectName: "tutorialBottom"
-        parent: root.stages
         anchors.fill: parent
         edgeSize: root.edgeSize
         paused: !shown || root.paused
@@ -114,7 +116,6 @@
     TutorialBottomFinish {
         id: tutorialBottomFinish
         objectName: "tutorialBottomFinish"
-        parent: root.stages
         anchors.fill: parent
         backgroundFadesOut: true
         paused: !shown || root.paused

=== modified file 'tests/mocks/GSettings.1.0/fake_gsettings.cpp'
--- tests/mocks/GSettings.1.0/fake_gsettings.cpp	2013-08-15 08:11:04 +0000
+++ tests/mocks/GSettings.1.0/fake_gsettings.cpp	2015-04-03 14:23:20 +0000
@@ -20,7 +20,9 @@
 
 GSettingsControllerQml* GSettingsControllerQml::s_controllerInstance = 0;
 
-GSettingsControllerQml::GSettingsControllerQml() {
+GSettingsControllerQml::GSettingsControllerQml()
+    : m_usageMode("Staged")
+{
 }
 
 GSettingsControllerQml::~GSettingsControllerQml() {
@@ -34,17 +36,29 @@
     return s_controllerInstance;
 }
 
-void GSettingsControllerQml::registerSettingsObject(GSettingsQml *obj) {
-    m_registeredGSettings.append(obj);
-}
-
-void GSettingsControllerQml::unRegisterSettingsObject(GSettingsQml *obj) {
-    m_registeredGSettings.removeOne(obj);
-}
-
-void GSettingsControllerQml::setPictureUri(const QString &str) {
-    Q_FOREACH (GSettingsQml *obj, m_registeredGSettings) {
-        obj->setPictureUri(str);
+QString GSettingsControllerQml::pictureUri() const
+{
+    return m_pictureUri;
+}
+
+void GSettingsControllerQml::setPictureUri(const QString &str)
+{
+    if (str != m_pictureUri) {
+        m_pictureUri = str;
+        Q_EMIT pictureUriChanged(m_pictureUri);
+    }
+}
+
+QString GSettingsControllerQml::usageMode() const
+{
+    return m_usageMode;
+}
+
+void GSettingsControllerQml::setUsageMode(const QString &usageMode)
+{
+    if (usageMode != m_usageMode) {
+        m_usageMode = usageMode;
+        Q_EMIT usageModeChanged(m_usageMode);
     }
 }
 
@@ -56,7 +70,7 @@
 }
 
 void GSettingsSchemaQml::setId(const QByteArray &id) {
-    if (m_id.isEmpty()) {
+    if (!m_id.isEmpty()) {
         qWarning("GSettings.schema.id may only be set on construction");
         return;
     }
@@ -69,7 +83,7 @@
 }
 
 void GSettingsSchemaQml::setPath(const QByteArray &path) {
-    if (m_path.isEmpty()) {
+    if (!m_path.isEmpty()) {
         qWarning("GSettings.schema.path may only be set on construction");
         return;
     }
@@ -77,26 +91,48 @@
     m_path = path;
 }
 
-GSettingsQml::GSettingsQml(QObject *parent): QObject(parent) {
+GSettingsQml::GSettingsQml(QObject *parent)
+    : QObject(parent)
+{
     m_schema = new GSettingsSchemaQml(this);
-    GSettingsControllerQml::instance()->registerSettingsObject(this);
-}
-
-GSettingsQml::~GSettingsQml() {
-    GSettingsControllerQml::instance()->unRegisterSettingsObject(this);
+    connect(GSettingsControllerQml::instance(), SIGNAL(pictureUriChanged(const QString &)),
+            this, SIGNAL(pictureUriChanged(const QString &)));
+    connect(GSettingsControllerQml::instance(), SIGNAL(usageModeChanged(const QString &)),
+            this, SIGNAL(usageModeChanged(const QString &)));
 }
 
 GSettingsSchemaQml * GSettingsQml::schema() const {
     return m_schema;
 }
 
-QString GSettingsQml::pictureUri() const {
-    return m_pictureUri;
-}
-
-void GSettingsQml::setPictureUri(const QString &str) {
-    if (str != m_pictureUri) {
-        m_pictureUri = str;
-        Q_EMIT pictureUriChanged(m_pictureUri);
+QString GSettingsQml::pictureUri() const
+{
+    if (m_schema->id() == "org.gnome.desktop.background") {
+        return GSettingsControllerQml::instance()->pictureUri();
+    } else {
+        return "";
+    }
+}
+
+void GSettingsQml::setPictureUri(const QString &str)
+{
+    if (m_schema->id() == "org.gnome.desktop.background") {
+        GSettingsControllerQml::instance()->setPictureUri(str);
+    }
+}
+
+QString GSettingsQml::usageMode() const
+{
+    if (m_schema->id() == "com.canonical.Unity8") {
+        return GSettingsControllerQml::instance()->usageMode();
+    } else {
+        return "";
+    }
+}
+
+void GSettingsQml::setUsageMode(const QString &usageMode)
+{
+    if (m_schema->id() == "com.canonical.Unity8") {
+        GSettingsControllerQml::instance()->setUsageMode(usageMode);
     }
 }

=== modified file 'tests/mocks/GSettings.1.0/fake_gsettings.h'
--- tests/mocks/GSettings.1.0/fake_gsettings.h	2014-08-26 08:14:44 +0000
+++ tests/mocks/GSettings.1.0/fake_gsettings.h	2015-04-03 14:23:20 +0000
@@ -46,25 +46,26 @@
 
     Q_PROPERTY(GSettingsSchemaQml* schema READ schema NOTIFY schemaChanged)
     Q_PROPERTY(QString pictureUri READ pictureUri WRITE setPictureUri NOTIFY pictureUriChanged)
+    Q_PROPERTY(QString usageMode READ usageMode WRITE setUsageMode NOTIFY usageModeChanged)
 
 public:
     GSettingsQml(QObject *parent = nullptr);
-    ~GSettingsQml();
 
     GSettingsSchemaQml * schema() const;
     QString pictureUri() const;
+    QString usageMode() const;
 
     void setPictureUri(const QString &str);
+    void setUsageMode(const QString &usageMode);
 
 Q_SIGNALS:
     void schemaChanged();
     void pictureUriChanged(const QString&);
+    void usageModeChanged(const QString&);
 
 private:
     GSettingsSchemaQml* m_schema;
 
-    QString m_pictureUri;
-
     friend class GSettingsSchemaQml;
 };
 
@@ -76,13 +77,22 @@
     static GSettingsControllerQml* instance();
     ~GSettingsControllerQml();
 
-    void registerSettingsObject(GSettingsQml *obj);
-    void unRegisterSettingsObject(GSettingsQml *obj);
-    Q_INVOKABLE void setPictureUri(const QString &str);
+    QString pictureUri() const;
+    void setPictureUri(const QString &str);
+
+    QString usageMode() const;
+    void setUsageMode(const QString &usageMode);
+
+Q_SIGNALS:
+    void pictureUriChanged(const QString&);
+    void usageModeChanged(const QString&);
 
 private:
     GSettingsControllerQml();
 
+    QString m_pictureUri;
+    QString m_usageMode;
+
     static GSettingsControllerQml* s_controllerInstance;
     QList<GSettingsQml *> m_registeredGSettings;
 };

=== modified file 'tests/mocks/GSettings.1.0/plugin.cpp'
--- tests/mocks/GSettings.1.0/plugin.cpp	2013-08-23 11:56:44 +0000
+++ tests/mocks/GSettings.1.0/plugin.cpp	2015-04-03 14:23:20 +0000
@@ -19,14 +19,8 @@
 
 #include <QtQml/qqml.h>
 
-static QObject* controllerProvider(QQmlEngine* /* engine */, QJSEngine* /* scriptEngine */)
-{
-    return GSettingsControllerQml::instance();
-}
-
 void FakeGSettingsQmlPlugin::registerTypes(const char *uri)
 {
-    qmlRegisterSingletonType<GSettingsControllerQml>(uri, 1, 0, "GSettingsController", controllerProvider);
     qmlRegisterType<GSettingsQml>(uri, 1, 0, "GSettings");
     qmlRegisterUncreatableType<GSettingsSchemaQml>(uri, 1, 0, "GSettingsSchema",
                                                    "GSettingsSchema can only be used inside of a GSettings component");

=== modified file 'tests/qmltests/Tutorial/tst_Tutorial.qml'
--- tests/qmltests/Tutorial/tst_Tutorial.qml	2015-02-05 21:13:16 +0000
+++ tests/qmltests/Tutorial/tst_Tutorial.qml	2015-04-03 14:23:20 +0000
@@ -17,6 +17,7 @@
 import QtQuick 2.0
 import QtTest 1.0
 import AccountsService 0.1
+import GSettings 1.0
 import LightDM 0.1 as LightDM
 import Ubuntu.Components 1.1
 import Unity.Application 0.1
@@ -45,6 +46,11 @@
         }
     }
 
+    GSettings {
+        id: unity8Settings
+        schema.id: "com.canonical.Unity8"
+    }
+
     Component.onCompleted: {
         // must set the mock mode before loading the Shell
         LightDM.Greeter.mockMode = "single-pin";
@@ -112,6 +118,7 @@
 
         function init() {
             tryCompare(shell, "enabled", true); // enabled by greeter when ready
+            unity8Settings.usageMode = "Staged";
             AccountsService.demoEdges = false;
             AccountsService.demoEdges = true;
             swipeAwayGreeter();
@@ -185,17 +192,19 @@
         }
 
         function checkRightEdge() {
-            touchFlick(shell, shell.width, halfHeight, halfWidth, halfHeight);
+            if (unity8Settings.usageMode === "Staged") {
+                touchFlick(shell, shell.width, halfHeight, halfWidth, halfHeight);
 
-            var stage = findChild(shell, "stage");
-            var spreadView = findChild(stage, "spreadView");
-            tryCompare(spreadView, "phase", 0);
+                var stage = findChild(shell, "stage");
+                var spreadView = findChild(stage, "spreadView");
+                tryCompare(spreadView, "phase", 0);
+            }
         }
 
         function checkBottomEdge() {
             // Can't actually check effect of swipe, since dash isn't really loaded
             var applicationsDisplayLoader = findChild(shell, "applicationsDisplayLoader");
-            tryCompare(applicationsDisplayLoader.item, "interactive", false);
+            tryCompare(applicationsDisplayLoader, "interactive", false);
         }
 
         function checkFinished() {
@@ -256,6 +265,14 @@
             goToPage(null);
         }
 
+        function test_walkthroughOnDesktop() {
+            unity8Settings.usageMode = "Windowed";
+            var page = goToPage("tutorialLeftFinish");
+            var tick = findChild(page, "tick");
+            tap(tick);
+            checkFinished();
+        }
+
         function test_launcherShortDrag() {
             // goToPage does a normal launcher pull.  But here we want to test
             // just barely pulling the launcher out and letting go (i.e. not

=== modified file 'tests/qmltests/tst_Shell.qml'
--- tests/qmltests/tst_Shell.qml	2015-03-18 10:16:42 +0000
+++ tests/qmltests/tst_Shell.qml	2015-04-03 14:23:20 +0000
@@ -988,6 +988,11 @@
             tryCompare(ApplicationManager, "focusedApplicationId", "gallery-app");
             compare(wizard.shown, false);
             compare(tutorial.running, false);
+            tryCompare(AccountsService, "demoEdges", false);
+            tryCompare(Wizard.System, "wizardEnabled", false);
+
+            var tutorialLeft = findChild(tutorial, "tutorialLeft");
+            compare(tutorialLeft, null); // should be destroyed with tutorial
         }
 
         function test_tapOnRightEdgeReachesApplicationSurface() {
@@ -1084,11 +1089,11 @@
             {
                 var buttonShowDashHome = findChild(launcher, "buttonShowDashHome");
                 var startPos = buttonShowDashHome.mapToItem(shell,
+                        buttonShowDashHome.width * 0.2,
+                        buttonShowDashHome.height * 0.8);
+                var endPos = buttonShowDashHome.mapToItem(shell,
                         buttonShowDashHome.width * 0.8,
                         buttonShowDashHome.height * 0.2);
-                var endPos = buttonShowDashHome.mapToItem(shell,
-                        buttonShowDashHome.width * 0.2,
-                        buttonShowDashHome.height * 0.8);
                 touchFlick(shell, startPos.x, startPos.y, endPos.x, endPos.y);
             }