Merge lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/manualorientation into lp:ubuntu-ui-toolkit

[Cris Dywan]

Define window within OrientationHelper to avoid a race condition

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:794
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/981/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy-vm/394
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/2949
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-amd64-ci/838
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-armhf-ci/838
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-armhf-ci/838/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-vm-saucy/290
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/4542
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/4542/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/2951
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/2951/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/2457
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/2500

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/ubuntu-ui-toolkit-ci/981/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:795
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/989/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy-vm/402
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/3011
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-amd64-ci/846
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-armhf-ci/846
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-armhf-ci/846/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-vm-saucy/298
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/4550
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/4550/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/3013
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/3013/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/2518
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/2562
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/46
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/42

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/ubuntu-ui-toolkit-ci/989/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:796
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/1077/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/58
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/58
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-amd64-ci/25
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/25
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/25/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/51
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/58
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/58/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/58
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/58/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/2688
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/2739
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/417
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/415

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/ubuntu-ui-toolkit-ci/1077/rebuild

review: Needs Fixing (continuous-integration)

[Cris Dywan (kalikiana)]

Looks like a Jenkins error? There's no change to autopilot tests in this branch.

StateNotFoundError: State not found for class 'ActionItem' and filters {'objectName': 'unexisting'}.

[Tim Peeters (tpeeters)]

Do you need to define window, or is it possible to check if window is defined before you use it?

[Cris Dywan (kalikiana)]

Two points on that:
If not defined, there's a race between the signals and window being available - if it's defined it can be used exactly when needed.
There's no signal to know that window was defined that I can see. It's a global variable.

[Tim Peeters (tpeeters)]

merge again? jenkins needs to pass.

[Tim Peeters (tpeeters)]

114 + * Author: Zsombor Egri <email address hidden>

really? ;)

[Tim Peeters (tpeeters)]

> Two points on that:
> If not defined, there's a race between the signals and window being available
> - if it's defined it can be used exactly when needed.
> There's no signal to know that window was defined that I can see. It's a
> global variable.

what happens when you define it here? Is there still a global variable in addition to your new local variable?

[Tim Peeters (tpeeters)]

Why do you add cpp tests and not simply add qml tests for the orientations in tst_components? Also I'd like to see a test that explicitly checks for a regression of the bug.

[Tim Peeters (tpeeters)]

> Why do you add cpp tests and not simply add qml tests for the orientations in
> tst_components? Also I'd like to see a test that explicitly checks for a
> regression of the bug.

You do check for a 'window' warning, but please add a reference to the bug there, or if you add a qml test, add the bug id in the function name.

[Tim Peeters (tpeeters)]

review: Needs Information

[Cris Dywan (kalikiana)]

> what happens when you define it here? Is there still a global variable in addition to your new local variable?

I don't understand the question. Defining window fixes the race condition. It's the same object otherwise.

> Why do you add cpp tests and not simply add qml tests for the orientations in tst_components?

I cannot produce the warnings in the qml test case with and without windowShown.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:799
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/1142/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/261
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/253
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-amd64-ci/90
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/90
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/90/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/248
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/261
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/261/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/253
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/253/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/2870
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/2922
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/787
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/788

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/ubuntu-ui-toolkit-ci/1142/rebuild

review: Approve (continuous-integration)

