[libtaskmanager] Expose process ID of application
ClosedPublic

Authored by broulik on Nov 7 2016, 3:28 PM.

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.
broulik updated this revision to Diff 7984.Nov 7 2016, 3:28 PM
broulik retitled this revision from to WIP: [libtaskmanager] Expose process ID of application.
broulik updated this object.
broulik edited the test plan for this revision. (Show Details)
broulik added a reviewer: Plasma.
broulik set the repository for this revision to R120 Plasma Workspace.
Restricted Application added a project: Plasma. · View Herald TranscriptNov 7 2016, 3:28 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
graesslin added inline comments.
libtaskmanager/xwindowtasksmodel.cpp
432

I would prefer if we expose it through KWindowInfo. Creating an additional NETWinInfo causes an additional x-server roundtrip here.

Also a word of warning: just because you have a pid, does not mean that the pid is from the local system. Also remote X clients might have the pid exposed. This means you also need to verify that the window is from the local system. And now the really bad news: that's not trivial. If you want to see a working implementation check kwin/client_machine.cpp. This is the result of about 15 years of KWin experience and trying to figure out correctly whether the window is from the local system without blocking. Yes, over the years we had multiple approaches which could crash, freeze and detect it incorrectly.

hein added a subscriber: hein.Nov 8 2016, 7:16 AM
hein added inline comments.
libtaskmanager/xwindowtasksmodel.cpp
432

I agree with this. I just removed extra uses of NETWinInfo and would prefer to keep things down to KWindowInfo.

broulik updated this revision to Diff 8130.Nov 14 2016, 9:51 AM
broulik retitled this revision from WIP: [libtaskmanager] Expose process ID of application to [libtaskmanager] Expose process ID of application.

Use KWindowInfo::pid() of Review 129362

hein requested changes to this revision.Nov 14 2016, 9:57 AM
hein added a reviewer: hein.

Can you add the role to WaylandTasksModel::data() with a FIXME comment similar to other ones there? It's useful to me to grep for TODOs.

This revision now requires changes to proceed.Nov 14 2016, 9:57 AM
broulik updated this revision to Diff 8131.Nov 14 2016, 10:11 AM
broulik edited edge metadata.

Add FIXME comment to WaylandTasksModel

hein accepted this revision.Nov 14 2016, 10:14 AM
hein edited edge metadata.

Thanks!

This revision is now accepted and ready to land.Nov 14 2016, 10:14 AM
This revision was automatically updated to reflect the committed changes.