Rework the file history menus to not need references to the file history

Keeping a pointer to the actual file history inside a special
file history menu led to many cases of use after free crashes,
so instead rework the actual file history to add the menu
items.

Fixes https://gitlab.com/kicad/code/kicad/issues/3741

common/bin_mod.cpp

@@ -45,7 +45,7 @@ void BIN_MOD::Init()
     // get file history size from common settings
     int fileHistorySize = Pgm().GetCommonSettings()->m_System.file_history_size;
 
-    m_history = new FILE_HISTORY( (unsigned) std::max( 0, fileHistorySize ), ID_FILE1 );
+    m_history = new FILE_HISTORY( (unsigned) std::max( 0, fileHistorySize ), ID_FILE1, ID_FILE_LIST_CLEAR );
     m_history->Load( *m_config );
 
     // Prepare On Line Help. Use only lower case for help file names, in order to

common/eda_base_frame.cpp

@@ -640,6 +640,15 @@ wxString EDA_BASE_FRAME::GetFileFromHistory( int cmdId, const wxString& type,
 }
 
 
+void EDA_BASE_FRAME::ClearFileHistory( FILE_HISTORY* aFileHistory )
+{
+    if( !aFileHistory )
+        aFileHistory = &Kiface().GetFileHistory();
+
+    aFileHistory->ClearFileHistory();
+}
+
+
 void EDA_BASE_FRAME::OnKicadAbout( wxCommandEvent& event )
 {
     void ShowAboutDialog(EDA_BASE_FRAME * aParent); // See AboutDialog_main.cpp

common/filehistory.cpp

@@ -33,8 +33,10 @@
 using namespace std::placeholders;
 
 
-FILE_HISTORY::FILE_HISTORY( size_t aMaxFiles, int aBaseFileId ) :
-        wxFileHistory( std::min( aMaxFiles, (size_t) MAX_FILE_HISTORY_SIZE ) )
+FILE_HISTORY::FILE_HISTORY( size_t aMaxFiles, int aBaseFileId, int aClearId, wxString aClearText )
+        : wxFileHistory( std::min( aMaxFiles, (size_t) MAX_FILE_HISTORY_SIZE ) ),
+          m_clearId( aClearId ),
+          m_clearText( aClearText )
 {
     SetBaseId( aBaseFileId );
 }
@@ -91,94 +93,90 @@ void FILE_HISTORY::SetMaxFiles( size_t aMaxFiles )
 
 void FILE_HISTORY::AddFileToHistory( const wxString &aFile )
 {
-    wxFileHistory::AddFileToHistory( aFile );
-
-    // Iterate over each menu associated with this file history, and if it is one of our
-    // FILE_HISTORY_MENUs, we force it to be refreshed (so that the items are all in the
-    // correct locations).
+    // Iterate over each menu removing our custom items
     for( wxList::compatibility_iterator node = m_fileMenus.GetFirst();
             node; node = node->GetNext() )
     {
         wxMenu* menu = static_cast<wxMenu*>( node->GetData() );
-
-        FILE_HISTORY_MENU* fileMenu = dynamic_cast<FILE_HISTORY_MENU*>( menu );
-
-        if( fileMenu )
-            fileMenu->RefreshMenu();
+        doRemoveClearitem( menu );
     }
-}
 
+    // Let wx add the items in the file history
+    wxFileHistory::AddFileToHistory( aFile );
 
-SELECTION_CONDITION FILE_HISTORY::FileHistoryNotEmpty( const FILE_HISTORY& aHistory )
-{
-    return std::bind( &FILE_HISTORY::isHistoryNotEmpty, _1, std::cref( aHistory ) );
+    // Add our custom items back
+    for( wxList::compatibility_iterator node = m_fileMenus.GetFirst();
+            node; node = node->GetNext() )
+    {
+        wxMenu* menu = static_cast<wxMenu*>( node->GetData() );
+        doAddClearItem( menu );
+    }
 }
 
 
-bool FILE_HISTORY::isHistoryNotEmpty( const SELECTION& aSelection, const FILE_HISTORY& aHistory )
+void FILE_HISTORY::AddFilesToMenu( wxMenu* aMenu )
 {
-    return aHistory.GetCount() != 0;
+    doRemoveClearitem( aMenu );
+    wxFileHistory::AddFilesToMenu( aMenu );
+    doAddClearItem( aMenu );
 }
 
 
-FILE_HISTORY_MENU::FILE_HISTORY_MENU( FILE_HISTORY& aHistory, wxString aClearText ) :
-    ACTION_MENU( false ),
-    m_fileHistory( aHistory ),
-    m_clearText( aClearText )
+void FILE_HISTORY::doRemoveClearitem( wxMenu* aMenu )
 {
-    m_fileHistory.UseMenu( this );
-    buildMenu();
-}
+    size_t      itemPos;
+    wxMenuItem* clearItem = aMenu->FindChildItem( m_clearId, &itemPos );
 
+    // Remove the separator if there is one
+    if( clearItem && itemPos > 1 )
+    {
+        wxMenuItem* sepItem = aMenu->FindItemByPosition( itemPos - 1 );
 
-FILE_HISTORY_MENU::~FILE_HISTORY_MENU()
-{
-    m_fileHistory.RemoveMenu( this );
-}
-
+        if( sepItem )
+            aMenu->Destroy( sepItem );
+    }
 
-void FILE_HISTORY_MENU::RefreshMenu()
-{
-    // We have to manually delete all menu items before we rebuild the menu
-    for( int i = GetMenuItemCount() - 1; i >= 0; --i )
-        Destroy( FindItemByPosition( i ) );
+    // Remove the clear and placeholder menu items
+    if( clearItem )
+        aMenu->Destroy( m_clearId );
 
-    buildMenu();
+    if( aMenu->FindChildItem( ID_FILE_LIST_EMPTY ) )
+        aMenu->Destroy( ID_FILE_LIST_EMPTY );
 }
 
 
-void FILE_HISTORY_MENU::buildMenu()
+void FILE_HISTORY::doAddClearItem( wxMenu* aMenu )
 {
-    if( m_fileHistory.GetCount() == 0 )
+    if( GetCount() == 0 )
     {
         // If the history is empty, we create an item to say there are no files
-        wxMenuItem* item = new wxMenuItem( this, wxID_ANY, _( "No Files" ) );
+        wxMenuItem* item = new wxMenuItem( nullptr, ID_FILE_LIST_EMPTY, _( "No Files" ) );
 
-        Append( item );
-        Enable( item->GetId(), false );
+        aMenu->Append( item );
+        aMenu->Enable( item->GetId(), false );
     }
-    else
-        m_fileHistory.AddFilesToMenu( this );
 
-    wxMenuItem* clearItem = new wxMenuItem( this, ID_FILE_LIST_CLEAR, m_clearText );
+    wxMenuItem* clearItem = new wxMenuItem( nullptr, m_clearId, m_clearText );
 
-    AppendSeparator();
-    Append( clearItem );
-    Connect( ID_FILE_LIST_CLEAR, wxEVT_COMMAND_MENU_SELECTED,
-            wxMenuEventHandler( FILE_HISTORY_MENU::onClearEntries ), NULL, this );
+    aMenu->AppendSeparator();
+    aMenu->Append( clearItem );
 }
 
 
-void FILE_HISTORY_MENU::onClearEntries( wxMenuEvent& aEvent )
+void FILE_HISTORY::ClearFileHistory()
 {
-    while( m_fileHistory.GetCount() > 0 )
-        m_fileHistory.RemoveFileFromHistory( 0 );
+    while( GetCount() > 0 )
+        RemoveFileFromHistory( 0 );
+}
+
 
-    RefreshMenu();
+SELECTION_CONDITION FILE_HISTORY::FileHistoryNotEmpty( const FILE_HISTORY& aHistory )
+{
+    return std::bind( &FILE_HISTORY::isHistoryNotEmpty, _1, std::cref( aHistory ) );
 }
 
 
-ACTION_MENU* FILE_HISTORY_MENU::create() const
+bool FILE_HISTORY::isHistoryNotEmpty( const SELECTION& aSelection, const FILE_HISTORY& aHistory )
 {
-    return new FILE_HISTORY_MENU( m_fileHistory, m_clearText );
+    return aHistory.GetCount() != 0;
 }

eeschema/menubar.cpp

@@ -53,21 +53,24 @@ void SCH_EDIT_FRAME::ReCreateMenuBar()
 
     //-- File menu -----------------------------------------------------------
     //
-    CONDITIONAL_MENU*         fileMenu = new CONDITIONAL_MENU( false, selTool );
-    static FILE_HISTORY_MENU* openRecentMenu;
+    CONDITIONAL_MENU*   fileMenu = new CONDITIONAL_MENU( false, selTool );
+    static ACTION_MENU* openRecentMenu;
 
-    if( Kiface().IsSingle() )   // not when under a project mgr
+    if( Kiface().IsSingle() )   // When not under a project mgr
     {
         FILE_HISTORY& fileHistory = Kiface().GetFileHistory();
 
-        // Create the menu if it does not exist. Adding a file to/from the history
-        // will automatically refresh the menu.
+        // Add this menu to the list of menus managed by the file history
+        // (the file history will be updated when adding/removing files in history)
         if( !openRecentMenu )
         {
-            openRecentMenu = new FILE_HISTORY_MENU( fileHistory );
+            openRecentMenu = new ACTION_MENU( false );
             openRecentMenu->SetTool( selTool );
             openRecentMenu->SetTitle( _( "Open Recent" ) );
             openRecentMenu->SetIcon( recent_xpm );
+
+            fileHistory.UseMenu( openRecentMenu );
+            fileHistory.AddFilesToMenu( openRecentMenu );
         }
 
         fileMenu->AddItem( ACTIONS::doNew,         EE_CONDITIONS::ShowAlways );

eeschema/sch_edit_frame.cpp

@@ -191,6 +191,7 @@ BEGIN_EVENT_TABLE( SCH_EDIT_FRAME, EDA_DRAW_FRAME )
     EVT_SIZE( SCH_EDIT_FRAME::OnSize )
 
     EVT_MENU_RANGE( ID_FILE1, ID_FILEMAX, SCH_EDIT_FRAME::OnLoadFile )
+    EVT_MENU( ID_FILE_LIST_CLEAR, SCH_EDIT_FRAME::OnClearFileHistory )
 
     EVT_MENU( ID_APPEND_PROJECT, SCH_EDIT_FRAME::OnAppendProject )
     EVT_MENU( ID_IMPORT_NON_KICAD_SCH, SCH_EDIT_FRAME::OnImportProject )
@@ -306,12 +307,6 @@ SCH_EDIT_FRAME::~SCH_EDIT_FRAME()
     g_CurrentSheet = nullptr;
     g_ConnectionGraph = nullptr;
     g_RootSheet = NULL;
-
-    // Since the file menu contains file history menus, we must ensure that the menu
-    // destructor is called before the file history objects are deleted since their destructor
-    // unregisters the menu from the history.
-    wxMenu* fileMenu = GetMenuBar()->Remove( 0 );
-    delete fileMenu;
 }
 
 
@@ -752,6 +747,12 @@ void SCH_EDIT_FRAME::OnLoadFile( wxCommandEvent& event )
 }
 
 
