From e2dbdeeb2eb1dd1e2ff97499e894a86d47d9e3db Mon Sep 17 00:00:00 2001 From: Mathias Gumz Date: Wed, 21 Jan 2015 22:16:19 +0100 Subject: 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 | 20 +++++++++++++------- src/Remember.cc | 10 +++++----- src/Remember.hh | 3 +++ src/Screen.cc | 52 ++++++++-------------------------------------------- src/Screen.hh | 5 ----- 5 files changed, 29 insertions(+), 61 deletions(-) diff --git a/src/MenuCreator.cc b/src/MenuCreator.cc index a90e234..7f7445f 100644 --- a/src/MenuCreator.cc +++ b/src/MenuCreator.cc @@ -56,6 +56,10 @@ #include #include +#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(); - BScreen::ExtraMenus::iterator it_end = s->extraWindowMenus().end(); - for (; it != it_end; ++it) { - it->second->disableTitle(); - menu.insertSubmenu(it->first, it->second); + if (s == 0) { + return false; } - + 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; } - diff --git a/src/Remember.cc b/src/Remember.cc index f946ad0..19ea5f0 100644 --- a/src/Remember.cc +++ b/src/Remember.cc @@ -1442,10 +1442,10 @@ void Remember::updateClientClose(WinClient &winclient) { } -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)); +void Remember::initForScreen(BScreen &screen) { } + +FbTk::Menu* Remember::createMenu(BScreen& screen) { + return createRememberMenu(screen); } + diff --git a/src/Remember.hh b/src/Remember.hh index a1abf5a..a6a980e 100644 --- a/src/Remember.hh +++ b/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 diff --git a/src/Screen.cc b/src/Screen.cc index 1d81191..2ae63a1 100644 --- a/src/Screen.cc +++ b/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(); - delete m_workspacemenu.release(); - delete m_windowmenu.release(); + m_windowmenu.reset(0); + m_rootmenu.reset(0); + 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()) { diff --git a/src/Screen.hh b/src/Screen.hh index 771899f..c9a6ede 100644 --- a/src/Screen.hh +++ b/src/Screen.hh @@ -82,7 +82,6 @@ public: typedef std::vector Workspaces; typedef std::vector WorkspaceNames; - typedef std::list > 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 m_image_control; std::auto_ptr m_configmenu, m_rootmenu, m_workspacemenu, m_windowmenu; - ExtraMenus m_extramenus; - Icons m_icon_list; std::auto_ptr m_slit; -- cgit v0.11.2