=== modified file 'modules/Ubuntu/Components/OrientationHelper.qml'
--- modules/Ubuntu/Components/OrientationHelper.qml	2013-07-29 11:14:35 +0000
+++ modules/Ubuntu/Components/OrientationHelper.qml	2013-10-31 16:49:34 +0000
@@ -92,6 +92,14 @@
     Component.onCompleted: orientationTransition.enabled = transitionEnabled
 
     /*!
+      Technically 'window' is defined by QML automatically however this can
+      happen very late. We define it here so there's no race condition.
+     */
+    Window {
+        id: window
+    }
+
+    /*!
       \internal
 
       Report the current orientation of the application via QWindow::contentOrientation.

=== added directory 'tests/unit_x11/tst_orientation'
=== added file 'tests/unit_x11/tst_orientation/Defaults.qml'
--- tests/unit_x11/tst_orientation/Defaults.qml	1970-01-01 00:00:00 +0000
+++ tests/unit_x11/tst_orientation/Defaults.qml	2013-10-31 16:49:34 +0000
@@ -0,0 +1,27 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser 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 Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import QtQuick 2.0
+import Ubuntu.Components 0.1
+
+Item {
+    width: 100
+    height: 100
+
+    OrientationHelper {
+        objectName: "helper"
+    }
+}

=== added file 'tests/unit_x11/tst_orientation/ManualAngle.qml'
--- tests/unit_x11/tst_orientation/ManualAngle.qml	1970-01-01 00:00:00 +0000
+++ tests/unit_x11/tst_orientation/ManualAngle.qml	2013-10-31 16:49:34 +0000
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser 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 Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import QtQuick 2.0
+import Ubuntu.Components 0.1
+
+Item {
+    width: 100
+    height: 100
+
+    OrientationHelper {
+        objectName: "helper"
+        orientationAngle: 90
+    }
+
+    Item {
+        objectName: "checkpoint"
+        property int contentOrientation: window.contentOrientation
+        property int orientation: Screen.Orientation
+    }
+}

=== added file 'tests/unit_x11/tst_orientation/tst_orientation.cpp'
--- tests/unit_x11/tst_orientation/tst_orientation.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit_x11/tst_orientation/tst_orientation.cpp	2013-10-31 16:49:34 +0000
@@ -0,0 +1,110 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser 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 Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Author: Christian Dywan <[e-mail]>
+ */
+
+#include <QtTest/QtTest>
+#include <QtTest/QSignalSpy>
+#include <QtCore/QCoreApplication>
+#include <QtQml/QQmlEngine>
+#include <QtQuick/QQuickView>
+#include <QtQuick/QQuickItem>
+#include <QVector3D>
+#include <QVector4D>
+#include <QQuaternion>
+#include <QMatrix4x4>
+#include <QtQuick/QQuickItem>
+#include <QtQml/QQmlProperty>
+
+class tst_OrientationTest : public QObject
+{
+    Q_OBJECT
+public:
+    tst_OrientationTest() {}
+
+private:
+    QString m_modulePath;
+
+    QQuickView *createView(const QString &file, QSignalSpy **spy = 0)
+    {
+        QQuickView *view = new QQuickView(0);
+        if (spy) {
+            *spy = new QSignalSpy(view->engine(), SIGNAL(warnings(QList<QQmlError>)));
+            (*spy)->setParent(view);
+        }
+        view->engine()->addImportPath(m_modulePath);
+        view->setSource(QUrl::fromLocalFile(file));
+        if (!view->rootObject()) {
+            return 0;
+        }
+        view->show();
+        QTest::qWaitForWindowExposed(view);
+        return view;
+    }
+
+private Q_SLOTS:
+
+    void initTestCase()
+    {
+        QDir modules ("../../../modules");
+        QVERIFY(modules.exists());
+        m_modulePath = modules.absolutePath();
+    }
+
+    void cleanupTestCase()
+    {
+    }
+
+    void test_defaults()
+    {
+        QSignalSpy *spy;
+        QQuickView *view = createView("Defaults.qml", &spy);
+        QVERIFY(view);
+        QQuickItem *helper = view->rootObject()->findChild<QQuickItem*>("helper");
+        QVERIFY(helper);
+        QCOMPARE(helper->property("automaticOrientation").toBool(), true);
+        // No warnings expected
+        QCOMPARE(spy->count(), 0);
+        delete view;
+    }
+
+    void test_manualAngle()
+    {
+        QSignalSpy *spy;
+        QQuickView *view = createView("ManualAngle.qml", &spy);
+        QVERIFY(view);
+        QQuickItem *helper = view->rootObject()->findChild<QQuickItem*>("helper");
+        QVERIFY(helper);
+        // No warning about "window" being undefined must appear
+        QCOMPARE(spy->count(), 0);
+        QCOMPARE(helper->property("orientationAngle").toInt(), 90);
+        // Verify expected values
+        QQuickItem *checkpoint = view->rootObject()->findChild<QQuickItem*>("checkpoint");
+        QVERIFY(checkpoint);
+        // window.contentOrientation 
+        QCOMPARE(checkpoint->property("contentOrientation").toInt(), helper->property("orientationAngle").toInt());
+        // Screen.Orientation
+        QCOMPARE(checkpoint->property("orientation").toInt(), helper->property("orientationAngle").toInt());
+        delete view;
+    }
+};
+
+QTEST_MAIN(tst_OrientationTest)
+
+#include "tst_orientation.moc"
+
+
+

=== added file 'tests/unit_x11/tst_orientation/tst_orientation.pro'
--- tests/unit_x11/tst_orientation/tst_orientation.pro	1970-01-01 00:00:00 +0000
+++ tests/unit_x11/tst_orientation/tst_orientation.pro	2013-10-31 16:49:34 +0000
@@ -0,0 +1,9 @@
+include(../test-include.pri)
+DEFINES += SRCDIR=\\\"$$PWD/\\\"
+
+SOURCES += \
+    tst_orientation.cpp
+
+OTHER_FILES += \
+    Defaults.qml \
+    ManualOrientation.qml

=== modified file 'tests/unit_x11/unit_x11.pro'
--- tests/unit_x11/unit_x11.pro	2013-10-11 05:56:57 +0000
+++ tests/unit_x11/unit_x11.pro	2013-10-31 16:49:34 +0000
@@ -1,3 +1,3 @@
 TEMPLATE = subdirs
 
-SUBDIRS += tst_components tst_test tst_inversemousearea tst_recreateview tst_statesaver tst_theme_engine
+SUBDIRS += tst_components tst_test tst_inversemousearea tst_recreateview tst_statesaver tst_theme_engine tst_orientation