+void SCH_EDIT_FRAME::OnClearFileHistory( wxCommandEvent& aEvent )
+{
+    ClearFileHistory();
+}
+
+
 void SCH_EDIT_FRAME::NewProject()
 {
     wxString pro_dir = m_mruPath;

eeschema/sch_edit_frame.h

@@ -716,6 +716,8 @@ private:
     void OnAppendProject( wxCommandEvent& event );
     void OnImportProject( wxCommandEvent& event );
 
+    void OnClearFileHistory( wxCommandEvent& aEvent );
+
     /**
      * Set the main window title bar text.
      *

gerbview/events_called_functions.cpp

@@ -62,15 +62,19 @@ BEGIN_EVENT_TABLE( GERBVIEW_FRAME, EDA_DRAW_FRAME )
     EVT_MENU( ID_GERBVIEW_EXPORT_TO_PCBNEW, GERBVIEW_FRAME::ExportDataInPcbnewFormat )
 
     EVT_MENU_RANGE( ID_FILE1, ID_FILEMAX, GERBVIEW_FRAME::OnGbrFileHistory )
+    EVT_MENU( ID_FILE_LIST_CLEAR, GERBVIEW_FRAME::OnClearGbrFileHistory )
 
     EVT_MENU_RANGE( ID_GERBVIEW_DRILL_FILE1, ID_GERBVIEW_DRILL_FILEMAX,
                     GERBVIEW_FRAME::OnDrlFileHistory )
+    EVT_MENU( ID_GERBVIEW_DRILL_FILE_LIST_CLEAR, GERBVIEW_FRAME::OnClearDrlFileHistory )
 
     EVT_MENU_RANGE( ID_GERBVIEW_ZIP_FILE1, ID_GERBVIEW_ZIP_FILEMAX,
                     GERBVIEW_FRAME::OnZipFileHistory )
+    EVT_MENU( ID_GERBVIEW_ZIP_FILE_LIST_CLEAR, GERBVIEW_FRAME::OnClearZipFileHistory )
 
     EVT_MENU_RANGE( ID_GERBVIEW_JOB_FILE1, ID_GERBVIEW_JOB_FILEMAX,
                     GERBVIEW_FRAME::OnJobFileHistory )
+    EVT_MENU( ID_GERBVIEW_JOB_FILE_LIST_CLEAR, GERBVIEW_FRAME::OnClearJobFileHistory )
 
     EVT_MENU( wxID_EXIT, GERBVIEW_FRAME::OnQuit )
 

gerbview/files.cpp

@@ -57,6 +57,11 @@ void GERBVIEW_FRAME::OnGbrFileHistory( wxCommandEvent& event )
     }
 }
 
+void GERBVIEW_FRAME::OnClearGbrFileHistory( wxCommandEvent& aEvent )
+{
+    ClearFileHistory();
+}
+
 
 void GERBVIEW_FRAME::OnDrlFileHistory( wxCommandEvent& event )
 {
@@ -72,6 +77,12 @@ void GERBVIEW_FRAME::OnDrlFileHistory( wxCommandEvent& event )
 }
 
 
+void GERBVIEW_FRAME::OnClearDrlFileHistory( wxCommandEvent& aEvent )
+{
+    ClearFileHistory( &m_drillFileHistory );
+}
+
+
 void GERBVIEW_FRAME::OnZipFileHistory( wxCommandEvent& event )
 {
     wxString filename;
@@ -85,6 +96,12 @@ void GERBVIEW_FRAME::OnZipFileHistory( wxCommandEvent& event )
 }
 
 
+void GERBVIEW_FRAME::OnClearZipFileHistory( wxCommandEvent& aEvent )
+{
+    ClearFileHistory( &m_zipFileHistory );
+}
+
+
 void GERBVIEW_FRAME::OnJobFileHistory( wxCommandEvent& event )
 {
     wxString filename = GetFileFromHistory( event.GetId(), _( "Job files" ), &m_jobFileHistory );
@@ -94,6 +111,11 @@ void GERBVIEW_FRAME::OnJobFileHistory( wxCommandEvent& event )
 }
 
 
+void GERBVIEW_FRAME::OnClearJobFileHistory( wxCommandEvent& aEvent )
+{
+    ClearFileHistory( &m_jobFileHistory );
+}
+
 /* File commands. */
 void GERBVIEW_FRAME::Files_io( wxCommandEvent& event )
 {

gerbview/gerbview_frame.cpp

@@ -58,12 +58,15 @@
 #include <panel_hotkeys_editor.h>
 
 
-GERBVIEW_FRAME::GERBVIEW_FRAME( KIWAY* aKiway, wxWindow* aParent ):
-    EDA_DRAW_FRAME( aKiway, aParent, FRAME_GERBER, wxT( "GerbView" ),
-        wxDefaultPosition, wxDefaultSize, KICAD_DEFAULT_DRAWFRAME_STYLE, GERBVIEW_FRAME_NAME ),
-    m_zipFileHistory( DEFAULT_FILE_HISTORY_SIZE, ID_GERBVIEW_ZIP_FILE1 ),
-    m_drillFileHistory( DEFAULT_FILE_HISTORY_SIZE, ID_GERBVIEW_DRILL_FILE1 ),
-    m_jobFileHistory( DEFAULT_FILE_HISTORY_SIZE, ID_GERBVIEW_JOB_FILE1 )
+GERBVIEW_FRAME::GERBVIEW_FRAME( KIWAY* aKiway, wxWindow* aParent )
+        : EDA_DRAW_FRAME( aKiway, aParent, FRAME_GERBER, wxT( "GerbView" ), wxDefaultPosition,
+                wxDefaultSize, KICAD_DEFAULT_DRAWFRAME_STYLE, GERBVIEW_FRAME_NAME ),
+          m_zipFileHistory( DEFAULT_FILE_HISTORY_SIZE, ID_GERBVIEW_ZIP_FILE1,
+                  ID_GERBVIEW_ZIP_FILE_LIST_CLEAR, _( "Clear Recent Zip Files" ) ),
+          m_drillFileHistory( DEFAULT_FILE_HISTORY_SIZE, ID_GERBVIEW_DRILL_FILE1,
+                  ID_GERBVIEW_DRILL_FILE_LIST_CLEAR, _( "Clear Recent Drill Files" ) ),
+          m_jobFileHistory( DEFAULT_FILE_HISTORY_SIZE, ID_GERBVIEW_JOB_FILE1,
+                  ID_GERBVIEW_JOB_FILE_LIST_CLEAR, _( "Clear Recent Job Files" ) )
 {
     m_gerberLayout = NULL;
     m_zoomLevelCoeff = ZOOM_FACTOR( 110 );   // Adjusted to roughly displays zoom level = 1
@@ -225,12 +228,6 @@ GERBVIEW_FRAME::~GERBVIEW_FRAME()
 
     GetGerberLayout()->GetImagesList()->DeleteAllImages();
     delete m_gerberLayout;
-
-    // Since the file menu contains file history menus, we must ensure that the menu
-    // destructor is called before the file history objects are deleted since their destructor
-    // unregisters the menu from the history.
-    wxMenu* fileMenu = GetMenuBar()->Remove( 0 );
-    delete fileMenu;
 }
 
 

gerbview/gerbview_frame.h

@@ -187,6 +187,11 @@ private:
     void            updateZoomSelectBox();
     void            unitsChangeRefresh() override;      // See class EDA_DRAW_FRAME
 
+    void OnClearJobFileHistory( wxCommandEvent& aEvent );
+    void OnClearZipFileHistory( wxCommandEvent& aEvent );
+    void OnClearDrlFileHistory( wxCommandEvent& aEvent );
+    void OnClearGbrFileHistory( wxCommandEvent& aEvent );
+
     // The Tool Framework initalization
     void setupTools();
 

gerbview/gerbview_id.h

@@ -72,16 +72,19 @@ enum gerbview_ids
     ID_GERBVIEW_DRILL_FILE,
     ID_GERBVIEW_DRILL_FILE1,
     ID_GERBVIEW_DRILL_FILEMAX = ID_GERBVIEW_DRILL_FILE + MAX_FILE_HISTORY_SIZE,
+    ID_GERBVIEW_DRILL_FILE_LIST_CLEAR,
 
     // IDs for job file history (ID_FILEnn is already in use)
     ID_GERBVIEW_JOB_FILE,
     ID_GERBVIEW_JOB_FILE1,
     ID_GERBVIEW_JOB_FILEMAX = ID_GERBVIEW_JOB_FILE + MAX_FILE_HISTORY_SIZE,
+    ID_GERBVIEW_JOB_FILE_LIST_CLEAR,
 
     // IDs for zip file history (ID_FILEnn is already in use)
     ID_GERBVIEW_ZIP_FILE,
     ID_GERBVIEW_ZIP_FILE1,
     ID_GERBVIEW_ZIP_FILEMAX = ID_GERBVIEW_ZIP_FILE + MAX_FILE_HISTORY_SIZE,
+    ID_GERBVIEW_ZIP_FILE_LIST_CLEAR,
 
     ID_GERBER_END_LIST
 };

gerbview/menubar.cpp

@@ -46,56 +46,66 @@ void GERBVIEW_FRAME::ReCreateMenuBar()
 
     //-- File menu -------------------------------------------------------
     //
-    CONDITIONAL_MENU*         fileMenu = new CONDITIONAL_MENU( false, selTool );
-    static FILE_HISTORY_MENU* openRecentGbrMenu;
-    static FILE_HISTORY_MENU* openRecentDrlMenu;
-    static FILE_HISTORY_MENU* openRecentJobMenu;
-    static FILE_HISTORY_MENU* openRecentZipMenu;
-    FILE_HISTORY&             recentGbrFiles = Kiface().GetFileHistory();
+    CONDITIONAL_MENU*   fileMenu = new CONDITIONAL_MENU( false, selTool );
+    static ACTION_MENU* openRecentGbrMenu;
+    static ACTION_MENU* openRecentDrlMenu;
+    static ACTION_MENU* openRecentJobMenu;
+    static ACTION_MENU* openRecentZipMenu;
+
+    FILE_HISTORY& recentGbrFiles = Kiface().GetFileHistory();
+    recentGbrFiles.SetClearText( _( "Clear Recent Gerber Files" ) );
 
 
     // Create the gerber file menu if it does not exist. Adding a file to/from the history
     // will automatically refresh the menu.
     if( !openRecentGbrMenu )
     {
-        openRecentGbrMenu =
-                new FILE_HISTORY_MENU( recentGbrFiles, _( "Clear Recent Gerber Files" ) );
+        openRecentGbrMenu = new ACTION_MENU( false );
         openRecentGbrMenu->SetTool( selTool );
         openRecentGbrMenu->SetTitle( _( "Open Recent Gerber File" ) );
         openRecentGbrMenu->SetIcon( recent_xpm );
+
+        recentGbrFiles.UseMenu( openRecentGbrMenu );
+        recentGbrFiles.AddFilesToMenu();
     }
 
     // Create the drill file menu if it does not exist. Adding a file to/from the history
     // will automatically refresh the menu.
     if( !openRecentDrlMenu )
     {
-        openRecentDrlMenu =
-                new FILE_HISTORY_MENU( m_drillFileHistory, _( "Clear Recent Drill Files" ) );
+        openRecentDrlMenu = new ACTION_MENU( false );
         openRecentDrlMenu->SetTool( selTool );
         openRecentDrlMenu->SetTitle( _( "Open Recent Drill File" ) );
         openRecentDrlMenu->SetIcon( recent_xpm );
+
+        m_drillFileHistory.UseMenu( openRecentDrlMenu );
+        m_drillFileHistory.AddFilesToMenu();
     }
 
     // Create the job file menu if it does not exist. Adding a file to/from the history
     // will automatically refresh the menu.
     if( !openRecentJobMenu )
     {
-        openRecentJobMenu =
-                new FILE_HISTORY_MENU( m_jobFileHistory, _( "Clear Recent Job Files" ) );
+        openRecentJobMenu = new ACTION_MENU( false );
         openRecentJobMenu->SetTool( selTool );
         openRecentJobMenu->SetTitle( _( "Open Recent Job File" ) );
         openRecentJobMenu->SetIcon( recent_xpm );
+
+        m_jobFileHistory.UseMenu( openRecentJobMenu );
+        m_jobFileHistory.AddFilesToMenu();
     }
 
     // Create the zip file menu if it does not exist. Adding a file to/from the history
     // will automatically refresh the menu.
     if( !openRecentZipMenu )
     {
-        openRecentZipMenu =
-                new FILE_HISTORY_MENU( m_zipFileHistory, _( "Clear Recent Zip Files" ) );
+        openRecentZipMenu = new ACTION_MENU( false );
         openRecentZipMenu->SetTool( selTool );
         openRecentZipMenu->SetTitle( _( "Open Recent Zip File" ) );
         openRecentZipMenu->SetIcon( recent_xpm );
+
+        m_zipFileHistory.UseMenu( openRecentZipMenu );
+        m_zipFileHistory.AddFilesToMenu();
     }
 
     fileMenu->AddItem( wxID_FILE, _( "Open &Gerber File(s)..." ),

include/eda_base_frame.h

@@ -415,6 +415,14 @@ public:
     wxString GetFileFromHistory( int cmdId, const wxString& type,
                                  FILE_HISTORY* aFileHistory = NULL );
 
+    /**
+     * Removes all files from the file history.
+     *
+     * @param aFileHistory The FILE_HISTORY in use. If null, the main application file
+     *                     history is used
+     */
+    void ClearFileHistory( FILE_HISTORY* aFileHistory = NULL );
+
     /**
      * Update the list of recently opened files.
      *

include/filehistory.h

@@ -47,8 +47,11 @@ public:
      *
      * @param aMaxFiles is the number of files to store in the history
      * @param aBaseFileId is the ID to use for the first file menu item
+     * @param aClearId is the ID to use for the clear menu menu item
+     * @param aClearText is the text to use for the menu item that clears the history.
      */
-    FILE_HISTORY( size_t aMaxFiles, int aBaseFileId );
+    FILE_HISTORY( size_t aMaxFiles, int aBaseFileId, int aClearId,
+            wxString aClearText = _( "Clear Recent Files" ) );
 
     /**
      * Loads history from a JSON settings object
@@ -82,13 +85,29 @@ public:
      * Adds a file to the history.
      *
      * This function overrides the default wxWidgets method to iterate through all
-     * menus associated with the file history, and if they are of the FILE_HISTORY_MENU
-     * type, call their RefreshMenu() function to update the menu display.
+     * menus associated with the file history, remove the added menu items, lets wx
+     * add the new files, and then re-adds the clear menu item.
      *
      * @param aFile is the filename of the file to add to the history.
      */
     void AddFileToHistory( const wxString &aFile ) override;
 
+    /**
+     * Add the files to all registered menus.
+     */
+    void AddFilesToMenu() override
+    {
+        // This is needed to ensure that the proper base class function is called
+        wxFileHistory::AddFilesToMenu();
+    }
+
+    /**
+     * Add the files to the specified menu
+     *
+     * @aMenu is the menu to operate on.
+     */
+    void AddFilesToMenu( wxMenu* aMenu ) override;
+
     /**
      * Update the number of files that will be contained inside the file history.
      *
@@ -96,6 +115,21 @@ public:
      */
     void SetMaxFiles( size_t aMaxFiles );
 
+    /**
+     * Set the text displayed on the menu item that clears the entire menu.
+     *
+     * @param aClearText is the text to use for the menu item
+     */
+    void SetClearText( wxString aClearText )
+    {
+        m_clearText = aClearText;
+    }
+
+    /**
+     * Clear all entries from the file history.
+     */
+    void ClearFileHistory();
+
     /**
      * Create a SELECTION_CONDITION that can be used to enable a menu item when the
      * file history has items in it.
@@ -105,51 +139,33 @@ public:
      */
     static SELECTION_CONDITION FileHistoryNotEmpty( const FILE_HISTORY& aHistory );
 
-private:
-    static bool isHistoryNotEmpty( const SELECTION& aSelection, const FILE_HISTORY& aHistory );
-};
-
-/**
- * This class implements a menu container for a file history. It adds in the ability to clear
- * the file history through a menu item.
- */
-class FILE_HISTORY_MENU : public ACTION_MENU
-{
-public:
+protected:
     /**
-     * Create the file history menu.
+     * Remove the clear menu item and the preceding separator from the given menu.
      *
-     * @param aHistory is the file history to use in the menu
-     * @param aClearText is the text to use for the menu item that clears the history.
+     * @param aMenu is the menu to operate on
      */
-    FILE_HISTORY_MENU( FILE_HISTORY& aHistory, wxString aClearText = _( "Clear Recent Files" ) );
-
-    ~FILE_HISTORY_MENU();
+    void doRemoveClearitem( wxMenu* aMenu );
 
     /**
-     * Refresh the menu. This removes all entries from the menu and readds them, to ensure that the
-     * clear menu item is at the bottom of the menu.
+     * Add the clear menu item and the preceding separator to the given menu.
+     *
+     * @param aMenu is the menu to operate on
      */
-    void RefreshMenu();
+    void doAddClearItem( wxMenu* aMenu );
 
 private:
-    //! @copydoc ACTION_MENU::create()
-    ACTION_MENU* create() const override;
-
     /**
-     * Construct the menu by adding the file history and menu items.
-     */
-    void buildMenu();
-
-    /**
-     * Event handler for when the clear menu item is activated.
+     * Test if the file history is empty. This function is designed to be used with a SELECTION_CONDITION
+     * to enable/disable the file history menu.
      *
-     * @param aEvent the menu event
+     * @param aSelection is unused
+     * @param aHistory is the file history to test for items
      */
-    void onClearEntries( wxMenuEvent& aEvent );
+    static bool isHistoryNotEmpty( const SELECTION& aSelection, const FILE_HISTORY& aHistory );
 
-    FILE_HISTORY& m_fileHistory;
-    wxString      m_clearText;
+    int      m_clearId;
+    wxString m_clearText;
 };
 
 #endif

include/id.h

@@ -86,6 +86,7 @@ enum main_id
     ID_FILE,
     ID_FILE1,
     ID_FILEMAX = ID_FILE + MAX_FILE_HISTORY_SIZE,
+    ID_FILE_LIST_EMPTY,
     ID_FILE_LIST_CLEAR,
 
     ID_PREFERENCES_HOTKEY_SHOW_CURRENT_LIST,

kicad/files-io.cpp

@@ -55,6 +55,12 @@ void KICAD_MANAGER_FRAME::OnFileHistory( wxCommandEvent& event )
 }
 
 
