Remove ReferenceCountable from Context and ContextInstancer

Include/RmlUi/Core/Context.h

@@ -53,7 +53,7 @@ enum class EventId : uint16_t;
 	@author Peter Curry
  */
 
-class RMLUICORE_API Context : public ReferenceCountable
+class RMLUICORE_API Context : public Releasable
 {
 public:
 	/// Constructs a new, uninitialised context. This should not be called directly, use Core::CreateContext()
@@ -219,17 +219,17 @@ public:
 
 	/// Sets the instancer to use for releasing this object.
 	/// @param[in] instancer The context's instancer.
-	void SetInstancer(ContextInstancer* instancer);
+	void SetInstancer(SharedPtr<ContextInstancer> instancer);
 
 protected:
-	virtual void OnReferenceDeactivate();
+	void Release() override;
 
 private:
 	String name;
 	Vector2i dimensions;
 	float density_independent_pixel_ratio;
 
-	ContextInstancer* instancer;
+	SharedPtr<ContextInstancer> instancer;
 
 	typedef SmallOrderedSet< ElementReference > ElementSet;
 	typedef std::vector< ElementReference > ElementList;

Include/RmlUi/Core/ContextInstancer.h

@@ -44,7 +44,7 @@ class Event;
 	@author Lloyd Weehuizen
  */
 
-class RMLUICORE_API ContextInstancer : public ReferenceCountable
+class RMLUICORE_API ContextInstancer : public Releasable
 {
 public:
 	virtual ~ContextInstancer();
@@ -52,17 +52,15 @@ public:
 	/// Instances a context.
 	/// @param[in] name Name of this context.
 	/// @return The instanced context.
-	virtual Context* InstanceContext(const String& name) = 0;
+	virtual UniquePtr<Context> InstanceContext(const String& name) = 0;
 
 	/// Releases a context previously created by this context.
 	/// @param[in] context The context to release.
 	virtual void ReleaseContext(Context* context) = 0;
 
+protected:
 	/// Releases this context instancer
 	virtual void Release() = 0;
-
-private:
-	virtual void OnReferenceDeactivate();
 };
 
 }

Include/RmlUi/Core/Core.h

@@ -132,7 +132,11 @@ RMLUICORE_API FileInterface* GetFileInterface();
 /// @param[in] dimensions The initial dimensions of the new context.
 /// @param[in] render_interface The custom render interface to use, or NULL to use the default.
 /// @return The new context, or NULL if the context could not be created.
-RMLUICORE_API Context* CreateContext(const String& name, const Vector2i& dimensions, RenderInterface* render_interface = NULL);
+RMLUICORE_API Context* CreateContext(const String& name, const Vector2i& dimensions, RenderInterface* render_interface = nullptr);
+/// Removes a context.
+/// @param[in] name The name of the context to remove.
+/// @return True if name is a valid context, false otherwise.
+RMLUICORE_API bool RemoveContext(const String& name);
 /// Fetches a previously constructed context by name.
 /// @param[in] name The name of the desired context.
 /// @return The desired context, or NULL if no context exists with the given name.

Include/RmlUi/Core/Factory.h

@@ -73,11 +73,11 @@ public:
 
 	/// Registers the instancer to use when instancing contexts.
 	/// @param[in] instancer The new context instancer.
-	static ContextInstancer* RegisterContextInstancer(ContextInstancer* instancer);
+	static ContextInstancer* RegisterContextInstancer(SharedPtr<ContextInstancer> instancer);
 	/// Instances a new context.
 	/// @param[in] name The name of the new context.
 	/// @return The new context, or NULL if no context could be created.
-	static Context* InstanceContext(const String& name);
+	static UniquePtr<Context> InstanceContext(const String& name);
 
 	/// Registers an element instancer that will be used to instance an element when the specified tag is encountered.
 	/// @param[in] name Name of the instancer; elements with this as their tag will use this instancer.

