Merge lp:~abreu-alexandre/webbrowser-app/xenial-fix-application-name-setup into lp:webbrowser-app/xenial

[Alexandre Abreu]

Fix container/webbrowser app_id; Properly set applicationname based on package name and app name

Fix container/webbrowser app_id; Properly set applicationname based on package name and app name

Unmerged revisions

1418. By Alexandre Abreu on 2016-05-11

Fix container/webbrowser app_id; Properly set applicationname based on package name and app name

=== modified file 'src/app/browserapplication.cpp'
--- src/app/browserapplication.cpp	2016-03-29 11:01:18 +0000
+++ src/app/browserapplication.cpp	2016-05-11 16:04:27 +0000
@@ -98,16 +98,6 @@
     return host;
 }
 
-QString BrowserApplication::appId() const
-{
-    Q_FOREACH(const QString& argument, m_arguments) {
-        if (argument.startsWith("--app-id=")) {
-            return argument.split("--app-id=")[1];
-        }
-    }
-    return QString();
-}
-
 #define MAKE_SINGLETON_FACTORY(type) \
     static QObject* type##_singleton_factory(QQmlEngine* engine, QJSEngine* scriptEngine) { \
         Q_UNUSED(engine); \
@@ -119,36 +109,54 @@
 MAKE_SINGLETON_FACTORY(MimeDatabase)
 MAKE_SINGLETON_FACTORY(Direction)
 
