From 4858a5103ed519481bf3524f7274caf9ed10b40d Mon Sep 17 00:00:00 2001 From: David Capello Date: Mon, 28 Mar 2022 16:02:27 -0300 Subject: [PATCH] Fix memory leaks in MainWindow Temporary/created subdocks must be deleted automatically, and children that are not part of the window hierarchy must be deleted explicitly now (using some std::unique_ptrs). --- src/app/ui/dock.cpp | 17 +++-- src/app/ui/dock.h | 5 +- src/app/ui/main_window.cpp | 140 ++++++++++++++++++------------------- src/app/ui/main_window.h | 45 ++++++------ 4 files changed, 108 insertions(+), 99 deletions(-) diff --git a/src/app/ui/dock.cpp b/src/app/ui/dock.cpp index de5fdde15..1cffa9f10 100644 --- a/src/app/ui/dock.cpp +++ b/src/app/ui/dock.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2021 Igara Studio S.A. +// Copyright (C) 2021-2022 Igara Studio S.A. // // This program is distributed under the terms of // the End-User License Agreement for Aseprite. @@ -88,19 +88,24 @@ Dock::Dock() initTheme(); } -void Dock::reset() +void Dock::resetDocks() { for (int i = 0; i < kSides; ++i) { auto child = m_sides[i]; if (!child) continue; else if (auto subdock = dynamic_cast(child)) { - subdock->reset(); + subdock->resetDocks(); + if (subdock->m_autoDelete) + delete subdock; } else if (auto tabs = dynamic_cast(child)) { for (auto child2 : tabs->children()) { - if (auto subdock2 = dynamic_cast(child2)) - subdock2->reset(); + if (auto subdock2 = dynamic_cast(child2)) { + subdock2->resetDocks(); + if (subdock2->m_autoDelete) + delete subdock2; + } } } m_sides[i] = nullptr; @@ -149,6 +154,7 @@ void Dock::dockRelativeTo(ui::Widget* relative, ASSERT(parent); Dock* subdock = new Dock; + subdock->m_autoDelete = true; parent->replaceChild(relative, subdock); subdock->dock(CENTER, relative); subdock->dock(side, widget, prefSize); @@ -204,6 +210,7 @@ Dock* Dock::subdock(int side) auto oldWidget = m_sides[i]; auto newSubdock = new Dock; + newSubdock->m_autoDelete = true; setSide(i, newSubdock); if (oldWidget) { diff --git a/src/app/ui/dock.h b/src/app/ui/dock.h index 231b45dd3..821e780b8 100644 --- a/src/app/ui/dock.h +++ b/src/app/ui/dock.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2021 Igara Studio S.A. +// Copyright (C) 2021-2022 Igara Studio S.A. // // This program is distributed under the terms of // the End-User License Agreement for Aseprite. @@ -33,7 +33,7 @@ public: Dock(); - void reset(); + void resetDocks(); // side = ui::LEFT, or ui::RIGHT, etc. void dock(int side, ui::Widget* widget, const gfx::Size& prefSize = gfx::Size()); @@ -73,6 +73,7 @@ private: std::array m_sides; std::array m_aligns; std::array m_sizes; + bool m_autoDelete = false; // Used to drag-and-drop sides. ui::Widget* m_capturedWidget = nullptr; diff --git a/src/app/ui/main_window.cpp b/src/app/ui/main_window.cpp index c6caa0a18..d9197273b 100644 --- a/src/app/ui/main_window.cpp +++ b/src/app/ui/main_window.cpp @@ -86,8 +86,10 @@ public: MainWindow::MainWindow() : ui::Window(ui::Window::DesktopWindow) + , m_tooltipManager(new TooltipManager) , m_dock(new Dock) , m_customizableDock(new Dock) + , m_layoutSelector(new LayoutSelector) , m_mode(NormalMode) , m_homeView(nullptr) , m_scalePanic(nullptr) @@ -110,9 +112,8 @@ MainWindow::MainWindow() // Refer to https://github.com/aseprite/aseprite/issues/3914 void MainWindow::initialize() { - m_tooltipManager = new TooltipManager; - m_menuBar = new MainMenuBar; - m_layoutSelector = new LayoutSelector; + m_menuBar = std::make_unique(); + m_layoutSelector = std::make_unique(); // Register commands to load menus+shortcuts for these commands Editor::registerCommands(); @@ -123,20 +124,20 @@ void MainWindow::initialize() // Setup the main menubar m_menuBar->setMenu(AppMenus::instance()->getRootMenu()); - m_notifications = new Notifications(); - m_statusBar = new StatusBar(m_tooltipManager); - m_toolBar = new ToolBar(); - m_tabsBar = new WorkspaceTabs(this); - m_workspace = new Workspace(); - m_previewEditor = new PreviewEditorWindow(); - m_colorBar = new ColorBar(m_tooltipManager); - m_contextBar = new ContextBar(m_tooltipManager, m_colorBar); + m_notifications = std::make_unique(); + m_statusBar = std::make_unique(m_tooltipManager); + m_toolBar = std::make_unique(); + m_tabsBar = std::make_unique(this); + m_workspace = std::make_unique(); + m_previewEditor = std::make_unique(); + m_colorBar = std::make_unique(m_tooltipManager); + m_contextBar = std::make_unique(m_tooltipManager, m_colorBar.get()); // The timeline (AniControls) tooltips will use the keyboard // shortcuts loaded above. - m_timeline = new Timeline(m_tooltipManager); + m_timeline = std::make_unique(m_tooltipManager); - m_workspace->setTabsBar(m_tabsBar); + m_workspace->setTabsBar(m_tabsBar.get()); m_workspace->BeforeViewChanged.connect(&MainWindow::onBeforeViewChange, this); m_workspace->ActiveViewChanged.connect(&MainWindow::onActiveViewChange, this); @@ -156,20 +157,20 @@ void MainWindow::initialize() addChild(m_tooltipManager); addChild(m_dock); - auto customizableDockPlaceholder = new Widget; - customizableDockPlaceholder->addChild(m_customizableDock); - customizableDockPlaceholder->InitTheme.connect([this] { + m_customizableDockPlaceholder = std::make_unique(); + m_customizableDockPlaceholder->addChild(m_customizableDock); + m_customizableDockPlaceholder->InitTheme.connect([this] { auto theme = static_cast(this->theme()); m_customizableDock->setBgColor(theme->colors.workspace()); }); - customizableDockPlaceholder->initTheme(); + m_customizableDockPlaceholder->initTheme(); - m_dock->top()->right()->dock(ui::RIGHT, m_notifications); - m_dock->top()->right()->dock(ui::CENTER, m_layoutSelector); - m_dock->top()->dock(ui::BOTTOM, m_tabsBar); - m_dock->top()->dock(ui::CENTER, m_menuBar); - m_dock->dock(ui::CENTER, customizableDockPlaceholder); - m_dock->dock(ui::BOTTOM, m_statusBar); + m_dock->top()->right()->dock(ui::RIGHT, m_notifications.get()); + m_dock->top()->right()->dock(ui::CENTER, m_layoutSelector.get()); + m_dock->top()->dock(ui::BOTTOM, m_tabsBar.get()); + m_dock->top()->dock(ui::CENTER, m_menuBar.get()); + m_dock->dock(ui::CENTER, m_customizableDockPlaceholder.get()); + m_dock->dock(ui::BOTTOM, m_statusBar.get()); setDefaultLayout(); @@ -190,45 +191,43 @@ void MainWindow::initialize() MainWindow::~MainWindow() { - m_dock->reset(); - m_customizableDock->reset(); + m_dock->resetDocks(); + m_customizableDock->resetDocks(); - delete m_scalePanic; + m_layoutSelector.reset(); + m_scalePanic.reset(); #ifdef ENABLE_SCRIPTING if (m_devConsoleView) { if (m_devConsoleView->parent() && m_workspace) - m_workspace->removeView(m_devConsoleView); - delete m_devConsoleView; + m_workspace->removeView(m_devConsoleView.get()); + m_devConsoleView.reset(); } #endif if (m_browserView) { if (m_browserView->parent() && m_workspace) - m_workspace->removeView(m_browserView); - delete m_browserView; + m_workspace->removeView(m_browserView.get()); + m_browserView.reset(); } if (m_homeView) { if (m_homeView->parent() && m_workspace) - m_workspace->removeView(m_homeView); - delete m_homeView; + m_workspace->removeView(m_homeView.get()); + m_homeView.reset(); } - if (m_contextBar) - delete m_contextBar; - if (m_previewEditor) - delete m_previewEditor; + m_contextBar.reset(); + m_previewEditor.reset(); // Destroy the workspace first so ~Editor can dettach slots from // ColorBar. TODO this is a terrible hack for slot/signal stuff, // connections should be handle in a better/safer way. - if (m_workspace) - delete m_workspace; + m_workspace.reset(); // Remove the root-menu from the menu-bar (because the rootmenu // module should destroy it). if (m_menuBar) - m_menuBar->setMenu(NULL); + m_menuBar->setMenu(nullptr); } void MainWindow::onLanguageChange() @@ -246,8 +245,8 @@ DocView* MainWindow::getDocView() HomeView* MainWindow::getHomeView() { if (!m_homeView) - m_homeView = new HomeView; - return m_homeView; + m_homeView = std::make_unique(); + return m_homeView.get(); } #ifdef ENABLE_UPDATER @@ -284,20 +283,20 @@ void MainWindow::showHomeOnOpen() // Show "Home" tab in the first position, and select it only if // there is no other view selected. - m_workspace->addView(m_homeView, 0); + m_workspace->addView(m_homeView.get(), 0); if (selectedTab) m_tabsBar->selectTab(selectedTab); else - m_tabsBar->selectTab(m_homeView); + m_tabsBar->selectTab(m_homeView.get()); } } void MainWindow::showHome() { if (!getHomeView()->parent()) { - m_workspace->addView(m_homeView, 0); + m_workspace->addView(m_homeView.get(), 0); } - m_tabsBar->selectTab(m_homeView); + m_tabsBar->selectTab(m_homeView.get()); } void MainWindow::showDefaultStatusBar() @@ -312,19 +311,19 @@ void MainWindow::showDefaultStatusBar() bool MainWindow::isHomeSelected() const { - return (m_homeView && m_workspace->activeView() == m_homeView); + return (m_homeView && m_workspace->activeView() == m_homeView.get()); } void MainWindow::showBrowser(const std::string& filename, const std::string& section) { if (!m_browserView) - m_browserView = new BrowserView; + m_browserView = std::make_unique(); m_browserView->loadFile(filename, section); if (!m_browserView->parent()) { - m_workspace->addView(m_browserView); - m_tabsBar->selectTab(m_browserView); + m_workspace->addView(m_browserView.get()); + m_tabsBar->selectTab(m_browserView.get()); } } @@ -332,11 +331,11 @@ void MainWindow::showDevConsole() { #ifdef ENABLE_SCRIPTING if (!m_devConsoleView) - m_devConsoleView = new DevConsoleView; + m_devConsoleView = std::make_unique(); if (!m_devConsoleView->parent()) { - m_workspace->addView(m_devConsoleView); - m_tabsBar->selectTab(m_devConsoleView); + m_workspace->addView(m_devConsoleView.get()); + m_tabsBar->selectTab(m_devConsoleView.get()); } #endif } @@ -376,28 +375,28 @@ void MainWindow::popTimeline() void MainWindow::setDefaultLayout() { - m_customizableDock->reset(); - m_customizableDock->dock(ui::LEFT, m_colorBar); - m_customizableDock->center()->dock(ui::TOP, m_contextBar); - m_customizableDock->center()->dock(ui::RIGHT, m_toolBar); + m_customizableDock->resetDocks(); + m_customizableDock->dock(ui::LEFT, m_colorBar.get()); + m_customizableDock->center()->dock(ui::TOP, m_contextBar.get()); + m_customizableDock->center()->dock(ui::RIGHT, m_toolBar.get()); m_customizableDock->center()->center()->dock(ui::BOTTOM, - m_timeline, + m_timeline.get(), gfx::Size(64 * guiscale(), 64 * guiscale())); - m_customizableDock->center()->center()->dock(ui::CENTER, m_workspace); + m_customizableDock->center()->center()->dock(ui::CENTER, m_workspace.get()); layout(); } void MainWindow::setDefaultMirrorLayout() { - m_customizableDock->reset(); - m_customizableDock->dock(ui::RIGHT, m_colorBar); - m_customizableDock->center()->dock(ui::TOP, m_contextBar); - m_customizableDock->center()->dock(ui::LEFT, m_toolBar); + m_customizableDock->resetDocks(); + m_customizableDock->dock(ui::RIGHT, m_colorBar.get()); + m_customizableDock->center()->dock(ui::TOP, m_contextBar.get()); + m_customizableDock->center()->dock(ui::LEFT, m_toolBar.get()); m_customizableDock->center()->center()->dock(ui::BOTTOM, - m_timeline, + m_timeline.get(), gfx::Size(64 * guiscale(), 64 * guiscale())); - m_customizableDock->center()->center()->dock(ui::CENTER, m_workspace); + m_customizableDock->center()->center()->dock(ui::CENTER, m_workspace.get()); layout(); } @@ -438,7 +437,8 @@ void MainWindow::onResize(ui::ResizeEvent& ev) if ((scale > 2) && (!m_scalePanic)) { const gfx::Size wa = nativeWindow->screen()->workarea().size(); if ((wa.w / scale < 256 || wa.h / scale < 256)) { - showNotification(m_scalePanic = new ScreenScalePanic); + m_scalePanic = std::make_unique(); + showNotification(m_scalePanic.get()); } } } @@ -541,7 +541,7 @@ void MainWindow::onContextMenuTab(Tabs* tabs, TabView* tabView) WorkspaceView* view = dynamic_cast(tabView); ASSERT(view); if (view) - view->onTabPopup(m_workspace); + view->onTabPopup(m_workspace.get()); } void MainWindow::onTabsContainerDoubleClicked(Tabs* tabs) @@ -627,11 +627,11 @@ void MainWindow::configureWorkspaceLayout() if (os::System::instance()->menus() == nullptr || pref.general.showMenuBar()) { if (!m_menuBar->parent()) - m_dock->top()->dock(CENTER, m_menuBar); + m_dock->top()->dock(CENTER, m_menuBar.get()); } else { if (m_menuBar->parent()) - m_dock->undock(m_menuBar); + m_dock->undock(m_menuBar.get()); } m_menuBar->setVisible(normal); @@ -650,7 +650,7 @@ void MainWindow::configureWorkspaceLayout() auto timelinePosition = pref.general.timelinePosition(); int side = ui::BOTTOM; - m_customizableDock->undock(m_timeline); + m_customizableDock->undock(m_timeline.get()); switch (timelinePosition) { case gen::TimelinePosition::LEFT: side = ui::LEFT; break; @@ -659,7 +659,7 @@ void MainWindow::configureWorkspaceLayout() } m_customizableDock->center()->center()->dock(side, - m_timeline, + m_timeline.get(), gfx::Size(64 * guiscale(), 64 * guiscale())); m_timeline->setVisible(isDoc && (m_mode == NormalMode || m_mode == ContextBarAndTimelineMode) && diff --git a/src/app/ui/main_window.h b/src/app/ui/main_window.h index 39be6ab90..5b492397d 100644 --- a/src/app/ui/main_window.h +++ b/src/app/ui/main_window.h @@ -55,13 +55,13 @@ public: MainWindow(); ~MainWindow(); - MainMenuBar* getMenuBar() { return m_menuBar; } - ContextBar* getContextBar() { return m_contextBar; } - StatusBar* statusBar() { return m_statusBar; } - WorkspaceTabs* getTabsBar() { return m_tabsBar; } - Timeline* getTimeline() { return m_timeline; } - Workspace* getWorkspace() { return m_workspace; } - PreviewEditorWindow* getPreviewEditor() { return m_previewEditor; } + MainMenuBar* getMenuBar() { return m_menuBar.get(); } + ContextBar* getContextBar() { return m_contextBar.get(); } + StatusBar* statusBar() { return m_statusBar.get(); } + WorkspaceTabs* getTabsBar() { return m_tabsBar.get(); } + Timeline* getTimeline() { return m_timeline.get(); } + Workspace* getWorkspace() { return m_workspace.get(); } + PreviewEditorWindow* getPreviewEditor() { return m_previewEditor.get(); } #ifdef ENABLE_UPDATER CheckUpdateDelegate* getCheckUpdateDelegate(); #endif @@ -130,23 +130,24 @@ private: ui::TooltipManager* m_tooltipManager; Dock* m_dock; Dock* m_customizableDock; - MainMenuBar* m_menuBar; - LayoutSelector* m_layoutSelector; - StatusBar* m_statusBar; - ColorBar* m_colorBar; - ContextBar* m_contextBar; - ToolBar* m_toolBar; - WorkspaceTabs* m_tabsBar; + std::unique_ptr m_customizableDockPlaceholder; + std::unique_ptr m_menuBar; + std::unique_ptr m_layoutSelector; + std::unique_ptr m_statusBar; + std::unique_ptr m_colorBar; + std::unique_ptr m_contextBar; + std::unique_ptr m_toolBar; + std::unique_ptr m_tabsBar; Mode m_mode; - Timeline* m_timeline; - Workspace* m_workspace; - PreviewEditorWindow* m_previewEditor; - HomeView* m_homeView; - Notifications* m_notifications; - INotificationDelegate* m_scalePanic; - BrowserView* m_browserView; + std::unique_ptr m_timeline; + std::unique_ptr m_workspace; + std::unique_ptr m_previewEditor; + std::unique_ptr m_homeView; + std::unique_ptr m_notifications; + std::unique_ptr m_scalePanic; + std::unique_ptr m_browserView; #ifdef ENABLE_SCRIPTING - DevConsoleView* m_devConsoleView; + std::unique_ptr m_devConsoleView; #endif };