+void KICAD_MANAGER_FRAME::OnClearFileHistory( wxCommandEvent& aEvent )
+{
+    ClearFileHistory( &PgmTop().GetFileHistory() );
+}
+
+
 void KICAD_MANAGER_FRAME::OnUnarchiveFiles( wxCommandEvent& event )
 {
     wxFileName fn = GetProjectFileName();

kicad/kicad_manager_frame.cpp

@@ -83,6 +83,7 @@ BEGIN_EVENT_TABLE( KICAD_MANAGER_FRAME, EDA_BASE_FRAME )
                     KICAD_MANAGER_FRAME::language_change )
 
     EVT_MENU_RANGE( ID_FILE1, ID_FILEMAX, KICAD_MANAGER_FRAME::OnFileHistory )
+    EVT_MENU( ID_FILE_LIST_CLEAR, KICAD_MANAGER_FRAME::OnClearFileHistory )
 
     // Special functions
     EVT_MENU( ID_INIT_WATCHED_PATHS, KICAD_MANAGER_FRAME::OnChangeWatchedPaths )
@@ -175,12 +176,6 @@ KICAD_MANAGER_FRAME::~KICAD_MANAGER_FRAME()
     delete m_toolManager;
 
     m_auimgr.UnInit();
-
-    // Since the file menu contains file history menus, we must ensure that the menu
-    // destructor is called before the file history objects are deleted since their destructor
-    // unregisters the menu from the history.
-    wxMenu* fileMenu = GetMenuBar()->Remove( 0 );
-    delete fileMenu;
 }
 
 

