Browse Source

Log an error message when externally closing a document owned by the debugger plugin

This can for example happen if if the user calls `Context::UnloadAllDocuments()` on the host context.

This is not allowed and will leave the debugger in an invalid state. Log an error to better diagnose the issue. Add some unit tests to check the expected behavior.
Michael Ragazzon 1 năm trước cách đây
mục cha
commit
c84ed5de78

+ 9 - 0
Source/Debugger/DebuggerPlugin.cpp

@@ -220,6 +220,15 @@ void DebuggerPlugin::OnContextDestroy(Context* context)
 
 void DebuggerPlugin::OnElementDestroy(Element* element)
 {
+	// Detect external destruction of any of the debugger documents. This can happen for example if the user calls
+	// `Context::UnloadAllDocuments()` on the host context.
+	if (element == menu_element || element == info_element || element == log_element)
+	{
+		Log::Message(Log::LT_ERROR,
+			"A document owned by the Debugger plugin was destroyed externally. This is not allowed. Consider shutting down the debugger instead.");
+		ReleaseElements();
+	}
+
 	if (info_element)
 		info_element->OnElementDestroy(element);
 }

+ 1 - 1
Source/Debugger/DebuggerPlugin.h

@@ -62,7 +62,7 @@ public:
 
 	/// Sets the context to be debugged.
 	/// @param[in] context The context to be debugged.
-	/// @return True if the debugger is initialised and the context was switched, false otherwise..
+	/// @return True if the debugger is initialised and the context was switched, false otherwise.
 	bool SetContext(Context* context);
 
 	/// Sets the visibility of the debugger.

+ 2 - 2
Source/Debugger/ElementLog.cpp

@@ -83,10 +83,10 @@ ElementLog::~ElementLog()
 	RemoveEventListener(EventId::Click, this);
 
 	if (beacon && beacon->GetFirstChild())
-	{
 		beacon->GetFirstChild()->RemoveEventListener(EventId::Click, this);
+
+	if (beacon && GetParentNode())
 		beacon->GetParentNode()->RemoveChild(beacon);
-	}
 
 	if (message_content)
 	{

+ 25 - 0
Tests/Source/UnitTests/Debugger.cpp

@@ -82,3 +82,28 @@ TEST_CASE("debugger")
 
 	TestsShell::ShutdownShell();
 }
+
+TEST_CASE("debugger.unload_documents")
+{
+	Context* context = TestsShell::GetContext(false);
+	Rml::Debugger::Initialise(context);
+
+	context->LoadDocument("assets/demo.rml");
+	TestsShell::RenderLoop();
+
+	// Closing documents from the debugger plugin is not allowed.
+	TestsShell::SetNumExpectedWarnings(1);
+
+	SUBCASE("UnloadDocument")
+	{
+		context->GetDocument("rmlui-debug-menu")->Close();
+	}
+	SUBCASE("UnloadAllDocuments")
+	{
+		context->UnloadAllDocuments();
+	}
+
+	context->Update();
+
+	TestsShell::ShutdownShell();
+}

+ 5 - 1
changelog.md

@@ -178,7 +178,6 @@ input { nav: auto; nav-right: #ok_button; }
 
 - Improved mesh utilities to construct background geometry for any given area of the element, including for elements with border-radius.
 - New Rectangle type to better represent many operations.
-- Debugger now displays the axis-aligned bounding box of selected elements, including any transforms and box shadows.
 - Visual tests:
   - Several new visual tests for the new features.
   - Highlight differences when comparing to previous capture by holding shift key.
@@ -191,6 +190,11 @@ input { nav: auto; nav-right: #ok_button; }
 - Fix XML parse error if single curly brace encountered at the end of a data binding string literal. #459 (thanks @Dakror)
 - Fix usage of data variables in selected `option`s. #509 #510 (thanks @Dakror)
 
+### Debugger plugin
+
+- The debugger can now display the axis-aligned bounding box of selected elements, including any transforms and box shadows.
+- Log an error message when externally closing documents owned by the debugger plugin.
+
 ### Lua plugin
 
 - Add `StopImmediatePropagation` to Rml::Event. #466 (thanks @ShawnCZek)