GUI2: made widget initialization process more secure against memory leaks

src/gui/core/window_builder.cpp

@@ -137,7 +137,7 @@ builder_widget_ptr create_widget_builder(const config& cfg)
 	FAIL("Unknown widget type " + cfg.ordered_begin()->key);
 }
 
-widget* build_single_widget_instance_helper(const std::string& type, const config& cfg)
+std::unique_ptr<widget> build_single_widget_instance_helper(const std::string& type, const config& cfg)
 {
 	const auto& iter = widget_builder_lookup().find(type);
 	VALIDATE(iter != widget_builder_lookup().end(), "Invalid widget type '" + type + "'");
@@ -265,49 +265,21 @@ builder_grid::builder_grid(const config& cfg)
 	DBG_GUI_P << "Window builder: grid has " << rows << " rows and " << cols << " columns.\n";
 }
 
-grid* builder_grid::build() const
+std::unique_ptr<widget> builder_grid::build() const
 {
-	return build(new grid());
+	auto result = std::make_unique<grid>();
+	build(*result);
+	return result;
 }
 
-widget* builder_grid::build(const replacements_map& replacements) const
+std::unique_ptr<widget> builder_grid::build(const replacements_map& replacements) const
 {
-	grid* result = new grid();
+	auto result = std::make_unique<grid>();
 	build(*result, replacements);
 	return result;
 }
 
-grid* builder_grid::build(grid* grid) const
-{
-	grid->set_id(id);
-	grid->set_linked_group(linked_group);
-	grid->set_rows_cols(rows, cols);
-
-	log_scope2(log_gui_general, "Window builder: building grid");
-
-	DBG_GUI_G << "Window builder: grid '" << id << "' has " << rows << " rows and " << cols << " columns.\n";
-
-	for(unsigned x = 0; x < rows; ++x) {
-		grid->set_row_grow_factor(x, row_grow_factor[x]);
-
-		for(unsigned y = 0; y < cols; ++y) {
-			if(x == 0) {
-				grid->set_column_grow_factor(y, col_grow_factor[y]);
-			}
-
-			DBG_GUI_G << "Window builder: adding child at " << x << ',' << y << ".\n";
-
-			const unsigned int i = x * cols + y;
-
-			widget* widget = widgets[i]->build();
-			grid->set_child(widget, x, y, flags[i], border_size[i]);
-		}
-	}
-
-	return grid;
-}
-
-void builder_grid::build(grid& grid, const replacements_map& replacements) const
+void builder_grid::build(grid& grid, optional_replacements replacements) const
 {
 	grid.set_id(id);
 	grid.set_linked_group(linked_group);
@@ -328,7 +300,14 @@ void builder_grid::build(grid& grid, const replacements_map& replacements) const
 			DBG_GUI_G << "Window builder: adding child at " << x << ',' << y << ".\n";
 
 			const unsigned int i = x * cols + y;
-			grid.set_child(widgets[i]->build(replacements), x, y, flags[i], border_size[i]);
+
+			if(replacements) {
+				auto widget = widgets[i]->build(replacements.value());
+				grid.set_child(std::move(widget), x, y, flags[i], border_size[i]);
+			} else {
+				auto widget = widgets[i]->build();
+				grid.set_child(std::move(widget), x, y, flags[i], border_size[i]);
+			}
 		}
 	}
 }

src/gui/core/window_builder.hpp

@@ -19,6 +19,7 @@
 #include "gui/core/linked_group_definition.hpp"
 #include "gui/widgets/grid.hpp"
 #include "color.hpp"
+#include "utils/optional_reference.hpp"
 
 #include <functional>
 