kicad/kicad_manager_frame.h

@@ -89,6 +89,7 @@ public:
     void OnBrowseInFileExplorer( wxCommandEvent& event );
 
     void OnFileHistory( wxCommandEvent& event );
+    void OnClearFileHistory( wxCommandEvent& aEvent );
     void OnExit( wxCommandEvent& event );
 
     void ReCreateMenuBar() override;
@@ -119,7 +120,7 @@ public:
 
     void RefreshProjectTree();
 
-    /** 
+    /**
      * Creates a new project by setting up and initial project, schematic, and board files.
      *
      * The project file is copied from the kicad.pro template file if possible.  Otherwise,

kicad/menubar.cpp

@@ -46,18 +46,24 @@ void KICAD_MANAGER_FRAME::ReCreateMenuBar()
 
     //-- File menu -----------------------------------------------------------
     //
-    CONDITIONAL_MENU*         fileMenu = new CONDITIONAL_MENU( false, controlTool );
-    FILE_HISTORY&             fileHistory = PgmTop().GetFileHistory();
-    static FILE_HISTORY_MENU* openRecentMenu;
+    CONDITIONAL_MENU* fileMenu    = new CONDITIONAL_MENU( false, controlTool );
+    FILE_HISTORY&     fileHistory = PgmTop().GetFileHistory();
+
+    fileHistory.SetClearText( _( "Clear Recent Projects" ) );
+
+    static ACTION_MENU* openRecentMenu;
 
     // Create the menu if it does not exist. Adding a file to/from the history
     // will automatically refresh the menu.
     if( !openRecentMenu )
     {
-        openRecentMenu = new FILE_HISTORY_MENU( fileHistory, _( "Clear Recent Projects" ) );
+        openRecentMenu = new ACTION_MENU( false );
         openRecentMenu->SetTool( controlTool );
         openRecentMenu->SetTitle( _( "Open Recent" ) );
         openRecentMenu->SetIcon( recent_xpm );
+
+        fileHistory.UseMenu( openRecentMenu );
+        fileHistory.AddFilesToMenu();
     }
 
     fileMenu->AddItem( KICAD_MANAGER_ACTIONS::newProject,      SELECTION_CONDITIONS::ShowAlways );

pagelayout_editor/files.cpp

@@ -79,6 +79,12 @@ void PL_EDITOR_FRAME::OnFileHistory( wxCommandEvent& event )
 }
 
 
+void PL_EDITOR_FRAME::OnClearFileHistory( wxCommandEvent& aEvent )
+{
+    ClearFileHistory();
+}
+
+
 /* File commands. */
 void PL_EDITOR_FRAME::Files_io( wxCommandEvent& event )
 {

pagelayout_editor/menubar.cpp

@@ -48,17 +48,20 @@ void PL_EDITOR_FRAME::ReCreateMenuBar()
         return IsContentModified();
     };
 
