Fix crashes in keyboard shortcuts by incorrect signals lifetime handling

src/app/commands/cmd_keyboard_shortcuts.cpp

@@ -30,9 +30,9 @@
 #include "ui/graphics.h"
 #include "ui/listitem.h"
 #include "ui/paint_event.h"
-#include "ui/size_hint_event.h"
 #include "ui/resize_event.h"
 #include "ui/separator.h"
+#include "ui/size_hint_event.h"
 
 #include "keyboard_shortcuts.xml.h"
 
@@ -46,6 +46,20 @@ using namespace skin;
 static int g_sep = 0;
 
 class KeyItem : public ListItem {
+
+  // Used to avoid deleting the Add/Change/Del buttons on
+  // kMouseLeaveMessage when a foreground window is popup on a signal
+  // generated by those same buttons.
+  struct LockButtons {
+    KeyItem* keyItem;
+    LockButtons(KeyItem* keyItem) : keyItem(keyItem) {
+      keyItem->m_lockButtons = true;
+    };
+    ~LockButtons() {
+      keyItem->m_lockButtons = false;
+    };
+  };
+
 public:
   KeyItem(const std::string& text, Key* key, AppMenuItem* menuitem, int level)
     : ListItem(text)
@@ -53,7 +67,8 @@ public:
     , m_keyOrig(key ? new Key(*key): NULL)
     , m_menuitem(menuitem)
     , m_level(level)
-    , m_hotAccel(-1) {
+    , m_hotAccel(-1)
+    , m_lockButtons(false) {
     gfx::Border border = this->border();
     border.top(0);
     border.bottom(0);
@@ -73,6 +88,7 @@ public:
 private:
 
   void onChangeAccel(int index) {
+    LockButtons lock(this);
     Accelerator origAccel = m_key->accels()[index];
     SelectAccelerator window(origAccel, m_key->keycontext());
     window.openWindowInForeground();
@@ -87,6 +103,7 @@ private:
   }
 
   void onDeleteAccel(int index) {
+    LockButtons lock(this);
     // We need to create a copy of the accelerator because
     // Key::disableAccel() will modify the accels() collection itself.
     ui::Accelerator accel = m_key->accels()[index];
@@ -103,6 +120,7 @@ private:
   }
 
   void onAddAccel() {
+    LockButtons lock(this);
     ui::Accelerator accel;
     SelectAccelerator window(accel, m_key ? m_key->keycontext(): KeyContext::Any);
     window.openWindowInForeground();
@@ -221,13 +239,15 @@ private:
             if (m_hotAccel != i) {
               m_hotAccel = i;
 
+              m_changeConn = base::Connection();
               m_changeButton.reset(new Button(""));
-              m_changeButton->Click.connect(base::Bind<void>(&KeyItem::onChangeAccel, this, i));
+              m_changeConn = m_changeButton->Click.connect(base::Bind<void>(&KeyItem::onChangeAccel, this, i));
               setup_mini_look(m_changeButton.get());
               addChild(m_changeButton.get());
 
+              m_deleteConn = base::Connection();
               m_deleteButton.reset(new Button(""));
-              m_deleteButton->Click.connect(base::Bind<void>(&KeyItem::onDeleteAccel, this, i));
+              m_deleteConn = m_deleteButton->Click.connect(base::Bind<void>(&KeyItem::onDeleteAccel, this, i));
               setup_mini_look(m_deleteButton.get());
               addChild(m_deleteButton.get());
 
@@ -251,8 +271,9 @@ private:
 
           if (i == 0 && !m_addButton &&
               (!m_menuitem || m_menuitem->getCommand())) {
+            m_addConn = base::Connection();
             m_addButton.reset(new Button(""));
-            m_addButton->Click.connect(base::Bind<void>(&KeyItem::onAddAccel, this));
+            m_addConn = m_addButton->Click.connect(base::Bind<void>(&KeyItem::onAddAccel, this));
             setup_mini_look(m_addButton.get());
             addChild(m_addButton.get());
 
@@ -273,9 +294,22 @@ private:
   }
 
   void destroyButtons() {
-    m_changeButton.reset();
-    m_deleteButton.reset();
-    m_addButton.reset();
+    m_changeConn = base::Connection();
+    m_deleteConn = base::Connection();
+    m_addConn = base::Connection();
+
+    if (!m_lockButtons) {
+      m_changeButton.reset();
+      m_deleteButton.reset();
+      m_addButton.reset();
+    }
+    // Just hide the buttons
+    else {
+      if (m_changeButton) m_changeButton->setVisible(false);
+      if (m_deleteButton) m_deleteButton->setVisible(false);
+      if (m_addButton) m_addButton->setVisible(false);
+    }
+
     m_hotAccel = -1;
   }
 
@@ -287,7 +321,11 @@ private:
   base::SharedPtr<ui::Button> m_changeButton;
   base::SharedPtr<ui::Button> m_deleteButton;
   base::SharedPtr<ui::Button> m_addButton;
+  base::ScopedConnection m_changeConn;
+  base::ScopedConnection m_deleteConn;
+  base::ScopedConnection m_addConn;
   int m_hotAccel;
+  bool m_lockButtons;
 };
 
 class KeyboardShortcutsWindow : public app::gen::KeyboardShortcuts {