@@ -32,7 +33,6 @@ class window;
 /** Contains the info needed to instantiate a widget. */
 struct builder_widget
 {
-public:
 	/**
 	 * The replacements type is used to define replacement types.
 	 *
@@ -41,17 +41,23 @@ struct builder_widget
 	 * using and `[instance]' widget this decision can be postponed until
 	 * instantiation.
 	 */
-	typedef std::map<std::string, std::shared_ptr<builder_widget>> replacements_map;
+	using replacements_map = std::map<std::string, std::shared_ptr<builder_widget>>;
+
+	/**
+	 * @todo: evaluate whether to combine the two @ref build() functions with this
+	 * as the sole parameter.
+	 */
+	using optional_replacements = utils::optional_reference<const replacements_map>;
 
 	explicit builder_widget(const config& cfg);
 
 	virtual ~builder_widget()
 	{
 	}
 
-	virtual widget* build() const = 0;
+	virtual std::unique_ptr<widget> build() const = 0;
 
-	virtual widget* build(const replacements_map& replacements) const = 0;
+	virtual std::unique_ptr<widget> build(const replacements_map& replacements) const = 0;
 
 	/** Parameters for the widget. */
 	std::string id;
@@ -61,8 +67,8 @@ struct builder_widget
 	color_t debug_border_color;
 };
 
-typedef std::shared_ptr<builder_widget> builder_widget_ptr;
-typedef std::shared_ptr<const builder_widget> builder_widget_const_ptr;
+using builder_widget_ptr = std::shared_ptr<builder_widget>;
+using builder_widget_const_ptr = std::shared_ptr<const builder_widget>;
 
 /**
  * Create a widget builder.
@@ -77,32 +83,41 @@ typedef std::shared_ptr<const builder_widget> builder_widget_const_ptr;
 builder_widget_ptr create_widget_builder(const config& cfg);
 
 /**
- * Helper function to implement @ref build_single_widget_instance. This keeps the main
- * logic in the implementation despite said function being a template and therefor
- * needing to be fully implemented in the declaration.
+ * Implementation detail for @ref build_single_widget_instance. Do not use directly!
+ *
+ * @param type                    String ID of the widget type.
+ * @param cfg                     Data config to pass to the widget's builder.
+ *
+ * @returns                       A shared_ptr of the base widget type containing
+ *                                the newly built widget.
  */
-widget* build_single_widget_instance_helper(const std::string& type, const config& cfg);
+std::unique_ptr<widget> build_single_widget_instance_helper(const std::string& type, const config& cfg);
 
 /**
  * Builds a single widget instance of the given type with the specified attributes.
  *
  * This should be used in place of creating a widget object directly, as it
  * allows the widget-specific builder code to be executed.
  *
+ * This is equivalent to calling @c build() on the result of @ref create_widget_builder.
+ *
  * @tparam T                      The final widget type. The widget pointer will be
  *                                cast to this.
  *
  * @param cfg                     Data config to pass to the widget's builder.
+ *
+ * @returns                       A shared_ptr of the given type containing the
+ *                                newly build widget.
  */
 template<typename T>
-T* build_single_widget_instance(const config& cfg = config())
+std::unique_ptr<T> build_single_widget_instance(const config& cfg = {})
 {
-	return dynamic_cast<T*>(build_single_widget_instance_helper(T::type(), cfg));
+	static_assert(std::is_base_of_v<widget, T>, "Type is not a widget type");
+	return std::unique_ptr<T>{ static_cast<T*>(build_single_widget_instance_helper(T::type(), cfg).release()) };
 }
 
 struct builder_grid : public builder_widget
 {
-public:
 	explicit builder_grid(const config& cfg);
 
 	unsigned rows;
@@ -121,15 +136,18 @@ struct builder_grid : public builder_widget
 	/** The widgets per grid cell. */
 	std::vector<builder_widget_ptr> widgets;
 
-	virtual grid* build() const override;
-	virtual widget* build(const replacements_map& replacements) const override;
+	/** Inherited from @ref builder_widget. */
+	virtual std::unique_ptr<widget> build() const override;
+
+	/** Inherited from @ref builder_widget. */
+	virtual std::unique_ptr<widget> build(const replacements_map& replacements) const override;
 
-	grid* build(grid* grid) const;
-	void build(grid& grid, const replacements_map& replacements) const;
+	void build(grid& grid, optional_replacements replacements = std::nullopt) const;
 };
 