Include/RmlUi/Core/ReferenceCountable.h

@@ -65,6 +65,7 @@ template<typename T>
 class RMLUICORE_API Releaser final : public ReleaserBase {
 public:
 	void operator()(T* target) const {
+		static_assert(std::is_base_of<Releasable, T>::value, "Releaser can only operate with classes derived from Releasable.");
 		Release(static_cast<Releasable*>(target));
 	}
 };

Include/RmlUi/Core/Types.h

@@ -98,10 +98,15 @@ typedef Matrix4< float, ColumnMajorStorage< float > > ColumnMajorMatrix4f;
 typedef Matrix4< float, RowMajorStorage< float > > RowMajorMatrix4f;
 typedef ColumnMajorMatrix4f Matrix4f;
 
+template<typename T>
+using UniquePtr = std::unique_ptr<T, Releaser<T>>;
+template<typename T>
+using SharedPtr = std::shared_ptr<T>;
+
 class Element;
 class ElementInstancer;
-using ElementPtr = std::unique_ptr<Element, Releaser<Element>>;
-using ElementInstancerPtr = std::unique_ptr<ElementInstancer, Releaser<ElementInstancer>>;
+using ElementPtr = UniquePtr<Element>;
+using ElementInstancerPtr = UniquePtr<ElementInstancer>;
 class ElementAnimation;
 class Property;
 class Variant;

Samples/basic/animation/src/main.cpp

@@ -377,7 +377,6 @@ int main(int RMLUI_UNUSED_PARAMETER(argc), char** RMLUI_UNUSED_PARAMETER(argv))
 	delete window;
 
 	// Shutdown RmlUi.
-	context->RemoveReference();
 	Rml::Core::Shutdown();
 
 	Shell::CloseWindow();

Samples/basic/benchmark/src/main.cpp

@@ -338,7 +338,6 @@ int main(int RMLUI_UNUSED_PARAMETER(argc), char** RMLUI_UNUSED_PARAMETER(argv))
 	delete window;
 
 	// Shutdown RmlUi.
-	context->RemoveReference();
 	Rml::Core::Shutdown();
 
 	Shell::CloseWindow();

Samples/basic/bitmapfont/src/main.cpp

@@ -112,7 +112,6 @@ int main(int RMLUI_UNUSED_PARAMETER(argc), char** RMLUI_UNUSED_PARAMETER(argv))
 	Shell::EventLoop(GameLoop);
 
 	// Shutdown RmlUi.
-	context->RemoveReference();
 	Rml::Core::Shutdown();
 
 	Shell::CloseWindow();

Samples/basic/customlog/src/main.cpp

@@ -125,7 +125,6 @@ int main(int RMLUI_UNUSED_PARAMETER(argc), char** RMLUI_UNUSED_PARAMETER(argv))
 	Shell::EventLoop(GameLoop);
 
 	// Shutdown RmlUi.
-	context->RemoveReference();
 	Rml::Core::Shutdown();
 
 	Shell::CloseWindow();

Samples/basic/drag/src/main.cpp

@@ -127,7 +127,6 @@ int main(int RMLUI_UNUSED_PARAMETER(argc), char** RMLUI_UNUSED_PARAMETER(argv))
 	delete inventory_2;
 
 	// Shutdown RmlUi.
-	context->RemoveReference();
 	Rml::Core::Shutdown();
 
 	Shell::CloseWindow();

Samples/basic/loaddocument/src/main.cpp

@@ -111,7 +111,6 @@ int main(int RMLUI_UNUSED_PARAMETER(argc), char** RMLUI_UNUSED_PARAMETER(argv))
 	Shell::EventLoop(GameLoop);
 
 	// Shutdown RmlUi.
-	context->RemoveReference();
 	Rml::Core::Shutdown();
 
 	Shell::CloseWindow();

Samples/basic/transform/src/main.cpp

