Bug2764: Open Project... in Scriptables shouldn't corrupt project...

... Now new project windows are opened when .aup3 files are opened; but
behavior chages only in that case.

Wherever opening of other files invoked import code, we still do or do
not make a new project in exactly the same cases as before; such as, when
opening multiple files with File > Open, be sure each imported file still opens
in its own separate window.

This means the decision whether to open a new project must be lowered into
ProjectFileManager, where the type of the file is discovered, and we pass it a
function object so it avoids a dependency cycle with ProjectManager.

It also means the checking for errors and closing of new projects in case of
failure must be replicated at all places where ProjectFileManager::OpenProject
is called directly.

The class ProjectManager::ProjectChooser simplifies this.

Recently introduced calls to SafeToOpenProjectInto(), before
ProjectManager::OpenProject(), are now lowered into that class, delaying the
safety check so it might also be called where ProjectFileManager is used
directly.

src/AudacityApp.cpp

@@ -813,8 +813,6 @@ bool AudacityApp::MRUOpen(const FilePath &fullPathStr) {
          if (ProjectFileManager::IsAlreadyOpen(fullPathStr))
             return false;
 
-         if (proj && !ProjectManager::SafeToOpenProjectInto(*proj))
-            proj = nullptr;
          ( void ) ProjectManager::OpenProject( proj, fullPathStr,
                true /* addtohistory */, false /* reuseNonemptyProject */ );
       }

src/ProjectFileManager.cpp

@@ -862,14 +862,9 @@ bool ProjectFileManager::IsAlreadyOpen(const FilePath &projPathName)
    return false;
 }
 
-// FIXME:? TRAP_ERR This should return a result that is checked.
-//    See comment in AudacityApp::MRUOpen().
-AudacityProject *ProjectFileManager::OpenFile(
+AudacityProject *ProjectFileManager::OpenFile( const ProjectChooserFn &chooser,
    const FilePath &fileNameArg, bool addtohistory)
 {
-   auto &project = mProject;
-   auto &window = ProjectWindow::Get( project );
-
    // On Win32, we may be given a short (DOS-compatible) file name on rare
    // occasions (e.g. stuff like "C:\PROGRA~1\AUDACI~1\PROJEC~1.AUP"). We
    // convert these to long file name first.
@@ -902,7 +897,7 @@ AudacityProject *ProjectFileManager::OpenFile(
 "You are trying to open an automatically created backup file.\nDoing this may result in severe data loss.\n\nPlease open the actual Audacity project file instead."),
          XO("Warning - Backup File Detected"),
          wxOK | wxCENTRE,
-         &window);
+         nullptr);
       return nullptr;
    }
 
@@ -911,7 +906,7 @@ AudacityProject *ProjectFileManager::OpenFile(
          XO("Could not open file: %s").Format( fileName ),
          XO("Error Opening File"),
          wxOK | wxCENTRE,
-         &window);
+         nullptr);
       return nullptr;
    }
 
@@ -932,7 +927,7 @@ AudacityProject *ProjectFileManager::OpenFile(
             XO("Could not open file: %s").Format( fileName ),
             XO("Error opening file"),
             wxOK | wxCENTRE,
-            &window);
+            nullptr);
          return nullptr;
       }
 
@@ -943,7 +938,7 @@ AudacityProject *ProjectFileManager::OpenFile(
             XO("File may be invalid or corrupted: \n%s").Format( fileName ),
             XO("Error Opening File or Project"),
             wxOK | wxCENTRE,
-            &window);
+            nullptr);
          return nullptr;
       }
 
