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

[Marco Trevisan (Treviño)]

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.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:4037
http://jenkins.qa.ubuntu.com/job/unity-ci/1379/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-xenial-amd64-ci/31
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-xenial-armhf-ci/31
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-xenial-i386-ci/31

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-ci/1379/rebuild

review: Approve (continuous-integration)

[Andrea Azzarone (azzar1)]

+1

review: Approve

=== modified file 'unity-shared/GnomeKeyGrabber.cpp'
--- unity-shared/GnomeKeyGrabber.cpp	2015-10-27 17:35:44 +0000
+++ unity-shared/GnomeKeyGrabber.cpp	2015-12-13 10:39:16 +0000
@@ -98,7 +98,9 @@
   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 @@
   {
     actions_ids_.push_back(action_id);
     actions_.push_back(action);
+    actions_customers_.push_back(1);
     return true;
   }
 
@@ -148,12 +151,23 @@
   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 @@
         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 @@
     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 @@
   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() << "\")";
 
=== modified file 'unity-shared/GnomeKeyGrabberImpl.h'
--- unity-shared/GnomeKeyGrabberImpl.h	2015-10-01 03:52:09 +0000
+++ unity-shared/GnomeKeyGrabberImpl.h	2015-12-13 10:39:16 +0000
@@ -52,7 +52,6 @@
   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 @@
 
   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; };