Fix segfault on shutdown

There was a problem deep within how the menus were connected and when and
what gets deleted. It was clearly related to a menu which was kind of
global. In order to better understand the code flow I eliminated the
ExtraMenu code: it was used only to get the Remember-Menu into the
Window-Menu. Instead of having a singleton of the Remember-Menu and fight
against the shaky interconnections we just create a new one on demand
and delete when the menu gets deleted. Looks like this fixes the problem.

The menu code needs more love anyway.

Closes #1118

src/MenuCreator.cc

@@ -56,6 +56,10 @@
 #include <iostream>
 #include <algorithm>
 
+#ifdef REMEMBER
+#include "Remember.hh"
+#endif // REMEMBER
+
 using std::cerr;
 using std::endl;
 using std::string;
@@ -88,6 +92,8 @@ enum {
 
     L_ALPHA,
 
+    L_REMEMBER,
+
     L_MENU_EXIT,
     L_MENU_ICONS,
 };
@@ -112,6 +118,8 @@ const FbTk::FbString& _l(const FbTk::FbString& label, size_t type) {
 
         _FB_XTEXT(Configmenu, Transparency, "Transparency", "Menu containing various transparency options"),
 
+        _FB_XTEXT(Remember, MenuItemName, "Remember...", "Remember item in menu"),
+
         _FB_XTEXT(Menu, Exit, "Exit", "Exit Command"),
         _FB_XTEXT(Menu, Icons, "Icons", "Iconic windows menu title"),
     };