-    static FILE_HISTORY_MENU* openRecentMenu;  // Open Recent submenu, static to remember this menu
-    FILE_HISTORY&             recentFiles = Kiface().GetFileHistory();
+    static ACTION_MENU* openRecentMenu;  // Open Recent submenu, static to remember this menu
+    FILE_HISTORY&       recentFiles = Kiface().GetFileHistory();
 
     // Create the menu if it does not exist. Adding a file to/from the history
     // will automatically refresh the menu.
     if( !openRecentMenu )
     {
-        openRecentMenu = new FILE_HISTORY_MENU( recentFiles );
+        openRecentMenu = new ACTION_MENU( false );
         openRecentMenu->SetTool( selTool );
         openRecentMenu->SetTitle( _( "Open Recent" ) );
         openRecentMenu->SetIcon( recent_xpm );
+
+        recentFiles.UseMenu( openRecentMenu );
+        recentFiles.AddFilesToMenu();
     }
 
     //-- File menu -------------------------------------------------------

pagelayout_editor/pl_editor_frame.cpp

@@ -65,6 +65,7 @@ BEGIN_EVENT_TABLE( PL_EDITOR_FRAME, EDA_DRAW_FRAME )
     EVT_MENU( wxID_FILE, PL_EDITOR_FRAME::Files_io )
 
     EVT_MENU_RANGE( ID_FILE1, ID_FILEMAX, PL_EDITOR_FRAME::OnFileHistory )