@@ -188,7 +188,6 @@ int main(int RMLUI_UNUSED_PARAMETER(argc), char** RMLUI_UNUSED_PARAMETER(argv))
 	delete window_2;
 
 	// Shutdown RmlUi.
-	context->RemoveReference();
 	Rml::Core::Shutdown();
 
 	Shell::CloseWindow();

Samples/basic/treeview/src/main.cpp

@@ -124,7 +124,6 @@ int main(int RMLUI_UNUSED_PARAMETER(argc), char** RMLUI_UNUSED_PARAMETER(argv))
 	Shell::EventLoop(GameLoop);
 
 	// Shutdown RmlUi.
-	context->RemoveReference();
 	Rml::Core::Shutdown();
 
 	Shell::CloseWindow();

Samples/invaders/src/main.cpp

@@ -154,7 +154,6 @@ int main(int RMLUI_UNUSED_PARAMETER(argc), char** RMLUI_UNUSED_PARAMETER(argv))
 	EventManager::Shutdown();
 
 	// Shutdown RmlUi.
-	context->RemoveReference();
 	Rml::Core::Shutdown();
 
 	Shell::CloseWindow();

Samples/tutorial/datagrid/src/main.cpp

@@ -108,7 +108,6 @@ int main(int RMLUI_UNUSED_PARAMETER(argc), char** RMLUI_UNUSED_PARAMETER(argv))
 	HighScores::Shutdown();
 
 	// Shutdown RmlUi.
-	context->RemoveReference();
 	Rml::Core::Shutdown();
 
 	Shell::CloseWindow();

Samples/tutorial/datagrid_tree/src/main.cpp

@@ -115,7 +115,6 @@ int main(int RMLUI_UNUSED_PARAMETER(argc), char** RMLUI_UNUSED_PARAMETER(argv))
 	HighScores::Shutdown();
 
 	// Shutdown RmlUi.
-	context->RemoveReference();
 	Rml::Core::Shutdown();
 
 	Shell::CloseWindow();

Samples/tutorial/drag/src/main.cpp

@@ -104,7 +104,6 @@ int main(int RMLUI_UNUSED_PARAMETER(argc), char** RMLUI_UNUSED_PARAMETER(argv))
 	delete inventory_2;
 
 	// Shutdown RmlUi.
-	context->RemoveReference();
 	Rml::Core::Shutdown();
 
 	Shell::CloseWindow();

Samples/tutorial/template/src/main.cpp

@@ -94,7 +94,6 @@ int main(int RMLUI_UNUSED_PARAMETER(argc), char** RMLUI_UNUSED_PARAMETER(argv))
 	Shell::EventLoop(GameLoop);
 
 	// Shutdown RmlUi.
-	context->RemoveReference();
 	Rml::Core::Shutdown();
 
 	Shell::CloseWindow();

Source/Core/Context.cpp

@@ -92,8 +92,7 @@ Context::~Context()
 
 	root.reset();
 
-	if (instancer)
-		instancer->RemoveReference();
+	instancer.reset();
 
 	if (render_interface)
 		render_interface->RemoveReference();
@@ -801,11 +800,10 @@ void Context::SetActiveClipRegion(const Vector2i& origin, const Vector2i& dimens
 }
 
 // Sets the instancer to use for releasing this object.
-void Context::SetInstancer(ContextInstancer* _instancer)
+void Context::SetInstancer(SharedPtr<ContextInstancer> _instancer)
 {
 	RMLUI_ASSERT(instancer == nullptr);
-	instancer = _instancer;
-	instancer->AddReference();	
+	instancer = std::move(_instancer);
 }
 
 // Internal callback for when an element is removed from the hierarchy.
@@ -1194,9 +1192,9 @@ void Context::SendEvents(const ElementSet& old_items, const ElementSet& new_item
 	std::for_each(elements.begin(), elements.end(), func);
 }
 
