HIERARCHY_NAVIG_DLG: fix crash when in a subsheet one open the dialog

and switch to the paren sheet from the context menu.

Mainly, change the way the dialog is managed from the sch_edit_frame:
the pointer to find the created dialog is replaced by a call to wxWidows::FindWindowByName().

Using a pointer set and cleared by events is a recipe for crashes.
This commit is contained in:
jean-pierre charras 2020-01-08 15:01:22 +01:00
parent 9e5d52f92d
commit fd0d5fdb38
4 changed files with 45 additions and 30 deletions

View File

@ -80,7 +80,7 @@ HIERARCHY_TREE::HIERARCHY_TREE( HIERARCHY_NAVIG_DLG* parent ) :
HIERARCHY_NAVIG_DLG::HIERARCHY_NAVIG_DLG( SCH_EDIT_FRAME* aParent ) : HIERARCHY_NAVIG_DLG::HIERARCHY_NAVIG_DLG( SCH_EDIT_FRAME* aParent ) :
DIALOG_SHIM( aParent, wxID_ANY, _( "Navigator" ), wxDefaultPosition, wxDefaultSize, DIALOG_SHIM( aParent, wxID_ANY, _( "Navigator" ), wxDefaultPosition, wxDefaultSize,
wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER ) wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER, HIERARCHY_NAVIG_DLG_WNAME )
{ {
wxASSERT( dynamic_cast< SCH_EDIT_FRAME* >( aParent ) ); wxASSERT( dynamic_cast< SCH_EDIT_FRAME* >( aParent ) );
@ -130,6 +130,9 @@ HIERARCHY_NAVIG_DLG::HIERARCHY_NAVIG_DLG( SCH_EDIT_FRAME* aParent ) :
Bind( wxEVT_TREE_ITEM_ACTIVATED, &HIERARCHY_NAVIG_DLG::onSelectSheetPath, this ); Bind( wxEVT_TREE_ITEM_ACTIVATED, &HIERARCHY_NAVIG_DLG::onSelectSheetPath, this );
// Manage a simple click on a selection, if the selection changes // Manage a simple click on a selection, if the selection changes
Bind( wxEVT_TREE_SEL_CHANGED, &HIERARCHY_NAVIG_DLG::onSelectSheetPath, this ); Bind( wxEVT_TREE_SEL_CHANGED, &HIERARCHY_NAVIG_DLG::onSelectSheetPath, this );
// Connect close event for the dialog:
this->Connect( wxEVT_CLOSE_WINDOW, wxCloseEventHandler( HIERARCHY_NAVIG_DLG::OnCloseNav ) );
} }
@ -138,6 +141,7 @@ HIERARCHY_NAVIG_DLG::~HIERARCHY_NAVIG_DLG()
Unbind( wxEVT_TREE_ITEM_ACTIVATED, &HIERARCHY_NAVIG_DLG::onSelectSheetPath, this ); Unbind( wxEVT_TREE_ITEM_ACTIVATED, &HIERARCHY_NAVIG_DLG::onSelectSheetPath, this );
Unbind( wxEVT_TREE_SEL_CHANGED, &HIERARCHY_NAVIG_DLG::onSelectSheetPath, this ); Unbind( wxEVT_TREE_SEL_CHANGED, &HIERARCHY_NAVIG_DLG::onSelectSheetPath, this );
m_Tree->Disconnect( wxEVT_CHAR, wxKeyEventHandler( HIERARCHY_TREE::onChar ) ); m_Tree->Disconnect( wxEVT_CHAR, wxKeyEventHandler( HIERARCHY_TREE::onChar ) );
this->Disconnect( wxEVT_CLOSE_WINDOW, wxCloseEventHandler( HIERARCHY_NAVIG_DLG::OnCloseNav ) );
} }
@ -184,12 +188,16 @@ void HIERARCHY_NAVIG_DLG::buildHierarchyTree( SCH_SHEET_PATH* aList, wxTreeItemI
void HIERARCHY_NAVIG_DLG::UpdateHierarchyTree() void HIERARCHY_NAVIG_DLG::UpdateHierarchyTree()
{ {
Freeze();
m_currSheet = m_SchFrameEditor->GetCurrentSheet(); m_currSheet = m_SchFrameEditor->GetCurrentSheet();
wxTreeItemId root = m_Tree->GetRootItem(); wxTreeItemId root = m_Tree->GetRootItem();
m_Tree->DeleteChildren( root ); m_Tree->DeleteChildren( root );
m_list.clear(); m_list.clear();
m_list.push_back( g_RootSheet ); m_list.push_back( g_RootSheet );
buildHierarchyTree( &m_list, &root ); buildHierarchyTree( &m_list, &root );
Thaw();
} }
void HIERARCHY_NAVIG_DLG::onSelectSheetPath( wxTreeEvent& event ) void HIERARCHY_NAVIG_DLG::onSelectSheetPath( wxTreeEvent& event )
@ -200,15 +208,18 @@ void HIERARCHY_NAVIG_DLG::onSelectSheetPath( wxTreeEvent& event )
wxTreeItemId ItemSel = m_Tree->GetSelection(); wxTreeItemId ItemSel = m_Tree->GetSelection();
m_SchFrameEditor->SetCurrentSheet(( (TreeItemData*) m_Tree->GetItemData( ItemSel ) )->m_SheetPath ); m_SchFrameEditor->SetCurrentSheet(( (TreeItemData*) m_Tree->GetItemData( ItemSel ) )->m_SheetPath );
m_SchFrameEditor->DisplayCurrentSheet(); m_SchFrameEditor->DisplayCurrentSheet();
if( m_SchFrameEditor->GetNavigatorStaysOpen() == false ) if( m_SchFrameEditor->GetNavigatorStaysOpen() == false )
Close( true ); Close( true );
} }
void HIERARCHY_NAVIG_DLG::OnClose( wxCloseEvent& event )
void HIERARCHY_NAVIG_DLG::OnCloseNav( wxCloseEvent& event )
{ {
m_SchFrameEditor->CloseHierarchyNavigator(); Destroy();
} }
void SCH_EDIT_FRAME::DisplayCurrentSheet() void SCH_EDIT_FRAME::DisplayCurrentSheet()
{ {
m_toolManager->RunAction( ACTIONS::cancelInteractive, true ); m_toolManager->RunAction( ACTIONS::cancelInteractive, true );

View File

@ -31,6 +31,9 @@
#include <wx/imaglist.h> #include <wx/imaglist.h>
#include <wx/treectrl.h> #include <wx/treectrl.h>
// The window name of the hierarchy navigator, used to find it
#define HIERARCHY_NAVIG_DLG_WNAME "hierarchy_navig_dlg"
class SCH_EDIT_FRAME; class SCH_EDIT_FRAME;
class SCH_SHEET_PATH; class SCH_SHEET_PATH;
@ -65,7 +68,9 @@ public:
HIERARCHY_NAVIG_DLG( SCH_EDIT_FRAME* aParent ); HIERARCHY_NAVIG_DLG( SCH_EDIT_FRAME* aParent );
~HIERARCHY_NAVIG_DLG(); ~HIERARCHY_NAVIG_DLG();
void OnClose( wxCloseEvent& event );
void OnCloseNav( wxCloseEvent& event );
/** /**
* Update the hierarchical tree of the schematic. * Update the hierarchical tree of the schematic.
*/ */

View File

@ -248,7 +248,6 @@ SCH_EDIT_FRAME::SCH_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ):
m_findReplaceDialog = nullptr; m_findReplaceDialog = nullptr;
m_findReplaceStatusPopup = nullptr; m_findReplaceStatusPopup = nullptr;
m_hierarchyDialog = nullptr;
SetForceHVLines( true ); SetForceHVLines( true );
SetSpiceAdjustPassiveValues( false ); SetSpiceAdjustPassiveValues( false );
@ -562,10 +561,9 @@ void SCH_EDIT_FRAME::OnCloseWindow( wxCloseEvent& aEvent )
m_findReplaceDialog = nullptr; m_findReplaceDialog = nullptr;
} }
if( m_hierarchyDialog ) if( FindHierarchyNavigator() )
{ {
m_hierarchyDialog->Destroy(); FindHierarchyNavigator()->Close( true );
m_hierarchyDialog = nullptr;
} }
SCH_SCREENS screens; SCH_SCREENS screens;
@ -676,26 +674,33 @@ wxFindReplaceData* SCH_EDIT_FRAME::GetFindReplaceData()
return nullptr; return nullptr;
} }
HIERARCHY_NAVIG_DLG* SCH_EDIT_FRAME::FindHierarchyNavigator()
{
wxWindow* navigator = wxWindow::FindWindowByName( HIERARCHY_NAVIG_DLG_WNAME );
return static_cast< HIERARCHY_NAVIG_DLG* >( navigator );
}
void SCH_EDIT_FRAME::UpdateHierarchyNavigator( bool aForceUpdate ) void SCH_EDIT_FRAME::UpdateHierarchyNavigator( bool aForceUpdate )
{ {
if( aForceUpdate ) if( aForceUpdate )
{ {
if( m_hierarchyDialog ) if( FindHierarchyNavigator() )
CloseHierarchyNavigator(); FindHierarchyNavigator()->Close();
m_hierarchyDialog = new HIERARCHY_NAVIG_DLG( this ); HIERARCHY_NAVIG_DLG* hierarchyDialog = new HIERARCHY_NAVIG_DLG( this );
m_hierarchyDialog->Connect(
wxEVT_CLOSE_WINDOW, wxCloseEventHandler( HIERARCHY_NAVIG_DLG::OnClose ) ); hierarchyDialog->Show( true );
m_hierarchyDialog->Show( true );
} }
else else
{ {
if( m_hierarchyDialog ) if( FindHierarchyNavigator() )
m_hierarchyDialog->UpdateHierarchyTree(); FindHierarchyNavigator()->UpdateHierarchyTree();
} }
} }
void SCH_EDIT_FRAME::ShowFindReplaceDialog( bool aReplace ) void SCH_EDIT_FRAME::ShowFindReplaceDialog( bool aReplace )
{ {
if( m_findReplaceStatusPopup ) if( m_findReplaceStatusPopup )
@ -748,11 +753,6 @@ void SCH_EDIT_FRAME::OnFindDialogClose()
m_findReplaceDialog = nullptr; m_findReplaceDialog = nullptr;
} }
void SCH_EDIT_FRAME::CloseHierarchyNavigator()
{
m_hierarchyDialog->Destroy();
m_hierarchyDialog = nullptr;
}
void SCH_EDIT_FRAME::OnLoadFile( wxCommandEvent& event ) void SCH_EDIT_FRAME::OnLoadFile( wxCommandEvent& event )
{ {

View File

@ -139,7 +139,6 @@ private:
DIALOG_SCH_FIND* m_findReplaceDialog; DIALOG_SCH_FIND* m_findReplaceDialog;
STATUS_TEXT_POPUP* m_findReplaceStatusPopup; STATUS_TEXT_POPUP* m_findReplaceStatusPopup;
HIERARCHY_NAVIG_DLG* m_hierarchyDialog;
/// Flag to indicate show hidden pins. /// Flag to indicate show hidden pins.
bool m_showAllPins; bool m_showAllPins;
@ -357,11 +356,16 @@ public:
/** /**
* Run the Hierarchy Navigator dialog. * Run the Hierarchy Navigator dialog.
* @param aForceUpdate When true, creates a new dialog. And if a dialog * @param aForceUpdate: When true, creates a new dialog. And if a dialog
* already exist, it destroys it first. * already exist, it destroys it first.
*/ */
void UpdateHierarchyNavigator( bool aForceUpdate = false ); void UpdateHierarchyNavigator( bool aForceUpdate = false );
/**
* @return a reference to the Hierarchy Navigator dialog if exists, or nullptr.
*/
HIERARCHY_NAVIG_DLG* FindHierarchyNavigator();
void ShowFindReplaceStatus( const wxString& aMsg ); void ShowFindReplaceStatus( const wxString& aMsg );
void ClearFindReplaceStatus(); void ClearFindReplaceStatus();
@ -375,11 +379,6 @@ public:
*/ */
void OnFindDialogClose(); void OnFindDialogClose();
/**
* Destroy the Hierarchy Navigator dialog.
*/
void CloseHierarchyNavigator();
/** /**
* Breaks a single segment into two at the specified point. * Breaks a single segment into two at the specified point.
* *