Code review comment for lp:~abreu-alexandre/webbrowser-app/fix-application-name-setup

Can you add a Q_ASSERT(!appId.isEmpty()) to the beginning of BrowserApplication::initialize()?
Passing an empty app id would lead to undefined behaviour I think.
That would also render the check for appIdParts.isEmpty() in SingleInstanceManager::getProfilePathFromAppId() useless.

In single-instance-manager.cpp, the additional include of <QDebug> is useless, it’s already included earlier on.
The additional include of <QtCore/QCoreApplication> also appears unused. Can you please remove it?

> https://bugs.launchpad.net/ubuntu/+source/ubuntu-ui-toolkit/+bug/1555542
Can you shorten that link to https://launchpad.net/bugs/1555542 ?

> appIdParts.mid(1, 1)[0]
Isn’t that equivalent to appIdParts[1] ?

> return profilePath
> + QDir::separator()
> + appDesktopName;
That could go on one single line.

It doesn’t look like getProfilePathFromAppId() needs to be a static member of SingleInstanceManager, it could simply be a function inside an unnamed namespace.

There’s a meaningless additional blank line in webbrowser-app.cpp, can it be reverted?

In tst_SingleInstanceManagerTests.cpp, the second parameter to run() should not be an empty string, this is unexpected. Can you use an non-empty string token?

Have you considered my suggestion of completely bailing out if no app id is explicitly set for the webapp container? I don’t think it makes sense to have apps that run in a "generic" container anyway, does it?