@@ -602,14 +610,13 @@ bool MenuCreator::createWindowMenuItem(const string &type,
         }
 #endif // HAVE_XRENDER
     } else if (type == "extramenus") {
+#ifdef REMEMBER
         BScreen* s = Fluxbox::instance()->findScreen(screen);
-        BScreen::ExtraMenus::iterator it = s->extraWindowMenus().begin();
+        if (s == 0) {
-        BScreen::ExtraMenus::iterator it_end = s->extraWindowMenus().end();
+            return false;
-        for (; it != it_end; ++it) {
-            it->second->disableTitle();
-            menu.insertSubmenu(it->first, it->second);
         }
-
+        menu.insertSubmenu(_l("", L_REMEMBER), Remember::createMenu(*s));
+#endif
     } else if (type == "sendto") {
         menu.insertSubmenu(_l(label, L_SENDTO), 
             new SendToMenu(*Fluxbox::instance()->findScreen(screen)));
@@ -634,4 +641,3 @@ bool MenuCreator::createWindowMenuItem(const string &type,
     return true;
 }
 
-

src/Remember.cc

@@ -1442,10 +1442,10 @@ void Remember::updateClientClose(WinClient &winclient) {
 
 }
 
-void Remember::initForScreen(BScreen &screen) {
+void Remember::initForScreen(BScreen &screen) { }
-    // All windows get the remember menu.
-    _FB_USES_NLS;
-    screen.addExtraWindowMenu(_FB_XTEXT(Remember, MenuItemName, "Remember...", "Remember item in menu"),
-                              createRememberMenu(screen));
 
+FbTk::Menu* Remember::createMenu(BScreen& screen) {
+
+    return createRememberMenu(screen);
 }
+

src/Remember.hh

@@ -41,6 +41,7 @@ class Application;
 
 namespace FbTk {
 class AutoReloadHelper;
+class Menu;
 }
 
 /**
@@ -125,6 +126,8 @@ public:
 
     void initForScreen(BScreen &screen);
 
+    static FbTk::Menu* createMenu(BScreen& screen);
+
     // Functions we ignore (zero from AtomHandler)
     // Leaving here in case they might be useful later
 

src/Screen.cc

@@ -357,8 +357,8 @@ BScreen::BScreen(FbTk::ResourceManager &rm,
 
     m_configmenu.reset(MenuCreator::createMenu(_FB_XTEXT(Menu, Configuration,
                                   "Configuration", "Title of configuration menu"), *this));
-    setupConfigmenu(*m_configmenu.get());
     m_configmenu->setInternalMenu();
+    setupConfigmenu(*m_configmenu.get());
 
     // check which desktop we should start on
     int first_desktop = 0;
@@ -388,9 +388,7 @@ BScreen::~BScreen() {
 
     if (! managed)
         return;
-    
+
-    m_configmenu.reset(0);
-    
     m_toolbar.reset(0);
 
     FbTk::EventManager *evm = FbTk::EventManager::instance();
@@ -407,35 +405,6 @@ BScreen::~BScreen() {
     // we need to destroy it before we destroy workspaces
     m_workspacemenu.reset(0);
 
-    if (m_extramenus.size()) {
-        // check whether extramenus are included in windowmenu
-        // if not, we clean them ourselves
-        bool extramenus_in_windowmenu = false;
-        for (size_t i = 0, n = m_windowmenu->numberOfItems(); i < n; i++)
-            if (m_windowmenu->find(i)->submenu() == m_extramenus.begin()->second) {
-                extramenus_in_windowmenu = true;
-                break;
-            }
-
-        ExtraMenus::iterator mit = m_extramenus.begin();
-        ExtraMenus::iterator mit_end = m_extramenus.end();
-        for (; mit != mit_end; ++mit) {
-            // we set them to NOT internal so that they will be deleted when the
-            // menu is cleaned up. We can't delete them here because they are
-            // still in the menu
-            // (They need to be internal for most of the time so that if we
-            // rebuild the menu, then they won't be removed.
-            if (! extramenus_in_windowmenu) {
-                // not attached to our windowmenu
-                // so we clean it up
-                delete mit->second;
-            } else {
-                // let the parent clean it up
-                mit->second->setInternalMenu(false);
-            }
-        }
-    }
-
     removeWorkspaceNames();
     using namespace FbTk::STLUtil;
     destroyAndClear(m_workspaces_list);
@@ -461,9 +430,10 @@ BScreen::~BScreen() {
     // slit must be destroyed before headAreas (Struts)
     m_slit.reset(0);
 
-    delete m_rootmenu.release();
+    m_windowmenu.reset(0);
-    delete m_workspacemenu.release();
+    m_rootmenu.reset(0);
-    delete m_windowmenu.release();
+    m_workspacemenu.reset(0);
+    m_configmenu.reset(0);
 
     // TODO fluxgen: check if this is the right place
     for (size_t i = 0; i < m_head_areas.size(); i++)
@@ -471,6 +441,7 @@ BScreen::~BScreen() {
 
     delete m_focus_control;
     delete m_placement_strategy;
+
 }
 
 bool BScreen::isRestart() {
@@ -736,13 +707,6 @@ void BScreen::cycleFocus(int options, const ClientPattern *pat, bool reverse) {
 
 }
 
-void BScreen::addExtraWindowMenu(const FbTk::FbString &label, FbTk::Menu *menu) {
-    menu->setInternalMenu();
-    menu->disableTitle();
-    m_extramenus.push_back(make_pair(label, menu));
-    rereadWindowMenu();
-}
-
 void BScreen::reconfigure() {
     Fluxbox *fluxbox = Fluxbox::instance();
 
@@ -754,7 +718,7 @@ void BScreen::reconfigure() {
 
     m_menutheme->setDelay(*resource.menu_delay);
 
-    // realize the number of workspaces from the init-file
+    // provide the number of workspaces from the init-file
     const unsigned int nr_ws = *resource.workspaces;
     if (nr_ws > m_workspaces_list.size()) {
         while(nr_ws != m_workspaces_list.size()) {

src/Screen.hh

@@ -82,7 +82,6 @@ public:
 
     typedef std::vector<Workspace *> Workspaces;
     typedef std::vector<std::string> WorkspaceNames;
-    typedef std::list<std::pair<FbTk::FbString, FbTk::Menu *> > ExtraMenus;
 
     BScreen(FbTk::ResourceManager &rm,
             const std::string &screenname, const std::string &altscreenname,
@@ -113,8 +112,6 @@ public:
     FbMenu &configMenu() { return *m_configmenu.get(); }
     const FbMenu &windowMenu() const { return *m_windowmenu.get(); }
     FbMenu &windowMenu() { return *m_windowmenu.get(); }
-    ExtraMenus &extraWindowMenus() { return m_extramenus; }
-    const ExtraMenus &extraWindowMenus() const { return m_extramenus; }
 
     FbWinFrame::TabPlacement getTabPlacement() const { return *resource.tab_placement; }
 
@@ -480,8 +477,6 @@ private:
     std::auto_ptr<FbTk::ImageControl> m_image_control;
     std::auto_ptr<FbMenu> m_configmenu, m_rootmenu, m_workspacemenu, m_windowmenu;
 
-    ExtraMenus m_extramenus;
-
     Icons m_icon_list;
 
     std::auto_ptr<Slit> m_slit;