@@ -961,6 +956,7 @@ AudacityProject *ProjectFileManager::OpenFile(
 #endif
 #ifdef USE_MIDI
          if (FileNames::IsMidi(fileName)) {
+            auto &project = chooser(false);
             // If this succeeds, indo history is incremented, and it also does
             // ZoomAfterImport:
             if(DoImportMIDI(project, fileName))
@@ -968,18 +964,21 @@ AudacityProject *ProjectFileManager::OpenFile(
             return nullptr;
          }
 #endif
+         auto &project = chooser(false);
          // Undo history is incremented inside this:
-         if (Import(fileName)) {
+         if (Get(project).Import(fileName)) {
+            // Undo history is incremented inside this:
             // Bug 2743: Don't zoom with lof.
             if (!fileName.AfterLast('.').IsSameAs(wxT("lof"), false))
-               window.ZoomAfterImport(nullptr);
+               ProjectWindow::Get(project).ZoomAfterImport(nullptr);
             return &project;
          }
          return nullptr;
       }
    }
 
-   return OpenProjectFile(fileName, addtohistory);
+   auto &project = chooser(true);
+   return Get(project).OpenProjectFile(fileName, addtohistory);
 }
 
 AudacityProject *ProjectFileManager::OpenProjectFile(

src/ProjectFileManager.h

@@ -11,6 +11,7 @@ Paul Licameli split from AudacityProject.h
 #ifndef __AUDACITY_PROJECT_FILE_MANAGER__
 #define __AUDACITY_PROJECT_FILE_MANAGER__
 
+#include <functional>
 #include <memory>
 #include <vector>
 
@@ -81,13 +82,17 @@ public:
 
    static bool IsAlreadyOpen(const FilePath &projPathName);
 
+   //! A function that returns a project to use for opening a file; argument is true if opening a project file
+   using ProjectChooserFn = std::function<AudacityProject&(bool)>;
+
    /*!
     Opens files of many kinds.  In case of import (sound, MIDI, or .aup), the undo history is pushed.
+    @param chooser told whether opening a project file; decides which project to open into
     @param fileName the name and contents are examined to decide a type and open appropriately
     @param addtohistory whether to add .aup3 files to the MRU list (but always done for imports)
     @return if something was successfully opened, the project containing it; else null
     */
-   AudacityProject *OpenFile(
+   static AudacityProject *OpenFile( const ProjectChooserFn &chooser,
       const FilePath &fileName, bool addtohistory = true);
 
    bool Import(const FilePath &fileName,

src/ProjectManager.cpp

@@ -842,9 +842,12 @@ void ProjectManager::OnOpenAudioFile(wxCommandEvent & event)
 {
    const wxString &cmd = event.GetString();
    if (!cmd.empty()) {
-      if (auto project = ProjectFileManager::Get( mProject ).OpenFile(cmd)) {
+      ProjectChooser chooser{ &mProject, true };
+      if (auto project = ProjectFileManager::OpenFile(
+            std::ref(chooser), cmd)) {
          auto &window = GetProjectFrame( *project );
          window.RequestUserAttention();
+         chooser.Commit();
       }
    }
 }
@@ -871,8 +874,6 @@ void ProjectManager::OpenFiles(AudacityProject *proj)
       if (ProjectFileManager::IsAlreadyOpen(fileName))
          continue; // Skip ones that are already open.
 
-      if (proj && !SafeToOpenProjectInto(*proj))
-         proj = nullptr;
       proj = OpenProject( proj, fileName,
          true /* addtohistory */, false /* reuseNonemptyProject */ );
    }
@@ -901,39 +902,60 @@ bool ProjectManager::SafeToOpenProjectInto(AudacityProject &proj)
    return true;
 }
 
-AudacityProject *ProjectManager::OpenProject(
-   AudacityProject *pProject, const FilePath &fileNameArg,
-   bool addtohistory, bool)
+ProjectManager::ProjectChooser::~ProjectChooser()
 {
-   bool success = false;
-   AudacityProject *pNewProject = nullptr;
-   if ( ! pProject )
-      pProject = pNewProject = New();
-   auto cleanup = finally( [&] {
-      if ( pNewProject )
-         GetProjectFrame( *pNewProject ).Close(true);
-      else if ( !success )
+   if (mpUsedProject) {
+      if (mpUsedProject == mpGivenProject) {
          // Ensure that it happens here: don't wait for the application level
          // exception handler, because the exception may be intercepted
-         ProjectHistory::Get(*pProject).RollbackState();
+         ProjectHistory::Get(*mpGivenProject).RollbackState();
          // Any exception now continues propagating
-   } );
-   ProjectFileManager::Get( *pProject ).OpenFile( fileNameArg, addtohistory );
-
-   // The above didn't throw, so change what finally will do
-   success = true;
-   pNewProject = nullptr;
-
-   auto &projectFileIO = ProjectFileIO::Get( *pProject );
-   if( projectFileIO.IsRecovered() ) {
-      auto &window = ProjectWindow::Get( *pProject );
-      window.Zoom( window.GetZoomOfToFit() );
-      // "Project was recovered" replaces "Create new project" in Undo History.
-      auto &undoManager = UndoManager::Get( *pProject );
-      undoManager.RemoveStates(0, 1);
+      }
+      else
+         GetProjectFrame( *mpUsedProject ).Close(true);
    }
+}
 
-   return pProject;
+AudacityProject &
+ProjectManager::ProjectChooser::operator() ( bool openingProjectFile )
+{
+   if (mpGivenProject) {
+      // Always check before opening a project file (for safety);
+      // May check even when opening other files
+      // (to preserve old behavior; as with the File > Open command specifying
+      // multiple files of whatever types, so that each gets its own window)
+      bool checkReuse = (openingProjectFile || !mReuseNonemptyProject);
+      if (!checkReuse || SafeToOpenProjectInto(*mpGivenProject))
+         return *(mpUsedProject = mpGivenProject);
+   }
+   return *(mpUsedProject = New());
+}
+
+void ProjectManager::ProjectChooser::Commit()
+{
+   mpUsedProject = nullptr;
+}
+
+AudacityProject *ProjectManager::OpenProject(
+   AudacityProject *pGivenProject, const FilePath &fileNameArg,
+   bool addtohistory, bool reuseNonemptyProject)
+{
+   ProjectManager::ProjectChooser chooser{ pGivenProject, reuseNonemptyProject };
+   if (auto pProject = ProjectFileManager::OpenFile(
+      std::ref(chooser), fileNameArg, addtohistory )) {
+      chooser.Commit();
+
+      auto &projectFileIO = ProjectFileIO::Get( *pProject );
+      if( projectFileIO.IsRecovered() ) {
+         auto &window = ProjectWindow::Get( *pProject );
+         window.Zoom( window.GetZoomOfToFit() );
+         // "Project was recovered" replaces "Create new project" in Undo History.
+         auto &undoManager = UndoManager::Get( *pProject );
+         undoManager.RemoveStates(0, 1);
+      }
+      return pProject;
+   }
+   return nullptr;
 }
 
 // This is done to empty out the tracks, but without creating a new project.

src/ProjectManager.h

@@ -48,17 +48,52 @@ public:
    //! False when it is unsafe to overwrite proj with contents of an .aup3 file
    static bool SafeToOpenProjectInto(AudacityProject &proj);
 
+   //! Callable object that supplies the `chooser` argument of ProjectFileManager::OpenFile
+   /*!
+    Its operator(), called lower down in ProjectFileManager, decides which project to put new file data into,
+    using file type information deduced there.  It may have the side effect of creating a project.
+
+    At the higher level where it is constructed, it provides conditional RAII.
+    One indicates there that the file opening succeeded by calling Commit().  But if that is never
+    called, creation of projects, or changes to a preexisting project, are undone.
+    */
+   class ProjectChooser {
+   public:
+       /*!
+       @param pProject if not null, an existing project to reuse if possible
+       @param reuseNonemptyProject if true, may reuse the given project when nonempty,
+       but only if importing (not for a project file)
+       */
+      ProjectChooser( AudacityProject *pProject, bool reuseNonemptyProject )
+         : mpGivenProject{ pProject }
+         , mReuseNonemptyProject{ reuseNonemptyProject }
+      {}
+      //! Don't copy.  Use std::ref to pass it to ProjectFileManager
+      ProjectChooser( const ProjectChooser& ) PROHIBITED;
+      //! Destroy any fresh project, or rollback the existing project, unless committed
+      ~ProjectChooser();
+      //! May create a fresh project
+      AudacityProject &operator() ( bool openingProjectFile );
+      //! Commit the creation of any fresh project or changes to the existing project
+      void Commit();
+
+   private:
+      AudacityProject *mpGivenProject;
+      AudacityProject *mpUsedProject = nullptr;
+      bool mReuseNonemptyProject;
+   };
+
    //! Open a file into an AudacityProject, returning the project, or nullptr for failure
    /*!
     If an exception escapes this function, no projects are created.
-    @param pProject if not null, a project that may be reused
+    @param pGivenProject if not null, a project that may be reused
     @param fileNameArg path to the file to open; not always an Audacity project file, may be an import
     @param addtohistory whether to add .aup3 files to the MRU list (but always done for imports)
     @param reuseNonemptyProject if true, may reuse the given project when nonempty,
        but only if importing (not for a project file)
     */
    static AudacityProject *OpenProject(
-      AudacityProject *pProject,
+      AudacityProject *pGivenProject,
       const FilePath &fileNameArg, bool addtohistory, bool reuseNonemptyProject);
 
    void ResetProjectToEmpty();

src/commands/OpenSaveCommands.cpp

@@ -65,8 +65,10 @@ bool OpenProjectCommand::Apply(const CommandContext & context){
    }
    else
    {
-      ProjectFileManager::Get( context.project )
-         .OpenFile(mFileName, mbAddToHistory);
+      ProjectManager::ProjectChooser chooser{ &context.project, true };
+      if(ProjectFileManager::OpenFile(
+         std::ref(chooser), mFileName, mbAddToHistory))
+         chooser.Commit();
    }
    const auto &newFileName = projectFileIO.GetFileName();