-void Context::OnReferenceDeactivate()
+void Context::Release()
 {
-	if (instancer != nullptr)
+	if (instancer)
 	{
 		instancer->ReleaseContext(this);
 	}

Source/Core/ContextInstancer.cpp

@@ -36,10 +36,5 @@ ContextInstancer::~ContextInstancer()
 {
 }
 
-void ContextInstancer::OnReferenceDeactivate()
-{
-	Release();
-}
-
 }
 }

Source/Core/ContextInstancerDefault.cpp

@@ -41,10 +41,9 @@ ContextInstancerDefault::~ContextInstancerDefault()
 {
 }
 
-Context* ContextInstancerDefault::InstanceContext(const String& name)
+UniquePtr<Context> ContextInstancerDefault::InstanceContext(const String& name)
 {
-	Context* new_context = new Context(name);
-	return new_context;
+	return UniquePtr<Context>(new Context(name));
 }
 
 // Releases a context previously created by this context.

Source/Core/ContextInstancerDefault.h

@@ -49,14 +49,14 @@ public:
 	/// Instances a context.
 	/// @param[in] name Name of this context.
 	/// @return The instanced context.
-	virtual Context* InstanceContext(const String& name);
+	UniquePtr<Context> InstanceContext(const String& name) override;
 
 	/// Releases a context previously created by this context.
 	/// @param[in] context The context to release.
-	virtual void ReleaseContext(Context* context);
+	void ReleaseContext(Context* context) override;
 
 	/// Releases this context instancer.
-	virtual void Release();
+	void Release() override;
 };
 
 }

Source/Core/Core.cpp

@@ -51,30 +51,13 @@ static FileInterfaceDefault file_interface_default;
 #endif
 static bool initialised = false;
 
-typedef UnorderedMap< String, Context* > ContextMap;
+using ContextMap = UnorderedMap< String, UniquePtr<Context> >;
 static ContextMap contexts;
 
 #ifndef RMLUI_VERSION
 	#define RMLUI_VERSION "custom"
 #endif
 
-/**
-	A 'plugin' for unobtrusively intercepting the destruction of contexts.
- */
-
-class PluginContextRelease : public Plugin
-{
-	public:
-		virtual void OnShutdown()
-		{
-			delete this;
-		}
-
-		virtual void OnContextDestroy(Context* context)
-		{
-			contexts.erase(context->GetName());
-		}
-};
 
 bool Initialise()
 {
@@ -112,7 +95,6 @@ bool Initialise()
 	Factory::Initialise();
 
 	// Notify all plugins we're starting up.
-	PluginRegistry::RegisterPlugin(new PluginContextRelease());
 	PluginRegistry::NotifyInitialise();
 
 	initialised = true;
@@ -125,9 +107,6 @@ void Shutdown()
 	// Notify all plugins we're being shutdown.
 	PluginRegistry::NotifyShutdown();
 
-	// Release all remaining contexts.
-	for (ContextMap::iterator itr = contexts.begin(); itr != contexts.end(); ++itr)
-		Core::Log::Message(Log::LT_WARNING, "Context '%s' still active on shutdown.", (*itr).first.c_str());
 	contexts.clear();
 
 	TemplateCache::Shutdown();
@@ -225,26 +204,25 @@ FileInterface* GetFileInterface()
 Context* CreateContext(const String& name, const Vector2i& dimensions, RenderInterface* custom_render_interface)
 {
 	if (!initialised)
-		return NULL;
+		return nullptr;
 
-	if (custom_render_interface == NULL &&
-		render_interface == NULL)
+	if (!custom_render_interface && !render_interface)
 	{
 		Log::Message(Log::LT_WARNING, "Failed to create context '%s', no render interface specified and no default render interface exists.", name.c_str());
-		return NULL;
+		return nullptr;
 	}
 
-	if (GetContext(name) != NULL)
+	if (GetContext(name))
 	{
 		Log::Message(Log::LT_WARNING, "Failed to create context '%s', context already exists.", name.c_str());
-		return NULL;
+		return nullptr;
 	}
 
-	Context* new_context = Factory::InstanceContext(name);
-	if (new_context == NULL)
+	UniquePtr<Context> new_context = Factory::InstanceContext(name);
+	if (!new_context)
 	{
 		Log::Message(Log::LT_WARNING, "Failed to instance context '%s', instancer returned NULL.", name.c_str());
-		return NULL;
+		return nullptr;
 	}
 
 	// Set the render interface on the context, and add a reference onto it.
@@ -263,11 +241,24 @@ Context* CreateContext(const String& name, const Vector2i& dimensions, RenderInt
 		// install an identity view, by default
 		new_context->ProcessViewChange(Matrix4f::Identity());
 	}
-	contexts[name] = new_context;
 
-	PluginRegistry::NotifyContextCreate(new_context);
+	Context* new_context_raw = new_context.get();
+	contexts[name] = std::move(new_context);
 
-	return new_context;
+	PluginRegistry::NotifyContextCreate(new_context_raw);
+
+	return new_context_raw;
+}
+
+bool RemoveContext(const String& name)
+{
+	auto it = contexts.find(name);
+	if (it != contexts.end())
+	{
+		contexts.erase(it);
+		return true;
+	}
+	return false;
 }
 
 // Fetches a previously constructed context by name.
@@ -275,9 +266,9 @@ Context* GetContext(const String& name)
 {
 	ContextMap::iterator i = contexts.find(name);
 	if (i == contexts.end())
-		return NULL;
+		return nullptr;
 
-	return (*i).second;
+	return i->second.get();
 }
 
 // Fetches a context by index.
@@ -296,9 +287,9 @@ Context* GetContext(int index)
 	}
 
 	if (i == contexts.end())
