GUI2: minor refactoring of window construction to guard against memory leaks

This ensures the window object is managed by a smart pointer from the moment of its
creation, instead of being passed around as a raw pointer first. If an exception were
thrown during window creation, it would have resulted in a memory leak.

* Moved window_ member assignment to build_window() in both modal_dialog and modeless_dialog.
* Added an assertion after window creation in modeless_dialog.
* Renamed the global build() function to build_window_impl() to avoid confusion with
  builder_widget::build.
This commit is contained in:
Charles Dang 2018-04-04 17:05:51 +11:00
parent f4fb2a6dcc
commit 464f7ce628
8 changed files with 43 additions and 42 deletions

View file

@ -46,11 +46,11 @@ namespace gui2
* be tuned. This page will describe what can be tuned.
*
*/
window* build(const builder_window::window_resolution* definition)
window_ptr_t build_window_impl(const builder_window::window_resolution* definition)
{
// We set the values from the definition since we can only determine the
// best size (if needed) after all widgets have been placed.
window* win = new window(definition);
window_ptr_t win = std::make_unique<window>(definition);
assert(win);
for(const auto& lg : definition->linked_groups) {
@ -75,15 +75,15 @@ window* build(const builder_window::window_resolution* definition)
win->init_grid(definition->grid);
}
win->add_to_keyboard_chain(win);
win->add_to_keyboard_chain(win.get());
return win;
}
window* build(const std::string& type)
window_ptr_t build_window_impl(const std::string& type)
{
const builder_window::window_resolution& definition = get_window_builder(type);
window* window = build(&definition);
window_ptr_t window = build_window_impl(&definition);
window->set_id(type);
return window;
}

View file

@ -14,28 +14,20 @@
#pragma once
#include "color.hpp"
#include "gui/auxiliary/typed_formula.hpp"
#include "gui/core/linked_group_definition.hpp"
#include "gui/widgets/grid.hpp"
#include "color.hpp"
#include "utils/functional.hpp"
#include <memory>
class config;
namespace gui2
{
class window;
/**
* Builds a window.
*
* @param type The type id string of the window, this window
* must be registered at startup.
*/
window* build(const std::string& type);
/** Contains the info needed to instantiate a widget. */
struct builder_widget
{
@ -211,9 +203,19 @@ private:
std::string description_;
};
using window_ptr_t = std::unique_ptr<window>;
/**
* Builds a window.
*/
window* build(const builder_window::window_resolution* res);
window_ptr_t build_window_impl(const builder_window::window_resolution* res);
/**
* Builds a window.
*
* @param type The type id string of the window, this window
* must be registered at startup.
*/
window_ptr_t build_window_impl(const std::string& type);
} // namespace gui2

View file

@ -72,8 +72,7 @@ bool modal_dialog::show(const unsigned auto_close_time)
return false;
}
window_.reset(build_window());
assert(window_.get());
build_window();
post_build(*window_);
@ -215,9 +214,10 @@ field_label* modal_dialog::register_label(const std::string& id,
return field;
}
window* modal_dialog::build_window() const
void modal_dialog::build_window()
{
return build(window_id());
window_ = build_window_impl(window_id());
assert(window_);
}
void modal_dialog::post_build(window& /*window*/)

View file

@ -322,7 +322,7 @@ protected:
protected:
/** The window object build for this dialog. */
std::unique_ptr<window> window_;
window_ptr_t window_;
private:
/** Returns the window exit status, 0 means not shown. */
@ -373,12 +373,10 @@ private:
/**
* Builds the window.
*
* Every dialog shows it's own kind of window, this function should return
* the window to show.
*
* @returns The window to show.
* Every dialog shows its own kind of window. This function handles the building
* of said window. @ref window_ should be non-null after this is called.
*/
window* build_window() const;
void build_window();
/**
* Actions to be taken directly after the window is build.

View file

@ -41,7 +41,7 @@ void modeless_dialog::show(const bool allow_interaction, const unsigned /*auto_c
hide();
window_.reset(build_window());
build_window();
post_build(*window_);
@ -66,9 +66,10 @@ void modeless_dialog::hide()
window_.reset(nullptr); }
}
window* modeless_dialog::build_window() const
void modeless_dialog::build_window()
{
return build(window_id());
window_ = build_window_impl(window_id());
assert(window_);
}
void modeless_dialog::post_build(window& /*window*/)

View file

@ -14,6 +14,8 @@
#pragma once
#include "gui/core/window_builder.hpp"
#include <memory>
#include <string>
@ -86,7 +88,7 @@ public:
protected:
/** The window, used in show. */
std::unique_ptr<window> window_;
window_ptr_t window_;
private:
/** The id of the window to build. */
@ -95,12 +97,10 @@ private:
/**
* Builds the window.
*
* Every dialog shows it's own kind of window, this function should return
* the window to show.
*
* @returns The window to show.
* Every dialog shows its own kind of window. This function handles the building
* of said window. @ref window_ should be non-null after this is called.
*/
window* build_window() const;
void build_window();
/**
* Actions to be taken directly after the window is build.

View file

@ -63,7 +63,7 @@ class distributor;
class window : public panel, public cursor::setter
{
friend class debug_layout_graph;
friend window* build(const builder_window::window_resolution*);
friend window_ptr_t build_window_impl(const builder_window::window_resolution*);
friend struct window_implementation;
friend class invalidate_layout_blocker;
friend class pane;

View file

@ -79,11 +79,11 @@ namespace {
lua_State* L;
scoped_dialog* prev;
static scoped_dialog* current;
std::unique_ptr<gui2::window> window;
gui2::window_ptr_t window;
typedef std::map<gui2::widget*, int> callback_map;
callback_map callbacks;
scoped_dialog(lua_State* l, gui2::window* w);
scoped_dialog(lua_State* l, gui2::window_ptr_t w);
~scoped_dialog();
private:
scoped_dialog(const scoped_dialog&) = delete;
@ -91,8 +91,8 @@ namespace {
scoped_dialog* scoped_dialog::current = nullptr;
scoped_dialog::scoped_dialog(lua_State* l, gui2::window* w)
: L(l), prev(current), window(w), callbacks()
scoped_dialog::scoped_dialog(lua_State* l, gui2::window_ptr_t w)
: L(l), prev(current), window(std::move(w)), callbacks()
{
lua_pushstring(L, dlgclbkKey);
lua_createtable(L, 1, 0);
@ -245,7 +245,7 @@ int show_dialog(lua_State* L)
config def_cfg = luaW_checkconfig(L, 1);
gui2::builder_window::window_resolution def(def_cfg);
scoped_dialog w(L, gui2::build(&def));
scoped_dialog w(L, gui2::build_window_impl(&def));
if(!lua_isnoneornil(L, 2)) {
lua_pushvalue(L, 2);