Merge lp:~3v1n0/unity/key-grabber-ref-counting into lp:unity

Proposed by Marco Trevisan (Treviño)
Status: Merged
Approved by: Andrea Azzarone
Approved revision: no longer in the source branch.
Merged at revision: 4059
Proposed branch: lp:~3v1n0/unity/key-grabber-ref-counting
Merge into: lp:unity
Diff against target: 109 lines (+18/-22)
2 files modified
unity-shared/GnomeKeyGrabber.cpp (+17/-21)
unity-shared/GnomeKeyGrabberImpl.h (+1/-1)
To merge this branch: bzr merge lp:~3v1n0/unity/key-grabber-ref-counting
Reviewer Review Type Date Requested Status
Andrea Azzarone (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+280343@code.launchpad.net

Commit message

GnomeKeyGrabber: refcount the actions and remove them only when nobody needs

There might be multiple customers for an action (i.e. a menu entry and a dbus customer)
so we can't remove them in this case without ref-counting.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Andrea Azzarone (azzar1) wrote :

+1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'unity-shared/GnomeKeyGrabber.cpp'
2--- unity-shared/GnomeKeyGrabber.cpp 2015-10-27 17:35:44 +0000
3+++ unity-shared/GnomeKeyGrabber.cpp 2015-12-13 10:39:16 +0000
4@@ -98,7 +98,9 @@
5 auto it = std::find(actions_.begin(), actions_.end(), action);
6 if (it != actions_.end())
7 {
8- action_id = actions_ids_[it - actions_.begin()];
9+ auto action_index = it - actions_.begin();
10+ action_id = actions_ids_[action_index];
11+ ++actions_customers_[action_index];
12 LOG_DEBUG(logger) << "Key binding \"" << action.keyToString() << "\" is already grabbed, reusing id " << action_id;
13 return true;
14 }
15@@ -107,6 +109,7 @@
16 {
17 actions_ids_.push_back(action_id);
18 actions_.push_back(action);
19+ actions_customers_.push_back(1);
20 return true;
21 }
22
23@@ -148,12 +151,23 @@
24 if (!index || index >= actions_.size())
25 return false;
26
27+ if (actions_customers_[index] > 1)
28+ {
29+ LOG_DEBUG(logger) << "Not removing action " << actions_[index].keyToString()
30+ << " as it is used by multiple customers ("
31+ << actions_customers_[index] << ")";
32+
33+ --actions_customers_[index];
34+ return false;
35+ }
36+
37 CompAction* action = &(actions_[index]);
38 LOG_DEBUG(logger) << "RemoveAction (\"" << action->keyToString() << "\")";
39
40 screen_->removeAction(action);
41 actions_.erase(actions_.begin() + index);
42 actions_ids_.erase(actions_ids_.begin() + index);
43+ actions_customers_.erase(actions_customers_.begin() + index);
44
45 return true;
46 }
47@@ -258,7 +272,7 @@
48 if (it != actions_by_owner_.end())
49 {
50 for (auto action_id : it->second.actions)
51- RemoveActionForOwner(action_id, name);
52+ RemoveActionByID(action_id);
53
54 actions_by_owner_.erase(it);
55 }
56@@ -284,7 +298,7 @@
57 if (actions.empty())
58 actions_by_owner_.erase(it);
59
60- return RemoveActionForOwner(action_id, owner);
61+ return RemoveActionByID(action_id);
62 }
63
64 LOG_WARN(logger) << "Action " << action_id << " was not registered by " << owner << ". "
65@@ -292,24 +306,6 @@
66 return false;
67 }
68
69-bool GnomeGrabber::Impl::RemoveActionForOwner(uint32_t action_id, std::string const& owner)
70-{
71- for (auto it = actions_by_owner_.begin(); it != actions_by_owner_.end(); ++it)
72- {
73- if (it->first == owner)
74- continue;
75-
76- auto const& actions = it->second.actions;
77- if (actions.find(action_id) != actions.end())
78- {
79- LOG_DEBUG(logger) << "Action " << action_id << " registered for multiple destinations, not removed";
80- return false;
81- }
82- }
83-
84- return RemoveActionByID(action_id);
85-}
86-
87 void GnomeGrabber::Impl::ActivateDBusAction(CompAction const& action, uint32_t action_id, uint32_t device, uint32_t timestamp) const
88 {
89 LOG_DEBUG(logger) << "ActivateAction (" << action_id << " \"" << action.keyToString() << "\")";
90
91=== modified file 'unity-shared/GnomeKeyGrabberImpl.h'
92--- unity-shared/GnomeKeyGrabberImpl.h 2015-10-01 03:52:09 +0000
93+++ unity-shared/GnomeKeyGrabberImpl.h 2015-12-13 10:39:16 +0000
94@@ -52,7 +52,6 @@
95 uint32_t GrabDBusAccelerator(std::string const& owner, std::string const& accelerator, uint32_t flags);
96 bool UnGrabDBusAccelerator(std::string const& sender, uint32_t action_id);
97 void ActivateDBusAction(CompAction const& action, uint32_t id, uint32_t device, uint32_t timestamp) const;
98- bool RemoveActionForOwner(uint32_t action_id, std::string const& owner);
99
100 bool IsActionPostponed(CompAction const& action) const;
101
102@@ -63,6 +62,7 @@
103
104 uint32_t current_action_id_;
105 std::vector<uint32_t> actions_ids_;
106+ std::vector<uint32_t> actions_customers_;
107 CompAction::Vector actions_;
108
109 struct OwnerActions { glib::DBusNameWatcher::Ptr watcher; std::unordered_set<uint32_t> actions; };