tdf#57414 autorecovery: not in RecoveryList? always delete tmp files

In all but one case, the storeToRecoveryFiles were deleted
when the entry was removed from the RecoveryList
(and likely it should have been removed in that case as well,
since the program could not open that file anyway).

So, move that function into the flushConfig function
to ensure that recovery files are not orphaned
in the backup folder.

If the URLs are not cleared, and the recovery tries to load
the non-existing file, then LO will assert (or maybe crash).

(For implts_markDocumentAsSaved,
it does the deletefiles/flushCOnfig outside of the SAFE area,
but updates m_lDocCache with *pIt = aInfo inside the SAFE area,
so those deletefiles/clears cannot be cleaned up.)

(implts_cleanUpWorkingEntry and implts_deregisterDocument
both remove the m_lDocCache entry competely, so it was not
necessary to clear the URLs in those two cases.)

Change-Id: I765f3a815f28082495a7f286a8b28b977e0cad75
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/155118
Reviewed-by: Justin Luth <jluth@mail.com>
Tested-by: Jenkins

1 files changed, 9 insertions, 7 deletions

diff --git a/framework/source/services/autorecovery.cxx b/framework/source/services/autorecovery.cxx
index f0affa0cf7f3..1071423da954 100644
--- a/framework/source/services/autorecovery.cxx
+++ b/framework/source/services/autorecovery.cxx
@@ -575,7 +575,7 @@ private:
     void implts_readAutoSaveConfig();
 
     // TODO document me
-    void implts_flushConfigItem(const AutoRecovery::TDocumentInfo& rInfo, bool bRemoveIt = false,
+    void implts_flushConfigItem(AutoRecovery::TDocumentInfo& rInfo, bool bRemoveIt = false,
                                 bool bAllowAdd = true);
 
     // TODO document me
@@ -1981,7 +1981,7 @@ void AutoRecovery::implts_persistAllActiveViewNames()
     }
 }
 
-void AutoRecovery::implts_flushConfigItem(const AutoRecovery::TDocumentInfo& rInfo, bool bRemoveIt,
+void AutoRecovery::implts_flushConfigItem(AutoRecovery::TDocumentInfo& rInfo, bool bRemoveIt,
                                           bool bAllowAdd)
 {
     std::shared_ptr<comphelper::ConfigurationChanges> batch(
@@ -2007,6 +2007,11 @@ void AutoRecovery::implts_flushConfigItem(const AutoRecovery::TDocumentInfo& rIn
             // DO IT!
             try
             {
+                osl::File::remove(rInfo.OldTempURL);
+                osl::File::remove(rInfo.NewTempURL);
+                rInfo.OldTempURL.clear();
+                rInfo.NewTempURL.clear();
+
                 xModify->removeByName(sID);
             }
             catch(const css::container::NoSuchElementException&)
@@ -2540,8 +2545,6 @@ void AutoRecovery::implts_deregisterDocument(const css::uno::Reference< css::fra
     if (bStopListening)
         implts_stopModifyListeningOnDoc(aInfo);
 
-    AutoRecovery::st_impl_removeFile(aInfo.OldTempURL);
-    AutoRecovery::st_impl_removeFile(aInfo.NewTempURL);
     implts_flushConfigItem(aInfo, true); // sal_True => remove it from config
 }
 
@@ -3323,7 +3326,8 @@ AutoRecovery::ETimerType AutoRecovery::implts_openDocs(const DispatchParams& aPa
                 info.DocumentState |=  DocState::Damaged;
             }
 
-            implts_flushConfigItem(info, true);
+            implts_flushConfigItem(info, /*bRemoveIt=*/true);
+
             implts_informListener(eJob,
                 AutoRecovery::implst_createFeatureStateEvent(eJob, OPERATION_UPDATE, &info));
 
@@ -3898,8 +3902,6 @@ void AutoRecovery::implts_cleanUpWorkingEntry(const DispatchParams& aParams)
     if (pIt != m_lDocCache.end())
     {
         AutoRecovery::TDocumentInfo& rInfo = *pIt;
-        AutoRecovery::st_impl_removeFile(rInfo.OldTempURL);
-        AutoRecovery::st_impl_removeFile(rInfo.NewTempURL);
         implts_flushConfigItem(rInfo, true); // sal_True => remove it from xml config!
 
         m_lDocCache.erase(pIt);