-		return NULL;
+		return nullptr;
 
-	return (*i).second;
+	return i->second.get();
 }
 
 // Returns the number of active contexts.

Source/Core/Factory.cpp

@@ -67,7 +67,7 @@ typedef UnorderedMap< String, std::unique_ptr<FontEffectInstancer> > FontEffectI
 static FontEffectInstancerMap font_effect_instancers;
 
 // The context instancer.
-static ContextInstancer* context_instancer = NULL;
+static SharedPtr<ContextInstancer> context_instancer;
 
 // The event instancer
 static EventInstancer* event_instancer = NULL;
@@ -86,8 +86,8 @@ Factory::~Factory()
 bool Factory::Initialise()
 {
 	// Bind the default context instancer.
-	if (context_instancer == NULL)
-		context_instancer = new ContextInstancerDefault();
+	if (!context_instancer)
+		context_instancer = std::make_shared<ContextInstancerDefault>();
 
 	// Bind default event instancer
 	if (event_instancer == NULL)
@@ -130,9 +130,7 @@ void Factory::Shutdown()
 
 	font_effect_instancers.clear();
 
-	if (context_instancer)
-		context_instancer->RemoveReference();
-	context_instancer = NULL;
+	context_instancer.reset();
 
 	if (event_listener_instancer)
 		event_listener_instancer->RemoveReference();
@@ -146,23 +144,19 @@ void Factory::Shutdown()
 }
 
 // Registers the instancer to use when instancing contexts.
-ContextInstancer* Factory::RegisterContextInstancer(ContextInstancer* instancer)
+ContextInstancer* Factory::RegisterContextInstancer(SharedPtr<ContextInstancer> instancer)
 {
-	instancer->AddReference();
-	if (context_instancer != NULL)
-		context_instancer->RemoveReference();
-
-	context_instancer = instancer;
-	return instancer;
+	ContextInstancer* result = instancer.get();
+	context_instancer = std::move(instancer);
+	return result;
 }
 
 // Instances a new context.
