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. Fixes: #1525308 Approved by: PS Jenkins bot, Andrea Azzarone (bzr r4059)

2 files changed, 18 insertions, 22 deletions

diff --git a/unity-shared/GnomeKeyGrabber.cpp b/unity-shared/GnomeKeyGrabber.cpp
index 603daa2af..300b07ebe 100644
--- a/unity-shared/GnomeKeyGrabber.cpp
+++ b/unity-shared/GnomeKeyGrabber.cpp
@@ -98,7 +98,9 @@ bool GnomeGrabber::Impl::AddAction(CompAction const& action, uint32_t& action_id
 auto it = std::find(actions_.begin(), actions_.end(), action);
 if (it != actions_.end())
 {
- action_id = actions_ids_[it - actions_.begin()];
+ auto action_index = it - actions_.begin();
+ action_id = actions_ids_[action_index];
+ ++actions_customers_[action_index];
 LOG_DEBUG(logger) << "Key binding \"" << action.keyToString() << "\" is already grabbed, reusing id " << action_id;
 return true;
 }
@@ -107,6 +109,7 @@ bool GnomeGrabber::Impl::AddAction(CompAction const& action, uint32_t& action_id
 {
 actions_ids_.push_back(action_id);
 actions_.push_back(action);
+ actions_customers_.push_back(1);
 return true;
 }
 
@@ -148,12 +151,23 @@ bool GnomeGrabber::Impl::RemoveActionByIndex(size_t index)
 if (!index || index >= actions_.size())
 return false;
 
+ if (actions_customers_[index] > 1)
+ {
+ LOG_DEBUG(logger) << "Not removing action " << actions_[index].keyToString()
+ << " as it is used by multiple customers ("
+ << actions_customers_[index] << ")";
+
+ --actions_customers_[index];
+ return false;
+ }
+
 CompAction* action = &(actions_[index]);
 LOG_DEBUG(logger) << "RemoveAction (\"" << action->keyToString() << "\")";
 
 screen_->removeAction(action);
 actions_.erase(actions_.begin() + index);
 actions_ids_.erase(actions_ids_.begin() + index);
+ actions_customers_.erase(actions_customers_.begin() + index);
 
 return true;
 }
@@ -258,7 +272,7 @@ uint32_t GnomeGrabber::Impl::GrabDBusAccelerator(std::string const& owner, std::
 if (it != actions_by_owner_.end())
 {
 for (auto action_id : it->second.actions)
- RemoveActionForOwner(action_id, name);
+ RemoveActionByID(action_id);
 
 actions_by_owner_.erase(it);
 }
@@ -284,7 +298,7 @@ bool GnomeGrabber::Impl::UnGrabDBusAccelerator(std::string const& owner, uint32_
 if (actions.empty())
 actions_by_owner_.erase(it);
 
- return RemoveActionForOwner(action_id, owner);
+ return RemoveActionByID(action_id);
 }
 
 LOG_WARN(logger) << "Action " << action_id << " was not registered by " << owner << ". "
@@ -292,24 +306,6 @@ bool GnomeGrabber::Impl::UnGrabDBusAccelerator(std::string const& owner, uint32_
 return false;
 }
 
-bool GnomeGrabber::Impl::RemoveActionForOwner(uint32_t action_id, std::string const& owner)
-{
- for (auto it = actions_by_owner_.begin(); it != actions_by_owner_.end(); ++it)
- {
- if (it->first == owner)
- continue;
-
- auto const& actions = it->second.actions;
- if (actions.find(action_id) != actions.end())
- {
- LOG_DEBUG(logger) << "Action " << action_id << " registered for multiple destinations, not removed";
- return false;
- }
- }
-
- return RemoveActionByID(action_id);
-}
-
 void GnomeGrabber::Impl::ActivateDBusAction(CompAction const& action, uint32_t action_id, uint32_t device, uint32_t timestamp) const
 {
 LOG_DEBUG(logger) << "ActivateAction (" << action_id << " \"" << action.keyToString() << "\")";
diff --git a/unity-shared/GnomeKeyGrabberImpl.h b/unity-shared/GnomeKeyGrabberImpl.h
index 2c03827dd..50926e9a8 100644
--- a/unity-shared/GnomeKeyGrabberImpl.h
+++ b/unity-shared/GnomeKeyGrabberImpl.h
@@ -52,7 +52,6 @@ struct GnomeGrabber::Impl
 uint32_t GrabDBusAccelerator(std::string const& owner, std::string const& accelerator, uint32_t flags);
 bool UnGrabDBusAccelerator(std::string const& sender, uint32_t action_id);
 void ActivateDBusAction(CompAction const& action, uint32_t id, uint32_t device, uint32_t timestamp) const;
- bool RemoveActionForOwner(uint32_t action_id, std::string const& owner);
 
 bool IsActionPostponed(CompAction const& action) const;
 
@@ -63,6 +62,7 @@ struct GnomeGrabber::Impl
 
 uint32_t current_action_id_;
 std::vector<uint32_t> actions_ids_;
+ std::vector<uint32_t> actions_customers_;
 CompAction::Vector actions_;
 
 struct OwnerActions { glib::DBusNameWatcher::Ptr watcher; std::unordered_set<uint32_t> actions; };