Merge lp:~dandrader/qtubuntu/betterSessionName into lp:qtubuntu

[Daniel d'Andrada]

Generate better mir session names

Try to come up with some meaningful session name to uniquely identify this session,
helping with shell debugging

Immensely helpful to help make sense of unity8.log when debugging multiple/nested prompt sessions use cases such as in bug 1586219.

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

PASSED: Continuous integration, rev:317
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/63/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/1759
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1785
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1732
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1732
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1732
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1725
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1725/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1725
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1725/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1725
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1725/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1725
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1725/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1725
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1725/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1725
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1725/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1725
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1725/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1725
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1725/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1725
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1725/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

On 01/06/2016 09:48, Nick Dedekind wrote:
>> +QByteArray UbuntuClientIntegration::generateSessionName(QStringList &args)
>> >+{
> Perhaps use QApplication::applicationName first? I actually had a branch for this about a thousand years ago, but it never went anywhere.
>

QApplication::applicationName might not be set yet when
QPlatformIntegration is created (which is really early). It can be set
or even changed at any time by calling
QCoreApplicaiton::setApplicationName, and we would have not way of
updating your session id to follow those changes. So the only thing sure
to exist at QPlatformIntegration creation is the application arguments.

[Gerry Boland (gerboland)]

+for (int i = 0; i < args.count(); ++i) {
+ QString arg = args[i];
We have Q_FOREACH

Do you really need to read the FileInfo object? Why not just drop the ".qml" from the file name?

+ u_application_id_new_from_stringn(sessionName.data(), sessionName.count());
Does the api expect sessionName to always exist, or will it make a copy itself?

review: Needs Fixing

[Daniel d'Andrada (dandrader)]

On 01/06/2016 12:01, Gerry Boland wrote:
> Review: Needs Fixing
>
> +for (int i = 0; i < args.count(); ++i) {
> + QString arg = args[i];
> We have Q_FOREACH

I personally find this macro ugly, but if you fancy it...
But it is less code and thus less error prone. Done.

> Do you really need to read the FileInfo object? Why not just drop the ".qml" from the file name?
Yes. The argument could be a full file path like this:

/foo/bar/whatever/zumba.qml

Which would make for a cumbersome session name. QFileInfo neatly and
safely parses that filepath for me.

I like the ".qml" suffix as it tells us it's a plain qml app run via
qmlscene.

> + u_application_id_new_from_stringn(sessionName.data(), sessionName.count());
> Does the api expect sessionName to always exist, or will it make a copy itself?

It makes a copy. What a horrible API it would be otherwise.

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

PASSED: Continuous integration, rev:318
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/64/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/1762
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1788
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1735
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1735
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1735
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1728
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1728/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1728
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1728/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1728
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1728/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1728
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1728/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1728
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1728/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1728
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1728/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1728
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1728/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1728
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1728/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1728
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1728/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

> > Do you really need to read the FileInfo object? Why not just drop the ".qml"
> from the file name?
> Yes. The argument could be a full file path like this:
>
> /foo/bar/whatever/zumba.qml
>
> Which would make for a cumbersome session name. QFileInfo neatly and
> safely parses that filepath for me.
>
> I like the ".qml" suffix as it tells us it's a plain qml app run via
> qmlscene.

Yeah, that's ok. I just want to keep app startup light, using QFileInfo to parse a file name just strikes me as heavy for something that only those reading file logs will appreciate. But I won't block on that.

> > + u_application_id_new_from_stringn(sessionName.data(),
> sessionName.count());
> > Does the api expect sessionName to always exist, or will it make a copy
> itself?
>
> It makes a copy. What a horrible API it would be otherwise.

You never know :)

[Daniel d'Andrada (dandrader)]

On 01/06/2016 12:58, Gerry Boland wrote:
> Yeah, that's ok. I just want to keep app startup light, using QFileInfo to parse a file name just strikes me as heavy for something that only those reading file logs will appreciate. But I won't block on that.

QFileInfo creation looks pretty tame and lightweight. It pretty much
just stores the given filepath in a internal string.

The only real code we execute happens when we call QFileInfo::fileName,
where it essentially finds the last separator char in the string and
then "return m_filePath.mid(m_lastSeparator + 1);".

[Gerry Boland (gerboland)]

Ok, thanks for checking that. Change looks good to me!

review: Approve

[lp:~dandrader/qtubuntu/betterSessionName]

322. By Daniel d'Andrada on 2016-06-03

Generate better mir session names

Try to come up with some meaningful session name to uniquely identify this session,
helping with shell debugging

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

PASSED: Continuous integration, rev:318
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/69/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/1818
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1843
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1784
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1784
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1784
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1774
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1774/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1774
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1774/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1774
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1774/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1774
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1774/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1774
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1774/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1774
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1774/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1774
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1774/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1774
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1774/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1774
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1774/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

=== modified file 'src/ubuntumirclient/integration.cpp'
--- src/ubuntumirclient/integration.cpp	2016-04-28 14:44:18 +0000
+++ src/ubuntumirclient/integration.cpp	2016-06-03 13:56:35 +0000
@@ -28,6 +28,7 @@
 #include "window.h"
 
 // Qt
+#include <QFileInfo>
 #include <QGuiApplication>
 #include <private/qguiapplication_p.h>
 #include <qpa/qplatformnativeinterface.h>
@@ -77,8 +78,12 @@
     , mClipboard(new UbuntuClipboard)
     , mScaleFactor(1.0)
 {
-    setupOptions();
-    setupDescription();
+    {
+        QStringList args = QCoreApplication::arguments();
+        setupOptions(args);
+        QByteArray sessionName = generateSessionName(args);
+        setupDescription(sessionName);
+    }
 
     // Create new application instance
     mInstance = u_application_instance_new_from_description_with_options(mDesc, mOptions);
@@ -161,9 +166,8 @@
     return mServices;
 }
 
-void UbuntuClientIntegration::setupOptions()
+void UbuntuClientIntegration::setupOptions(QStringList &args)
 {
-    QStringList args = QCoreApplication::arguments();
     int argc = args.size() + 1;
     char **argv = new char*[argc];
     for (int i = 0; i < argc - 1; i++)
@@ -177,10 +181,11 @@
     delete [] argv;
 }
 
-void UbuntuClientIntegration::setupDescription()
+void UbuntuClientIntegration::setupDescription(QByteArray &sessionName)
 {
     mDesc = u_application_description_new();
-    UApplicationId* id = u_application_id_new_from_stringn("QtUbuntu", 8);
+
+    UApplicationId* id = u_application_id_new_from_stringn(sessionName.data(), sessionName.count());
     u_application_description_set_application_id(mDesc, id);
 
     UApplicationLifecycleDelegate* delegate = u_application_lifecycle_delegate_new();
@@ -190,6 +195,35 @@
     u_application_description_set_application_lifecycle_delegate(mDesc, delegate);
 }
 
+QByteArray UbuntuClientIntegration::generateSessionName(QStringList &args)
+{
+    // Try to come up with some meaningful session name to uniquely identify this session,
+    // helping with shell debugging
+
+    if (args.count() == 0) {
+        return QByteArray("QtUbuntu");
+    } if (args[0].contains("qmlscene")) {
+        return generateSessionNameFromQmlFile(args);
+    } else {
+        // use the executable name
+        QFileInfo fileInfo(args[0]);
+        return fileInfo.fileName().toLocal8Bit();
+    }
+}
+
+QByteArray UbuntuClientIntegration::generateSessionNameFromQmlFile(QStringList &args)
+{
+    Q_FOREACH (QString arg, args) {
+        if (arg.endsWith(".qml")) {
+            QFileInfo fileInfo(arg);
+            return fileInfo.fileName().toLocal8Bit();
+        }
+    }
+
+    // give up
+    return "qmlscene";
+}
+
 QPlatformWindow* UbuntuClientIntegration::createPlatformWindow(QWindow* window) const
 {
     return const_cast<UbuntuClientIntegration*>(this)->createPlatformWindow(window);

=== modified file 'src/ubuntumirclient/integration.h'
--- src/ubuntumirclient/integration.h	2016-04-28 14:09:54 +0000
+++ src/ubuntumirclient/integration.h	2016-06-03 13:56:35 +0000
@@ -76,8 +76,10 @@
     void destroyScreen(UbuntuScreen *screen);
 
 private:
-    void setupOptions();
-    void setupDescription();
+    void setupOptions(QStringList &args);
+    void setupDescription(QByteArray &sessionName);
+    static QByteArray generateSessionName(QStringList &args);
+    static QByteArray generateSessionNameFromQmlFile(QStringList &args);
 
     UbuntuNativeInterface* mNativeInterface;
     QPlatformFontDatabase* mFontDb;