-Context* Factory::InstanceContext(const String& name)
+UniquePtr<Context> Factory::InstanceContext(const String& name)
 {
-	Context* new_context = context_instancer->InstanceContext(name);
-	if (new_context != NULL)
+	UniquePtr<Context> new_context = context_instancer->InstanceContext(name);
+	if (new_context)
 		new_context->SetInstancer(context_instancer);
-
 	return new_context;
 }
 

Source/Debugger/Plugin.cpp

@@ -211,15 +211,6 @@ void Plugin::OnShutdown()
 	// and that we don't try send the messages to the debug log window
 	ReleaseElements();
 
-	if (!elements.empty())
-	{
-		Core::Log::Message(Core::Log::LT_WARNING, "%u leaked elements detected.", elements.size());
-
-		int count = 0;
-		for (ElementInstanceMap::iterator i = elements.begin(); i != elements.end(); ++i)
-			Core::Log::Message(Core::Log::LT_WARNING, "\t(%d) %s -> %s", count++, (*i)->GetTagName().c_str(), (*i)->GetAddress().c_str());
-	}
-
 	delete this;
 }
 
@@ -229,7 +220,7 @@ void Plugin::OnContextDestroy(Core::Context* context)
 	if (context == debug_context)
 	{
 		// The context we're debugging is being destroyed, so we need to remove our debug hook elements.
-		SetContext(NULL);
+		SetContext(nullptr);
 	}
 
 	if (context == host_context)
@@ -238,24 +229,15 @@ void Plugin::OnContextDestroy(Core::Context* context)
 
 		ReleaseElements();
 
-		Geometry::SetContext(NULL);
-		context = NULL;
+		Geometry::SetContext(nullptr);
+		context = nullptr;
 	}
 }
 
-// Called whenever an element is created.
-void Plugin::OnElementCreate(Core::Element* element)
-{
-	// Store the stack addresses for this frame.
-	elements.insert(element);
-}
-
 // Called whenever an element is destroyed.
 void Plugin::OnElementDestroy(Core::Element* element)
 {
-	elements.erase(element);
-
-	if (info_element != NULL)
+	if (info_element)
 		info_element->OnElementDestroy(element);
 }
 
@@ -381,23 +363,31 @@ void Plugin::ReleaseElements()
 {
 	if (menu_element)
 	{
-		menu_element = NULL;
+		if (auto parent = menu_element->GetParentNode())
+			parent->RemoveChild(menu_element);
+		menu_element = nullptr;
 	}
 
 	if (info_element)
 	{
-		info_element = NULL;
+		if (auto parent = info_element->GetParentNode())
+			parent->RemoveChild(info_element);
+		info_element = nullptr;
 	}
 
 	if (log_element)
 	{
-		log_element = NULL;
+		if (auto parent = log_element->GetParentNode())
+			parent->RemoveChild(log_element);
+		log_element = nullptr;
 		delete log_hook;
 	}
 
 	if (hook_element)
 	{
-		hook_element = NULL;
+		if (auto parent = hook_element->GetParentNode())
+			parent->RemoveChild(hook_element);
+		hook_element = nullptr;
 	}
 }
 

Source/Debugger/Plugin.h

@@ -86,9 +86,6 @@ public:
 	/// @param[in] context The destroyed context.
 	void OnContextDestroy(Core::Context* context) override;
 
-	/// Called whenever an element is created.
-	/// @param[in] element The created element.
-	void OnElementCreate(Core::Element* element) override;
 	/// Called whenever an element is destroyed.
 	/// @param[in] element The destroyed element.
 	void OnElementDestroy(Core::Element* element) override;
@@ -124,10 +121,6 @@ private:
 
 	bool render_outlines;
 
-	// Keep track of instanced elements for leak tracking.
-	typedef std::unordered_set< Core::Element* > ElementInstanceMap;
-	ElementInstanceMap elements;
-
 	// Singleton instance
 	static Plugin* instance;
 };