From 87b394a5557fa38e81da79aebcec0ff4f8d5a8f2 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 20 Oct 2021 23:21:04 +0100 Subject: [PATCH 1/4] Use wxScopedPtr in MiscGUIFuncsTestCase for automatic cleanup Replace manual calls to Destroy() to ensure that the objects created in this test will be destroyed even if a check fails. --- tests/misc/guifuncs.cpp | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/tests/misc/guifuncs.cpp b/tests/misc/guifuncs.cpp index 05fef40d89..b0c4af4aea 100644 --- a/tests/misc/guifuncs.cpp +++ b/tests/misc/guifuncs.cpp @@ -25,6 +25,7 @@ #include "wx/clipbrd.h" #include "wx/dataobj.h" #include "wx/panel.h" +#include "wx/scopedptr.h" #include "asserthelper.h" @@ -143,12 +144,12 @@ void MiscGUIFuncsTestCase::ClientToScreen() wxWindow* const tlw = wxTheApp->GetTopWindow(); CPPUNIT_ASSERT( tlw ); - wxPanel* const - p1 = new wxPanel(tlw, wxID_ANY, wxPoint(0, 0), wxSize(100, 50)); - wxPanel* const - p2 = new wxPanel(tlw, wxID_ANY, wxPoint(0, 50), wxSize(100, 50)); + wxScopedPtr const + p1(new wxPanel(tlw, wxID_ANY, wxPoint(0, 0), wxSize(100, 50))); + wxScopedPtr const + p2(new wxPanel(tlw, wxID_ANY, wxPoint(0, 50), wxSize(100, 50))); wxWindow* const - b = new wxWindow(p2, wxID_ANY, wxPoint(10, 10), wxSize(30, 10)); + b = new wxWindow(p2.get(), wxID_ANY, wxPoint(10, 10), wxSize(30, 10)); // We need this to realize the windows created above under wxGTK. wxYield(); @@ -166,9 +167,6 @@ void MiscGUIFuncsTestCase::ClientToScreen() tlwOrig + wxPoint(10, 60), b->ClientToScreen(wxPoint(0, 0)) ); - - p1->Destroy(); - p2->Destroy(); } namespace @@ -206,9 +204,11 @@ void MiscGUIFuncsTestCase::FindWindowAtPoint() // assertion messages. parent->SetLabel("parent"); - wxWindow* btn1 = new TestButton(parent, "1", wxPoint(10, 10)); - wxWindow* btn2 = new TestButton(parent, "2", wxPoint(10, 90)); - wxWindow* btn3 = new TestButton(btn2, "3", wxPoint(20, 20)); + wxScopedPtr btn1(new TestButton(parent, "1", wxPoint(10, 10))); + wxScopedPtr btn2(new TestButton(parent, "2", wxPoint(10, 90))); + + // No need to use wxScopedPtr<> for this one, it will be deleted by btn2. + wxWindow* btn3 = new TestButton(btn2.get(), "3", wxPoint(20, 20)); // We need this to realize the windows created above under wxGTK. wxYield(); @@ -265,8 +265,4 @@ void MiscGUIFuncsTestCase::FindWindowAtPoint() btn3->GetLabel(), GetLabelOfWindowAtPoint(parent, 31, 111) ); - - btn1->Destroy(); - btn2->Destroy(); - // btn3 was already deleted when its parent was } From 85503d1dcd6614996f7fa97e226e23f71695568e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 20 Oct 2021 23:32:50 +0100 Subject: [PATCH 2/4] Get rid of CppUnit boilerplate in MiscGUIFuncsTestCase Simplify code by not defining an unnecessary test case class and using CATCH macros directly. No real changes. --- tests/misc/guifuncs.cpp | 149 +++++++++++----------------------------- 1 file changed, 42 insertions(+), 107 deletions(-) diff --git a/tests/misc/guifuncs.cpp b/tests/misc/guifuncs.cpp index b0c4af4aea..c47b941d6a 100644 --- a/tests/misc/guifuncs.cpp +++ b/tests/misc/guifuncs.cpp @@ -30,103 +30,74 @@ #include "asserthelper.h" // ---------------------------------------------------------------------------- -// test class +// the tests // ---------------------------------------------------------------------------- -class MiscGUIFuncsTestCase : public CppUnit::TestCase -{ -public: - MiscGUIFuncsTestCase() { } - -private: - CPPUNIT_TEST_SUITE( MiscGUIFuncsTestCase ); - CPPUNIT_TEST( DisplaySize ); - CPPUNIT_TEST( URLDataObject ); - CPPUNIT_TEST( ParseFileDialogFilter ); - CPPUNIT_TEST( ClientToScreen ); - CPPUNIT_TEST( FindWindowAtPoint ); - CPPUNIT_TEST_SUITE_END(); - - void DisplaySize(); - void URLDataObject(); - void ParseFileDialogFilter(); - void ClientToScreen(); - void FindWindowAtPoint(); - - wxDECLARE_NO_COPY_CLASS(MiscGUIFuncsTestCase); -}; - -// register in the unnamed registry so that these tests are run by default -CPPUNIT_TEST_SUITE_REGISTRATION( MiscGUIFuncsTestCase ); - -// also include in its own registry so that these tests can be run alone -CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( MiscGUIFuncsTestCase, "MiscGUIFuncsTestCase" ); - -void MiscGUIFuncsTestCase::DisplaySize() +TEST_CASE("GUI::DisplaySize", "[guifuncs]") { // test that different (almost) overloads return the same results int w, h; wxDisplaySize(&w, &h); wxSize sz = wxGetDisplaySize(); - CPPUNIT_ASSERT_EQUAL( w, sz.x ); - CPPUNIT_ASSERT_EQUAL( h, sz.y ); + CHECK( sz.x == w ); + CHECK( sz.y == h ); // test that passing NULL works as expected, e.g. doesn't crash wxDisplaySize(NULL, NULL); wxDisplaySize(&w, NULL); wxDisplaySize(NULL, &h); - CPPUNIT_ASSERT_EQUAL( w, sz.x ); - CPPUNIT_ASSERT_EQUAL( h, sz.y ); + CHECK( sz.x == w ); + CHECK( sz.y == h ); // test that display PPI is something reasonable sz = wxGetDisplayPPI(); - CPPUNIT_ASSERT( sz.x < 1000 ); - CPPUNIT_ASSERT( sz.y < 1000 ); + CHECK( sz.x < 1000 ); + CHECK( sz.y < 1000 ); } -void MiscGUIFuncsTestCase::URLDataObject() -{ #if wxUSE_DATAOBJ +TEST_CASE("GUI::URLDataObject", "[guifuncs]") +{ // this tests for buffer overflow, see #11102 const char * const url = "http://something.long.to.overwrite.plenty.memory.example.com"; wxURLDataObject * const dobj = new wxURLDataObject(url); - CPPUNIT_ASSERT_EQUAL( url, dobj->GetURL() ); + CHECK( dobj->GetURL() == url ); wxClipboardLocker lockClip; - CPPUNIT_ASSERT( wxTheClipboard->SetData(dobj) ); + CHECK( wxTheClipboard->SetData(dobj) ); wxTheClipboard->Flush(); -#endif // wxUSE_DATAOBJ } +#endif // wxUSE_DATAOBJ -void MiscGUIFuncsTestCase::ParseFileDialogFilter() +TEST_CASE("GUI::ParseFileDialogFilter", "[guifuncs]") { wxArrayString descs, filters; - CPPUNIT_ASSERT_EQUAL + REQUIRE ( - 1, wxParseCommonDialogsFilter("Image files|*.jpg;*.png", descs, filters) + == 1 ); - CPPUNIT_ASSERT_EQUAL( "Image files", descs[0] ); - CPPUNIT_ASSERT_EQUAL( "*.jpg;*.png", filters[0] ); + CHECK( descs[0] == "Image files" ); + CHECK( filters[0] == "*.jpg;*.png" ); - CPPUNIT_ASSERT_EQUAL + REQUIRE ( - 2, wxParseCommonDialogsFilter ( "All files (*.*)|*.*|Python source (*.py)|*.py", descs, filters ) + == 2 ); - CPPUNIT_ASSERT_EQUAL( "*.*", filters[0] ); - CPPUNIT_ASSERT_EQUAL( "*.py", filters[1] ); + CHECK( filters[0] == "*.*" ); + CHECK( filters[1] == "*.py" ); // Test some invalid ones too. WX_ASSERT_FAILS_WITH_ASSERT @@ -139,10 +110,10 @@ void MiscGUIFuncsTestCase::ParseFileDialogFilter() ); } -void MiscGUIFuncsTestCase::ClientToScreen() +TEST_CASE("GUI::ClientToScreen", "[guifuncs]") { wxWindow* const tlw = wxTheApp->GetTopWindow(); - CPPUNIT_ASSERT( tlw ); + REQUIRE( tlw ); wxScopedPtr const p1(new wxPanel(tlw, wxID_ANY, wxPoint(0, 0), wxSize(100, 50))); @@ -156,17 +127,9 @@ void MiscGUIFuncsTestCase::ClientToScreen() const wxPoint tlwOrig = tlw->ClientToScreen(wxPoint(0, 0)); - CPPUNIT_ASSERT_EQUAL - ( - tlwOrig + wxPoint(0, 50), - p2->ClientToScreen(wxPoint(0, 0)) - ); + CHECK( p2->ClientToScreen(wxPoint(0, 0)) == tlwOrig + wxPoint(0, 50) ); - CPPUNIT_ASSERT_EQUAL - ( - tlwOrig + wxPoint(10, 60), - b->ClientToScreen(wxPoint(0, 0)) - ); + CHECK( b->ClientToScreen(wxPoint(0, 0)) == tlwOrig + wxPoint(10, 60) ); } namespace @@ -195,10 +158,10 @@ wxString GetLabelOfWindowAtPoint(wxWindow* parent, int x, int y) } // anonymous namespace -void MiscGUIFuncsTestCase::FindWindowAtPoint() +TEST_CASE("GUI::FindWindowAtPoint", "[guifuncs]") { wxWindow* const parent = wxTheApp->GetTopWindow(); - CPPUNIT_ASSERT( parent ); + REQUIRE( parent ); // Set a label to allow distinguishing it from the other windows in the // assertion messages. @@ -213,56 +176,28 @@ void MiscGUIFuncsTestCase::FindWindowAtPoint() // We need this to realize the windows created above under wxGTK. wxYield(); - CPPUNIT_ASSERT_EQUAL_MESSAGE - ( - "No window for a point outside of the window", - "NONE", - GetLabelOfWindowAtPoint(parent, 900, 900) - ); + INFO("No window for a point outside of the window"); + CHECK( GetLabelOfWindowAtPoint(parent, 900, 900) == "NONE" ); - CPPUNIT_ASSERT_EQUAL_MESSAGE - ( - "Point over a child control corresponds to it", - btn1->GetLabel(), - GetLabelOfWindowAtPoint(parent, 11, 11) - ); + INFO( "Point over a child control corresponds to it" ); + CHECK( GetLabelOfWindowAtPoint(parent, 11, 11) == btn1->GetLabel() ); - CPPUNIT_ASSERT_EQUAL_MESSAGE - ( - "Point outside of any child control returns the TLW itself", - parent->GetLabel(), - GetLabelOfWindowAtPoint(parent, 5, 5) - ); + INFO("Point outside of any child control returns the TLW itself"); + CHECK( GetLabelOfWindowAtPoint(parent, 5, 5) == parent->GetLabel() ); btn2->Disable(); - CPPUNIT_ASSERT_EQUAL_MESSAGE - ( - "Point over a disabled child control still corresponds to it", - btn2->GetLabel(), - GetLabelOfWindowAtPoint(parent, 11, 91) - ); + INFO("Point over a disabled child control still corresponds to it"); + CHECK( GetLabelOfWindowAtPoint(parent, 11, 91) == btn2->GetLabel() ); btn2->Hide(); - CPPUNIT_ASSERT_EQUAL_MESSAGE - ( - "Point over a hidden child control doesn't take it into account", - parent->GetLabel(), - GetLabelOfWindowAtPoint(parent, 11, 91) - ); + INFO("Point over a hidden child control doesn't take it into account"); + CHECK( GetLabelOfWindowAtPoint(parent, 11, 91) == parent->GetLabel() ); btn2->Show(); - CPPUNIT_ASSERT_EQUAL_MESSAGE - ( - "Point over child control corresponds to the child", - btn3->GetLabel(), - GetLabelOfWindowAtPoint(parent, 31, 111) - ); + INFO("Point over child control corresponds to the child"); + CHECK( GetLabelOfWindowAtPoint(parent, 31, 111) == btn3->GetLabel() ); btn3->Disable(); - CPPUNIT_ASSERT_EQUAL_MESSAGE - ( - "Point over disabled child controls still corresponds to this child", - btn3->GetLabel(), - GetLabelOfWindowAtPoint(parent, 31, 111) - ); + INFO("Point over disabled child controls still corresponds to this child"); + CHECK( GetLabelOfWindowAtPoint(parent, 31, 111) == btn3->GetLabel() ); } From 9e5c8eef245b342ef9d77cadf351b8d5410c5b4e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 20 Oct 2021 23:11:40 +0100 Subject: [PATCH 3/4] Make wxDumpWindow() public and available in wxMSW too This function was defined in wxGTK and wxOSX, but not in wxMSW or the other ports, but it can be useful there too, so make it public and define it in common code. --- include/wx/window.h | 3 +++ interface/wx/window.h | 17 +++++++++++++++++ src/common/wincmn.cpp | 25 +++++++++++++++++++++++++ src/gtk/window.cpp | 20 -------------------- src/osx/cocoa/window.mm | 3 --- src/osx/window_osx.cpp | 22 ---------------------- tests/misc/guifuncs.cpp | 13 +++++++++++++ 7 files changed, 58 insertions(+), 45 deletions(-) diff --git a/include/wx/window.h b/include/wx/window.h index d8a00f199f..c9b8869895 100644 --- a/include/wx/window.h +++ b/include/wx/window.h @@ -2100,6 +2100,9 @@ extern WXDLLIMPEXP_CORE wxWindow *wxGetActiveWindow(); // get the (first) top level parent window WXDLLIMPEXP_CORE wxWindow* wxGetTopLevelParent(wxWindowBase *win); +// Return a string with platform-dependent description of the window. +extern WXDLLIMPEXP_CORE wxString wxDumpWindow(wxWindowBase* win); + #if wxUSE_ACCESSIBILITY // ---------------------------------------------------------------------------- // accessible object for windows diff --git a/interface/wx/window.h b/interface/wx/window.h index 2918be982a..93186d4189 100644 --- a/interface/wx/window.h +++ b/interface/wx/window.h @@ -4251,5 +4251,22 @@ wxWindow* wxGetActiveWindow(); */ wxWindow* wxGetTopLevelParent(wxWindow* window); +/** + Return a string with human-readable platform-specific description of + the window useful for diagnostic purposes. + + The string returned from this function doesn't have any fixed form and can + vary between different wxWidgets ports and versions, but contains some + useful description of the window and uniquely identifies it. This can be + useful to include in debug or tracing messages. + + @param window Window pointer which is allowed to be @NULL. + + @header{wx/window.h} + + @since 3.1.6 + */ +wxString wxDumpWindow(wxWindow* window); + //@} diff --git a/src/common/wincmn.cpp b/src/common/wincmn.cpp index 42621feea4..2d1e13bb32 100644 --- a/src/common/wincmn.cpp +++ b/src/common/wincmn.cpp @@ -3661,6 +3661,31 @@ wxWindow* wxGetTopLevelParent(wxWindowBase *win_) return win; } +wxString wxDumpWindow(wxWindowBase* win) +{ + if ( !win ) + return wxString::FromAscii("[no window]"); + + wxString s = wxString::Format("%s@%p (", + win->GetClassInfo()->GetClassName(), win); + + wxString label = win->GetLabel(); + if ( label.empty() ) + label = win->GetName(); + + s += wxString::Format("\"%s\"", label); + + // Under MSW the HWND can be useful to find the window in Spy++ or similar + // program, so include it in the dump as well. +#ifdef __WXMSW__ + s += wxString::Format(", HWND=%p", win->GetHandle()); +#endif // __WXMSW__ + + s += ")"; + + return s; +} + #if wxUSE_ACCESSIBILITY // ---------------------------------------------------------------------------- // accessible object for windows diff --git a/src/gtk/window.cpp b/src/gtk/window.cpp index b9fc42d760..dbee540948 100644 --- a/src/gtk/window.cpp +++ b/src/gtk/window.cpp @@ -309,26 +309,6 @@ static wxPoint gs_lastGesturePoint; // the trace mask used for the focus debugging messages #define TRACE_FOCUS wxT("focus") -#if wxUSE_LOG_TRACE -// Function used to dump a brief description of a window. -static -wxString wxDumpWindow(wxWindowGTK* win) -{ - if ( !win ) - return "(no window)"; - - wxString s = wxString::Format("%s(%p", - win->GetClassInfo()->GetClassName(), win); - - wxString label = win->GetLabel(); - if ( !label.empty() ) - s += wxString::Format(", \"%s\"", label); - s += ")"; - - return s; -} -#endif // wxUSE_LOG_TRACE - // A handy function to run from under gdb to show information about the given // GtkWidget. Right now it only shows its type, we could enhance it to show // more information later but this is already pretty useful. diff --git a/src/osx/cocoa/window.mm b/src/osx/cocoa/window.mm index 7f34361392..537d091595 100644 --- a/src/osx/cocoa/window.mm +++ b/src/osx/cocoa/window.mm @@ -51,9 +51,6 @@ // debugging helpers // ---------------------------------------------------------------------------- -// This one is defined in window_osx.cpp. -extern wxString wxDumpWindow(wxWindowMac* win); - // These functions are called from the code but are also useful in the debugger // (especially wxDumpNSView(), as selectors can be printed out directly anyhow), // so make them just static instead of putting them in an anonymous namespace diff --git a/src/osx/window_osx.cpp b/src/osx/window_osx.cpp index d03e3b2467..99cbfe1c7c 100644 --- a/src/osx/window_osx.cpp +++ b/src/osx/window_osx.cpp @@ -181,28 +181,6 @@ wxIMPLEMENT_DYNAMIC_CLASS(wxBlindPlateWindow, wxWindow); wxBEGIN_EVENT_TABLE(wxBlindPlateWindow, wxWindow) wxEND_EVENT_TABLE() -// ---------------------------------------------------------------------------- -// debug helpers -// ---------------------------------------------------------------------------- - -// Function used to dump a brief description of a window. -extern -wxString wxDumpWindow(wxWindowMac* win) -{ - if ( !win ) - return "(no window)"; - - wxString s = wxString::Format("%s(%p", - win->GetClassInfo()->GetClassName(), win); - - wxString label = win->GetLabel(); - if ( !label.empty() ) - s += wxString::Format(", \"%s\"", label); - s += ")"; - - return s; -} - // ---------------------------------------------------------------------------- // constructors and such // ---------------------------------------------------------------------------- diff --git a/tests/misc/guifuncs.cpp b/tests/misc/guifuncs.cpp index c47b941d6a..1461a03147 100644 --- a/tests/misc/guifuncs.cpp +++ b/tests/misc/guifuncs.cpp @@ -201,3 +201,16 @@ TEST_CASE("GUI::FindWindowAtPoint", "[guifuncs]") INFO("Point over disabled child controls still corresponds to this child"); CHECK( GetLabelOfWindowAtPoint(parent, 31, 111) == btn3->GetLabel() ); } + +TEST_CASE("wxWindow::Dump", "[window]") +{ + CHECK_NOTHROW( wxDumpWindow(NULL) ); + + wxScopedPtr + button(new wxButton(wxTheApp->GetTopWindow(), wxID_ANY, "bloordyblop")); + + const std::string s = wxDumpWindow(button.get()).utf8_string(); + + CHECK_THAT( s, Catch::Contains("wxButton") ); + CHECK_THAT( s, Catch::Contains("bloordyblop") ); +} From ae2d8c34f94f353013ae7e3a2e5b23384c0e782b Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 20 Oct 2021 23:34:29 +0100 Subject: [PATCH 4/4] Use wxDumpWindow() in trace messages in wxWindow itself Instead of just showing the class name and the window name, show the result of wxDumpWindow() which is similar, but more informative. --- src/common/wincmn.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/common/wincmn.cpp b/src/common/wincmn.cpp index 2d1e13bb32..d700afb064 100644 --- a/src/common/wincmn.cpp +++ b/src/common/wincmn.cpp @@ -2672,9 +2672,7 @@ void wxWindowBase::SetConstraintSizes(bool recurse) } else if ( constr ) { - wxLogDebug(wxT("Constraints not satisfied for %s named '%s'."), - GetClassInfo()->GetClassName(), - GetName().c_str()); + wxLogDebug(wxT("Constraints not satisfied for %s."), wxDumpWindow(this)); } if ( recurse ) @@ -3340,8 +3338,7 @@ void wxWindowBase::ReleaseMouse() ( wxString::Format ( - "Releasing mouse in %p(%s) but it is not captured", - this, GetClassInfo()->GetClassName() + "Releasing mouse in %s but it is not captured", wxDumpWindow(this) ) ); } @@ -3351,9 +3348,8 @@ void wxWindowBase::ReleaseMouse() ( wxString::Format ( - "Releasing mouse in %p(%s) but it is captured by %p(%s)", - this, GetClassInfo()->GetClassName(), - winCapture, winCapture->GetClassInfo()->GetClassName() + "Releasing mouse in %s but it is captured by %s", + wxDumpWindow(this), wxDumpWindow(winCapture) ) ); }