+    EVT_MENU( ID_FILE_LIST_CLEAR, PL_EDITOR_FRAME::OnClearFileHistory )
 
     EVT_TOOL( ID_SHOW_REAL_MODE, PL_EDITOR_FRAME::OnSelectTitleBlockDisplayMode )
     EVT_TOOL( ID_SHOW_PL_EDITOR_MODE, PL_EDITOR_FRAME::OnSelectTitleBlockDisplayMode )
@@ -204,12 +205,6 @@ PL_EDITOR_FRAME::~PL_EDITOR_FRAME()
     // Shutdown all running tools
     if( m_toolManager )
         m_toolManager->ShutdownAllTools();
-
-    // Since the file menu contains file history menus, we must ensure that the menu
-    // destructor is called before the file history objects are deleted since their destructor
-    // unregisters the menu from the history.
-    wxMenu* fileMenu = GetMenuBar()->Remove( 0 );
-    delete fileMenu;
 }
 
 void PL_EDITOR_FRAME::setupTools()

pagelayout_editor/pl_editor_frame.h

@@ -252,6 +252,7 @@ public:
     virtual void PrintPage( wxDC* aDC ) override;
 
     void OnFileHistory( wxCommandEvent& event );
+    void OnClearFileHistory( wxCommandEvent& aEvent );
 
     /**
      * @return the filename of the current layout descr file

pcbnew/files.cpp

@@ -202,6 +202,11 @@ void PCB_EDIT_FRAME::OnFileHistory( wxCommandEvent& event )
     }
 }
 
+void PCB_EDIT_FRAME::OnClearFileHistory( wxCommandEvent& aEvent )
+{
+    ClearFileHistory();
+}
+
 
 void PCB_EDIT_FRAME::Files_io( wxCommandEvent& event )
 {

pcbnew/menubar_pcb_editor.cpp

@@ -59,7 +59,7 @@ void PCB_EDIT_FRAME::ReCreateMenuBar()
     //-- File menu -----------------------------------------------------------
     //
     CONDITIONAL_MENU*   fileMenu = new CONDITIONAL_MENU( false, selTool );
-    static FILE_HISTORY_MENU* openRecentMenu;
+    static ACTION_MENU* openRecentMenu;
     auto& disp_opt = GetDisplayOptions();
 
     if( Kiface().IsSingle() )   // not when under a project mgr
@@ -70,10 +70,13 @@ void PCB_EDIT_FRAME::ReCreateMenuBar()
         // will automatically refresh the menu.
         if( !openRecentMenu )
         {
-            openRecentMenu = new FILE_HISTORY_MENU( fileHistory );
+            openRecentMenu = new ACTION_MENU( false );
             openRecentMenu->SetTool( selTool );
             openRecentMenu->SetTitle( _( "Open Recent" ) );
             openRecentMenu->SetIcon( recent_xpm );
+
+            fileHistory.UseMenu( openRecentMenu );
+            fileHistory.AddFilesToMenu();
         }
 
         fileMenu->AddItem( ACTIONS::doNew,           SELECTION_CONDITIONS::ShowAlways );

pcbnew/pcb_edit_frame.cpp

@@ -114,6 +114,7 @@ BEGIN_EVENT_TABLE( PCB_EDIT_FRAME, PCB_BASE_FRAME )
 
     EVT_MENU( ID_IMPORT_NON_KICAD_BOARD, PCB_EDIT_FRAME::Files_io )
     EVT_MENU_RANGE( ID_FILE1, ID_FILEMAX, PCB_EDIT_FRAME::OnFileHistory )
+    EVT_MENU( ID_FILE_LIST_CLEAR, PCB_EDIT_FRAME::OnClearFileHistory )
 
     EVT_MENU( ID_GEN_EXPORT_FILE_GENCADFORMAT, PCB_EDIT_FRAME::ExportToGenCAD )
     EVT_MENU( ID_GEN_EXPORT_FILE_VRML, PCB_EDIT_FRAME::OnExportVRML )
@@ -333,12 +334,6 @@ PCB_EDIT_FRAME::~PCB_EDIT_FRAME()
     // Shutdown all running tools
     if( m_toolManager )
         m_toolManager->ShutdownAllTools();
-
-    // Since the file menu contains file history menus, we must ensure that the menu
-    // destructor is called before the file history objects are deleted since their destructor
-    // unregisters the menu from the history.
-    wxMenu* fileMenu = GetMenuBar()->Remove( 0 );
-    delete fileMenu;
 }
 
 

pcbnew/pcb_edit_frame.h

@@ -578,6 +578,7 @@ public:
     void GenD356File( wxCommandEvent& event );
 
     void OnFileHistory( wxCommandEvent& event );
+    void OnClearFileHistory( wxCommandEvent& aEvent );
 
     /**
      * Function Files_io