-
-bool BrowserApplication::initialize(const QString& qmlFileSubPath)
+bool BrowserApplication::initialize(const QString& qmlFileSubPath
+                                    , const QString& appId)
 {
     Q_ASSERT(m_window == 0);
 
+    if (appId.isEmpty()) {
+        qCritical() << "Cannot initialize the runtime environment: "
+                       "no application id detected.";
+        return false;
+    }
+
     if (m_arguments.contains("--help") || m_arguments.contains("-h")) {
         printUsage();
         return false;
     }
 
-    // Handle legacy platforms (i.e. current desktop versions, where
-    // applications are not started by the Ubuntu ApplicationManager).
-    if (qgetenv("APP_ID").isEmpty()) {
-        QString id = appId();
-        if (id.isEmpty()) {
-            id = QStringLiteral(APP_ID);
-        }
-        qputenv("APP_ID", id.toUtf8());
-    }
     // Ensure that application-specific data is written where it ought to.
-    QStringList appIdParts =
-        QString::fromUtf8(qgetenv("APP_ID")).split('_');
+    QStringList appIdParts = appId.split('_');
+
     QCoreApplication::setApplicationName(appIdParts.first());
     QCoreApplication::setOrganizationDomain(QCoreApplication::applicationName());
+
     // Get also the the first two components of the app ID: <package>_<app>,
     // which is needed by Online Accounts.
     QString unversionedAppId = QStringList(appIdParts.mid(0, 2)).join('_');
 
     // Ensure only one instance of the app is running.
-    if (m_singleton.run(m_arguments)) {
+    // For webapps using the container as a launcher, the predicate that
+    // is used to determine if this running instance is a duplicate of
+    // a running one, is based on the current APP_ID.
+    // The app id is formed as: <package name>_<app name>_<version>
+
+    // Where the <package name> is specified in the the manifest.json as
+    // "appName" and is specific for the whole click package.
+
+    // The <app name> portion is based on the desktop file name and is a short
+    // app name. This name is meaningful when more than one desktop file is
+    // found in a given click package.
+
+    // IMPORTANT:
+    // 1. When a click application contains more than one desktop file
+    // the bundle is considered a single app from the point of view of the
+    // cache and resource file locations. THOSE FILES ARE THEN SHARED between
+    // the instances.
+    // 2. To make sure that if more than one desktop file is found in a click package,
+    // those apps are not considered the same instance, the instance existance predicate
+    // is based on the <package name> AND the <app name> detailed above.
+    if (m_singleton.run(m_arguments, appId)) {
         connect(&m_singleton, SIGNAL(newInstanceLaunched(const QStringList&)),
                 SLOT(onNewInstanceLaunched(const QStringList&)));
     } else {

=== modified file 'src/app/browserapplication.h'
--- src/app/browserapplication.h	2016-01-15 09:29:22 +0000
+++ src/app/browserapplication.h	2016-05-11 16:04:27 +0000
@@ -45,7 +45,7 @@
     BrowserApplication(int& argc, char** argv);
     ~BrowserApplication();
 
-    bool initialize(const QString& qmlFileSubPath);
+    bool initialize(const QString& qmlFileSubPath, const QString& appId);
     int run();
 
 protected:
@@ -63,7 +63,6 @@
     void onNewInstanceLaunched(const QStringList& arguments) const;
 
 private:
-    QString appId() const;
     QString inspectorPort() const;
     QString inspectorHost() const;
 

=== modified file 'src/app/config.h.in'
--- src/app/config.h.in	2015-05-14 05:57:01 +0000
+++ src/app/config.h.in	2016-05-11 16:04:27 +0000
@@ -23,7 +23,6 @@
 #include <QtCore/QDir>
 #include <QtCore/QString>
 
-#define APP_ID "webbrowser-app"
 #define REMOTE_INSPECTOR_PORT 9221
 
 inline bool isRunningInstalled()

=== modified file 'src/app/single-instance-manager.cpp'
--- src/app/single-instance-manager.cpp	2016-04-08 13:49:25 +0000
+++ src/app/single-instance-manager.cpp	2016-05-11 16:04:27 +0000
@@ -33,12 +33,48 @@
 #include "single-instance-manager.h"
 
 namespace {
+
 const int kWaitForRunningInstanceToRespondMs = 1000;
 const int kWaitForRunningInstanceToAckMs = 1000;
 const int kDataStreamVersion = QDataStream::Qt_5_0;
 const QString kHeaderToken = QStringLiteral("MESSAGE");
 const QString kAckToken = QStringLiteral("ACK");
-}
+
+QString getProfilePathFromAppId(const QString& appId)
+{
+    QString profilePath =
+            QStandardPaths::writableLocation(QStandardPaths::DataLocation);
+
+    QStringList appIdParts = appId.split('_', QString::SkipEmptyParts);
+
+    QString appDesktopName;
+
+    // We try to get the "short app name" to try to uniquely identify
+    // the single instance profile path.
+
+    // In cases where you have a single click with multiple apps in it,
+    // the "app name" as defined in the click manifest.json file will be
+    // a proper way to distinguish a unique instance, it needs to take
+    // the desktop name into account.
+
+    // At the moment there is no clean way to get those click app name
+    // paths, see:
+    //  https://launchpad.net/bugs/1555542
+
+    if (appIdParts.size() >= 3) {
+        // Assume that we have a APP_ID that corresponds to:
+        // <manifest app name>_<desktop app name>_<version>
+        appDesktopName = appIdParts[1];
+    } else {
+        // We either run on desktop or as the webbrowser
+        appDesktopName = appIdParts.first();
+    }
+
+    return profilePath + QDir::separator() + appDesktopName;
+}
+
+}
+
 
 SingleInstanceManager::SingleInstanceManager(QObject* parent)
     : QObject(parent)
@@ -54,13 +90,13 @@
     return false;
 }
 
-bool SingleInstanceManager::run(const QStringList& arguments)
+bool SingleInstanceManager::run(const QStringList& arguments, const QString& appId)
 {
     if (m_server.isListening()) {
         return false;
     }
 
-    QDir profile(QStandardPaths::writableLocation(QStandardPaths::DataLocation));
+    QDir profile(getProfilePathFromAppId(appId));
     if (!profile.exists()) {
         if (!QDir::root().mkpath(profile.absolutePath())) {
             qCritical() << "Failed to create profile directory,"

=== modified file 'src/app/single-instance-manager.h'
--- src/app/single-instance-manager.h	2016-01-15 10:06:31 +0000
+++ src/app/single-instance-manager.h	2016-05-11 16:04:27 +0000
@@ -33,7 +33,7 @@
 public:
     SingleInstanceManager(QObject* parent=nullptr);
 
-    bool run(const QStringList& arguments);
+    bool run(const QStringList& arguments, const QString& appId);
 
 Q_SIGNALS:
     void newInstanceLaunched(const QStringList& arguments) const;

=== modified file 'src/app/webbrowser/webbrowser-app.cpp'
--- src/app/webbrowser/webbrowser-app.cpp	2016-02-05 15:20:10 +0000
+++ src/app/webbrowser/webbrowser-app.cpp	2016-05-11 16:04:27 +0000
@@ -76,7 +76,7 @@
     qmlRegisterSingletonType<DownloadsModel>(uri, 0, 1, "DownloadsModel", DownloadsModel_singleton_factory);
     qmlRegisterType<TextSearchFilterModel>(uri, 0, 1, "TextSearchFilterModel");
 
-    if (BrowserApplication::initialize("webbrowser/webbrowser-app.qml")) {
+    if (BrowserApplication::initialize("webbrowser/webbrowser-app.qml", QStringLiteral("webbrowser-app"))) {
         QStringList searchEnginesSearchPaths;
         searchEnginesSearchPaths << QStandardPaths::writableLocation(QStandardPaths::DataLocation) + "/searchengines";
         searchEnginesSearchPaths << UbuntuBrowserDirectory() + "/webbrowser/searchengines";

=== modified file 'src/app/webcontainer/webapp-container.cpp'
--- src/app/webcontainer/webapp-container.cpp	2016-02-29 13:32:17 +0000
+++ src/app/webcontainer/webapp-container.cpp	2016-05-11 16:04:27 +0000
@@ -92,12 +92,38 @@
 {
 }
 
+QString WebappContainer::appId() const
+{
+    Q_FOREACH(const QString& argument, m_arguments) {
+        if (argument.startsWith("--app-id=")) {
+            return argument.split("--app-id=")[1];
+        }
+    }
+    return QString();
+}
 
 bool WebappContainer::initialize()
 {
     earlyEnvironment();
 
-    if (BrowserApplication::initialize("webcontainer/webapp-container.qml")) {
+    if (qgetenv("APP_ID").isEmpty()) {
+        QString id = appId();
+        if (id.isEmpty()) {
+            qCritical() << "The application has been launched with no "
+                          "explicit or system provided app id. "
+                          "An application id can be set by using the --app-id "
+                          "command line parameter and setting it to a unique "
+                          "application specific value or using the APP_ID environment "
+                          "variable.";
+            return false;
+        }
+        qputenv("APP_ID", id.toUtf8());
+    }
+
+    if (BrowserApplication::initialize(
+                "webcontainer/webapp-container.qml",
+                QString::fromUtf8(qgetenv("APP_ID")))) {
+
         parseCommandLine();
         parseExtraConfiguration();
 

=== modified file 'src/app/webcontainer/webapp-container.h'
--- src/app/webcontainer/webapp-container.h	2016-02-27 21:45:57 +0000
+++ src/app/webcontainer/webapp-container.h	2016-05-11 16:04:27 +0000
@@ -54,6 +54,7 @@
     bool shouldNotValidateCommandLineUrls() const;
     bool isValidLocalIntentFilterFile(const QString& filename) const;
     void setupLocalSchemeFilterIfAny(QQmlContext* context, const QString& webappSearchPath);
+    QString appId() const;
 
 private:
     QString m_webappName;

=== modified file 'tests/autopilot/webapp_container/tests/__init__.py'
--- tests/autopilot/webapp_container/tests/__init__.py	2016-01-22 10:19:34 +0000
+++ tests/autopilot/webapp_container/tests/__init__.py	2016-05-11 16:04:27 +0000
@@ -57,6 +57,10 @@
             args.append(
                 '--desktop_file_hint=/usr/share/applications/'
                 'webbrowser-app.desktop')
+
+        if next(filter(lambda e: e.startswith('--appid'), args), None) is None:
+            args.append('--app-id=running.test')
+
         if envvars:
             for envvar_key in envvars:
                 self.useFixture(fixtures.EnvironmentVariable(

=== modified file 'tests/unittests/single-instance-manager/tst_SingleInstanceManagerTests.cpp'
--- tests/unittests/single-instance-manager/tst_SingleInstanceManagerTests.cpp	2016-01-18 14:45:12 +0000
+++ tests/unittests/single-instance-manager/tst_SingleInstanceManagerTests.cpp	2016-05-11 16:04:27 +0000
@@ -50,18 +50,18 @@
 
     void test_cannot_run_twice_same_instance()
     {
-        QVERIFY(singleton->run(QStringList()));
-        QVERIFY(!singleton->run(QStringList()));
+        QVERIFY(singleton->run(QStringList(), "appid"));
+        QVERIFY(!singleton->run(QStringList(), "appid"));
         QVERIFY(newInstanceSpy->isEmpty());
     }
 
     void test_arguments_passed_to_already_running_instance()
     {
-        QVERIFY(singleton->run(QStringList()));
+        QVERIFY(singleton->run(QStringList(), "appid"));
         SingleInstanceManager other;
         QStringList args;
         args << QStringLiteral("foo") << QStringLiteral("bar") << QStringLiteral("baz");
-        QVERIFY(!other.run(args));
+        QVERIFY(!other.run(args, "appid"));
         newInstanceSpy->wait();
         QCOMPARE(newInstanceSpy->first().at(0).toStringList(), args);
     }