Revamp Wayland application identification; align Wayland and X11 models
ClosedPublic

Authored by hein on Nov 7 2016, 1:49 PM.

Details

Summary

This adds support for app ids which are absolute paths to .desktop
files to WaylandTasksModel, matching the heuristic in XWindowsTasksModel.

Behavior between the two models is made more similar in various ways,
such as matching codepaths for generating the ids inserted into the
KActivities database in requestNewInstance/requestOpenUrls (also the
X11 model was missing the notifyAccessed calls) and producing launcher
URLs.

Icons are still left to KWin however, instead of reading them from
KService (which is what KWin does in any case on Wayland) or from the
.desktop file. The latter might make sense, but to avoid duplicated
code would then best be done in KWin.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
hein updated this revision to Diff 7972.Nov 7 2016, 1:49 PM
hein retitled this revision from to Revamp Wayland application identification; align Wayland and X11 models.
hein updated this object.
hein edited the test plan for this revision. (Show Details)
hein added reviewers: Plasma, graesslin, broulik.
hein added a subscriber: plasma-devel.
Restricted Application added a project: Plasma. · View Herald TranscriptNov 7 2016, 1:49 PM
graesslin added inline comments.Nov 7 2016, 2:19 PM
libtaskmanager/waylandtasksmodel.cpp
233–241

here you are iterating twice over the cache: once for the contains, once for the value.

Maybe do something like:

auto it = appDataCache.constFind(window);
if (it != addDataCache.end()) {
    return *it;
}
const AppData &data = appDataFromAppId(window->appId());
appDataCache.insert(window, data);
return data;
hein updated this revision to Diff 8003.Nov 8 2016, 8:30 AM

Optimize cache accessors.

Avoid double iteration of hashes.

graesslin added inline comments.Nov 8 2016, 8:37 AM
libtaskmanager/launchertasksmodel.cpp
134

nitpick: added newline

libtaskmanager/tasktools.cpp
145

QLatin1String

libtaskmanager/xwindowtasksmodel.cpp
984

this looks like an unrelated change?

985

QStringLiteral("...")

1005

QStringLiteral

hein marked 5 inline comments as done.Nov 8 2016, 8:41 AM
hein added inline comments.
libtaskmanager/launchertasksmodel.cpp
134

Intentional - my style tends to involve a lot of whitespace I guess (I personally find it more readable when browsing code).

libtaskmanager/tasktools.cpp
145

Done.

libtaskmanager/xwindowtasksmodel.cpp
984

It's documented in the change description though.

985

Done.

1005

Done.

hein updated this revision to Diff 8005.Nov 8 2016, 8:42 AM
hein marked 5 inline comments as done.

Sprinkle more QLatin1String/QStringLiteral.

graesslin added inline comments.Nov 8 2016, 10:15 AM
libtaskmanager/waylandtasksmodel.cpp
375

another QStringLiteral missing

394–395

here as well

libtaskmanager/xwindowtasksmodel.cpp
984

and here

1004

and here

hein updated this revision to Diff 8007.Nov 8 2016, 10:39 AM

More QStringLiteral ...

graesslin accepted this revision.Nov 8 2016, 1:57 PM
graesslin edited edge metadata.
This revision is now accepted and ready to land.Nov 8 2016, 1:57 PM
This revision was automatically updated to reflect the committed changes.