-typedef std::shared_ptr<builder_grid> builder_grid_ptr;
-typedef std::shared_ptr<const builder_grid> builder_grid_const_ptr;
+using builder_grid_ptr = std::shared_ptr<builder_grid>;
+using builder_grid_const_ptr = std::shared_ptr<const builder_grid>;
+using builder_grid_map = std::map<std::string, builder_grid_const_ptr>;
 
 class builder_window
 {
@@ -179,7 +197,6 @@ class builder_window
 	 */
 	struct window_resolution
 	{
-	public:
 		explicit window_resolution(const config& cfg);
 
 		unsigned window_width;

src/gui/core/window_builder/instance.cpp

@@ -30,12 +30,12 @@ builder_instance::builder_instance(const config& cfg)
 {
 }
 
-widget* builder_instance::build() const
+std::unique_ptr<widget> builder_instance::build() const
 {
 	return build(replacements_map());
 }
 
-widget* builder_instance::build(const replacements_map& replacements) const
+std::unique_ptr<widget> builder_instance::build(const replacements_map& replacements) const
 {
 	const replacements_map::const_iterator itor = replacements.find(id);
 	if(itor != replacements.end()) {

src/gui/core/window_builder/instance.hpp

@@ -29,9 +29,9 @@ struct builder_instance : public builder_widget
 {
 	explicit builder_instance(const config& cfg);
 
-	virtual widget* build() const override;
+	virtual std::unique_ptr<widget> build() const override;
 
-	virtual widget* build(const replacements_map& replacements) const override;
+	virtual std::unique_ptr<widget> build(const replacements_map& replacements) const override;
 
 	/**
 	 * Holds a copy of the cfg parameter in the constructor.

src/gui/dialogs/drop_down_menu.cpp

@@ -196,7 +196,7 @@ void drop_down_menu::pre_show(window& window)
 		find_widget<toggle_panel>(&new_row, "panel", false).set_tooltip(entry.tooltip);
 
 		if(entry.checkbox) {
-			toggle_button* checkbox = build_single_widget_instance<toggle_button>();
+			auto checkbox = build_single_widget_instance<toggle_button>();
 			checkbox->set_id("checkbox");
 			checkbox->set_value_bool(*entry.checkbox);
 
@@ -207,14 +207,14 @@ void drop_down_menu::pre_show(window& window)
 				}));
 			}
 
-			mi_grid.swap_child("icon", checkbox, false);
+			mi_grid.swap_child("icon", std::move(checkbox), false);
 		}
 
 		if(entry.image) {
-			image* img = build_single_widget_instance<image>();
+			auto img = build_single_widget_instance<image>();
 			img->set_label(*entry.image);
 
-			mi_grid.swap_child("label", img, false);
+			mi_grid.swap_child("label", std::move(img), false);
 		}
 	}
 

src/gui/dialogs/preferences_dialog.cpp

@@ -643,13 +643,13 @@ void preferences_dialog::post_build(window& window)
 			}
 
 			case avp::avd_type::SLIDER: {
-				slider* setter_widget = build_single_widget_instance<slider>(config {"definition", "minimal"});
+				auto setter_widget = build_single_widget_instance<slider>(config {"definition", "minimal"});
 				setter_widget->set_id("setter");
 				// Maximum must be set first or this will assert
 				setter_widget->set_value_range(option.cfg["min"].to_int(), option.cfg["max"].to_int());
 				setter_widget->set_step_size(option.cfg["step"].to_int(1));
 
-				details_grid.swap_child("setter", setter_widget, true);
+				details_grid.swap_child("setter", std::move(setter_widget), true);
 
 				slider& slide = find_widget<slider>(&details_grid, "setter", false);
 
@@ -690,10 +690,10 @@ void preferences_dialog::post_build(window& window)
 					selected = 0;
 				}
 
-				menu_button* setter_widget = build_single_widget_instance<menu_button>();
+				auto setter_widget = build_single_widget_instance<menu_button>();
 				setter_widget->set_id("setter");
 
-				details_grid.swap_child("setter", setter_widget, true);
+				details_grid.swap_child("setter", std::move(setter_widget), true);
 
 				menu_button& menu = find_widget<menu_button>(&details_grid, "setter", false);
 
@@ -713,10 +713,10 @@ void preferences_dialog::post_build(window& window)
 			case avp::avd_type::SPECIAL: {
 				//main_grid->remove_child("setter");
 
-				image* value_widget = build_single_widget_instance<image>();
+				auto value_widget = build_single_widget_instance<image>();
 				value_widget->set_label("icons/arrows/arrows_blank_right_25.png~CROP(3,3,18,18)");
 
-				main_grid->swap_child("value", value_widget, true);
+				main_grid->swap_child("value", std::move(value_widget), true);
 
 				break;
 			}

src/gui/widgets/addon_list.cpp

@@ -463,9 +463,9 @@ builder_addon_list::builder_addon_list(const config& cfg)
 	}
 }
 
-widget* builder_addon_list::build() const
+std::unique_ptr<widget> builder_addon_list::build() const
 {
-	addon_list* widget = new addon_list(*this);
+	auto widget = std::make_unique<addon_list>(*this);
 
 	DBG_GUI_G << "Window builder: placed add-on list '" << id <<
 		"' with definition '" << definition << "'.\n";

src/gui/widgets/addon_list.hpp

@@ -217,7 +217,7 @@ struct builder_addon_list : public builder_styled_widget
 
 	using builder_styled_widget::build;
 
-	virtual widget* build() const override;
+	virtual std::unique_ptr<widget> build() const override;
 
 private:
 	widget::visibility install_status_visibility_;

src/gui/widgets/button.cpp

@@ -177,9 +177,9 @@ builder_button::builder_button(const config& cfg)
 {
 }
 
-widget* builder_button::build() const
+std::unique_ptr<widget> builder_button::build() const
 {
-	button* widget = new button(*this);
+	auto widget = std::make_unique<button>(*this);
 
 	widget->set_retval(get_retval(retval_id_, retval_, id));
 

src/gui/widgets/button.hpp

@@ -148,7 +148,7 @@ struct builder_button : public builder_styled_widget
 
 	using builder_styled_widget::build;
 
-	virtual widget* build() const override;
+	virtual std::unique_ptr<widget> build() const override;
 
 private:
 	std::string retval_id_;

src/gui/widgets/chatbox.cpp

@@ -671,9 +671,9 @@ builder_chatbox::builder_chatbox(const config& cfg)
 {
 }
 
-widget* builder_chatbox::build() const
+std::unique_ptr<widget> builder_chatbox::build() const
 {
-	chatbox* widget = new chatbox(*this);
+	auto widget = std::make_unique<chatbox>(*this);
 
 	DBG_GUI_G << "Window builder: placed unit preview pane '" << id
 			  << "' with definition '" << definition << "'.\n";

src/gui/widgets/chatbox.hpp

@@ -264,7 +264,7 @@ struct builder_chatbox : public builder_styled_widget
 
 	using builder_styled_widget::build;
 
-	virtual widget* build() const override;
+	virtual std::unique_ptr<widget> build() const override;
 
 private:
 };

src/gui/widgets/container_base.cpp

@@ -267,7 +267,7 @@ void container_base::init_grid(const builder_grid& grid_builder)
 
 	assert(grid_.get_rows() == 0 && grid_.get_cols() == 0);
 
-	grid_builder.build(&grid_);
+	grid_builder.build(grid_);
 }
 
 point container_base::border_space() const