Store menus if smart pointers (RefCount)

This was originally intended to be a bugfix for an memory error reported by valgrind (accessing
freed memory). While debugging it, I found the menu ownership semantics confusing
(setInternalMenu() et al.), so I decided to get rid of it and store it in smart pointers
everywhere.
Looking back, I'm not sure if this was worth all the trouble, but the good news is that the
valgrind error disappeared. :)

7 files changed, 15 insertions, 31 deletions

diff --git a/src/FbTk/Menu.cc b/src/FbTk/Menu.cc
index e52b4dd..ccd76d3 100644
--- a/src/FbTk/Menu.cc
+++ b/src/FbTk/Menu.cc
@@ -126,8 +126,7 @@ Menu::Menu(FbTk::ThemeProxy<MenuTheme> &tm, ImageControl &imgctrl):
 
     m_title_vis = true;
 
-    m_internal_menu =
-        m_moving =
+    m_moving =
         m_closing =
         m_torn =
         m_visible = false;
@@ -227,7 +226,7 @@ int Menu::insert(const FbString &label, int pos) {
     return insert(new MenuItem(label, *this), pos);
 }
 
-int Menu::insert(const FbString &label, Menu *submenu, int pos) {
+int Menu::insert(const FbString &label, const RefCount<Menu> &submenu, int pos) {
     return insert(new MenuItem(label, submenu, this), pos);
 }
 
@@ -274,19 +273,6 @@ int Menu::remove(unsigned int index) {
         if (index != menuitems.size())
             fixMenuItemIndices();
 
-        if (item->submenu() != 0) {
-            Menu *tmp = item->submenu();
-            // if menu is internal we should just hide it instead
-            // if destroying it
-            if (! tmp->m_internal_menu) {
-                delete tmp;
-            }
-            // We can't internal_hide here, as the child may be deleted!
-//            } else
-//                tmp->internal_hide();
-        }
-
-
         delete item;
     }
 
@@ -374,8 +360,8 @@ void Menu::enterSubmenu() {
     if (!validIndex(m_active_index))
         return;
 
-    Menu *submenu = menuitems[m_active_index]->submenu();
-    if (submenu == 0)
+    RefCount<Menu> submenu = menuitems[m_active_index]->submenu();
+    if (! submenu)
         return;
 
     if (submenu->menuitems.size() == 0)
@@ -759,7 +745,7 @@ void Menu::drawSubmenu(unsigned int index) {
             clearItem(index);
 
         if (! item->submenu()->isVisible() && item->submenu()->numberOfItems() > 0) {
-            shown = item->submenu();
+            shown = item->submenu().get();
             item->showSubmenu();
             item->submenu()->raise();
         }
diff --git a/src/FbTk/Menu.hh b/src/FbTk/Menu.hh
index 4096295..5807592 100644
--- a/src/FbTk/Menu.hh
+++ b/src/FbTk/Menu.hh
@@ -65,14 +65,13 @@ public:
     /// add empty menu item
     int insert(const FbString &label, int pos=-1);
     /// add submenu
-    int insert(const FbString &label, Menu *submenu, int pos= -1);
+    int insert(const FbString &label, const RefCount<Menu> &submenu, int pos= -1);
     /// add menu item
     int insert(MenuItem *item, int pos=-1);
     /// remove an item
     int remove(unsigned int item);
     /// remove all items
     void removeAll();
-    void setInternalMenu(bool val = true) { m_internal_menu = val; }
     void setAlignment(Alignment a) { m_alignment = a; }
 
     /// raise this window
@@ -213,7 +212,6 @@ private:
     bool m_closing; ///< if we're right clicking on the menu title
     bool m_visible; ///< menu visibility
     bool m_torn; ///< torn from parent
-    bool m_internal_menu; ///< whether we should destroy this menu or if it's managed somewhere else
     bool m_title_vis; ///< title visibility
 
     int m_which_sub;
diff --git a/src/FbTk/MenuItem.cc b/src/FbTk/MenuItem.cc
index c1357fb..36ff442 100644
--- a/src/FbTk/MenuItem.cc
+++ b/src/FbTk/MenuItem.cc
@@ -27,7 +27,6 @@
 #include "Image.hh"
 #include "App.hh"
 #include "StringUtil.hh"
-#include "Menu.hh"
 #include <X11/keysym.h>
 
 namespace FbTk {
diff --git a/src/FbTk/MenuItem.hh b/src/FbTk/MenuItem.hh
index cc24228..227488f 100644
--- a/src/FbTk/MenuItem.hh
+++ b/src/FbTk/MenuItem.hh
@@ -22,6 +22,7 @@
 #ifndef FBTK_MENUITEM_HH
 #define FBTK_MENUITEM_HH
 
+#include "Menu.hh"
 #include "RefCount.hh"
 #include "Command.hh"
 #include "PixmapWithMask.hh"
@@ -32,7 +33,6 @@
 
 namespace FbTk {
 
-class Menu;
 class MenuTheme;
 class FbDrawable;
 template <class T> class ThemeProxy;
@@ -80,7 +80,7 @@ public:
           m_toggle_item(false)
     { }
 
-    MenuItem(const BiDiString &label, Menu *submenu, Menu *host_menu = 0)
+    MenuItem(const BiDiString &label, const RefCount<Menu> &submenu, Menu *host_menu = 0)
         : m_label(label),
           m_menu(host_menu),
           m_submenu(submenu),
@@ -98,7 +98,7 @@ public:
     virtual void setToggleItem(bool val) { m_toggle_item = val; }
     void setCloseOnClick(bool val) { m_close_on_click = val; }
     void setIcon(const std::string &filename, int screen_num);
-    virtual Menu *submenu() { return m_submenu; }
+    virtual const RefCount<Menu>& submenu() { return m_submenu; }
     /**
         @name accessors
     */
@@ -107,7 +107,7 @@ public:
     virtual const PixmapWithMask *icon() const {
         return m_icon.get() ? m_icon->pixmap.get() : 0;
     }
-    virtual const Menu *submenu() const { return m_submenu; }
+    virtual RefCount<const Menu> submenu() const { return m_submenu; }
     virtual bool isEnabled() const { return m_enabled; }
     virtual bool isSelected() const { return m_selected; }
     virtual bool isToggleItem() const { return m_toggle_item; }
@@ -150,7 +150,7 @@ public:
 private:
     BiDiString m_label; ///< label of this item
     Menu *m_menu; ///< the menu we live in
-    Menu *m_submenu; ///< a submenu, 0 if we don't have one
+    RefCount<Menu> m_submenu; ///< a submenu, 0 if we don't have one
     RefCount<Command<void> > m_command; ///< command to be executed
     bool m_enabled, m_selected;
     bool m_close_on_click, m_toggle_item;
diff --git a/src/FbTk/MultiButtonMenuItem.cc b/src/FbTk/MultiButtonMenuItem.cc
index 1b61a99..b10ba07 100644
--- a/src/FbTk/MultiButtonMenuItem.cc
+++ b/src/FbTk/MultiButtonMenuItem.cc
@@ -33,7 +33,8 @@ MultiButtonMenuItem::MultiButtonMenuItem(int buttons, const FbTk::BiDiString &la
     init(buttons);
 }
 
-MultiButtonMenuItem::MultiButtonMenuItem(int buttons, const FbTk::BiDiString &label, Menu *submenu):
+MultiButtonMenuItem::MultiButtonMenuItem(int buttons, const FbTk::BiDiString &label,
+        const RefCount<Menu> &submenu):
     MenuItem(label, submenu),
     m_button_exe(0),
     m_buttons(buttons) {
diff --git a/src/FbTk/MultiButtonMenuItem.hh b/src/FbTk/MultiButtonMenuItem.hh
index 0a9c5e6..427302c 100644
--- a/src/FbTk/MultiButtonMenuItem.hh
+++ b/src/FbTk/MultiButtonMenuItem.hh
@@ -31,7 +31,7 @@ namespace FbTk {
 class MultiButtonMenuItem: public FbTk::MenuItem {
 public:
     MultiButtonMenuItem(int buttons, const FbTk::BiDiString& label);  
-    MultiButtonMenuItem(int buttons, const FbTk::BiDiString& label, Menu *submenu);
+    MultiButtonMenuItem(int buttons, const FbTk::BiDiString& label, const RefCount<Menu> &submenu);
     virtual ~MultiButtonMenuItem();
     /// sets command to specified button
     void setCommand(int button, FbTk::RefCount<FbTk::Command<void> > &cmd);
diff --git a/src/FbTk/RadioMenuItem.hh b/src/FbTk/RadioMenuItem.hh
index ddd2274..cc3f00a 100644
--- a/src/FbTk/RadioMenuItem.hh
+++ b/src/FbTk/RadioMenuItem.hh
@@ -47,7 +47,7 @@ public:
         setToggleItem(true);
     }
 
-    RadioMenuItem(const FbString &label, Menu *submenu, Menu *host_menu = 0):
+    RadioMenuItem(const FbString &label, const RefCount<Menu> &submenu, Menu *host_menu = 0):
         MenuItem(label, submenu, host_menu) {
         setToggleItem(true);
     }