diff options
authoranwilli5 <>2016-06-05 23:06:05 -0400
committerAndras Timar <>2016-08-23 10:48:18 +0200
commit084427ccf6690ab80b7ee58d1475d4e528a30d82 (patch)
parentafa3f8c0b825f50c017dc48bffe74c15f6ec403f (diff)
tdf#96607 'Save as' doesn't update global auto-recovery state
The auto-recovery service maintains a list of structures (one for each open document) containing information needed to carry out the auto-save functionality. One such piece of information is the location of the backup file, stored in a struct member named 'OldTempURL'. At every auto-save interval, this list is iterated through and a function (implts_saveOneDoc) is called during each iteration to save the current state of the associated document. The algorithm works as follows: 1. A new backup file URL is chosen so as not to conflict with any already existing backup files in the backup directory. This URL is based on the file name and incorporates a number (starting at 0) that is incremented until a name is chosen that doesn't conflict. 2. The document is saved to this new backup file URL 3. The previous backup file (indicated by its structure's 'OldTempURL') is deleted 4. The new backup file URL is stored (in its structure's 'OldTempURL') for the next time the file needs to be saved. Assuming you start with a new Writer doc and then make some changes, when it is time to auto-save, the backup file name 'untitled_0.odt' (excluding path) will be selected, the latest state of the open file will be written to that backup file, and the full URL for the backup file will be saved into the struct 'OldTempURL' member. The next time changes are made and an auto-save occurs, this algorithm will result in the name 'untitled_1.odt' being selected, the file contents saved into this new file, 'untitled_0.odt' being deleted, and the full URL for the new backup file being saved in 'OldTempURL'. The third time through results in 'untitled_0.odt' being selected (since this file doesn't exist on disk), and subsequent iterations of auto-saving cause the backup file name to alternate between the two aforementioned. The problem occurs during a 'Save as' operation. When this happens, the backup file is deleted (which is fine - it was just saved, and the next auto-save will back it up) but 'OldTempURL' is not properly reset (see below for more info.) During the next auto-save, 'untitled_0.odt' will be selected for the new backup file name (since no file exists by this name), and one of two things will happen (based on how many auto-saves have occurred): 1. 'OldTempURL' points to 'untitled_1.odt', and the algorithm above continues to work correctly (at least in that it continues to backup file contents.) 2. 'OldTempURL' points to 'untitled_0.odt', the name chosen for the new backup file. In this case, the document contents will be saved to this file (step 2) but then the file will be deleted (step 3). 'OldTempURL' will maintain this URL from then on out, causing this case to be hit for all future auto-save intervals. So, 50% of the time (30 minutes out of every hour) auto-save will stop backing up file contents on a 'Save as'. The function that handles the 'Save as' case (implts_markDocumentAsSaved) clears 'OldTempURL' and sets other relavent struct members for a local variable copy of the global struct, but doesn't copy them back. :( These changes are effectively lost when the function returns. There are several other cases where this appears to be happening as well, but more work is needed to determine whether this is actually the case: - implts_prepareSessionShutdown - implts_saveDocs, handling the 'dangerousDocs' and in a few other places - implts_openDocs - implts_resetHandleStates Also, there is some JUnitTest code for auto-save, but it is currently disabled (and fails to run successfully.) It'd be great to get these working again, or to just write python equivalents. Implementing this would like take me a while, though, so for now I just tested manually to ensure that this fixes the issue. When I have some more time I'd like to work more on this, but I wanted to send this patch in for now to address bug #96607. This may also address bug #99890, since some of the struct members that don't make it into the global state relate to the file name. I haven't explicitly tested this case, though. Change-Id: Ic702d6f78e60c7cf828a1564ccca118dd45d152b Reviewed-on: Tested-by: Jenkins <> Reviewed-by: jan iversen <> Reviewed-on: Reviewed-by: Eike Rathke <> (cherry picked from commit e3759a3154de534a5edfd69f7face5b5c29c1ea9)
1 files changed, 13 insertions, 0 deletions
diff --git a/framework/source/services/autorecovery.cxx b/framework/source/services/autorecovery.cxx
index 7365cb770bb8..554481971036 100644
--- a/framework/source/services/autorecovery.cxx
+++ b/framework/source/services/autorecovery.cxx
@@ -2689,11 +2689,22 @@ void AutoRecovery::implts_markDocumentAsSaved(const css::uno::Reference< css::fr
aInfo = *pIt;
+ /* Since the document has been saved, update its entry in the document
+ * cache. We essentially reset the state of the document from an
+ * autorecovery perspective, updating things like the filename (which
+ * would change in the case of a 'Save as' operation) and the associated
+ * backup file URL. */
aInfo.DocumentState = AutoRecovery::E_UNKNOWN;
// TODO replace getLocation() with getURL() ... it's a workaround currently only!
css::uno::Reference< css::frame::XStorable > xDoc(aInfo.Document, css::uno::UNO_QUERY);
aInfo.OrgURL = xDoc->getLocation();
+ /* Save off the backup file URLs and then clear them. NOTE - it is
+ * important that we clear them - otherwise, we could enter a state
+ * where pIt->OldTempURL == pIt->NewTempURL and our backup algorithm
+ * in implts_saveOneDoc will write to that URL and then delete the file
+ * at that URL (bug #96607) */
sRemoveURL1 = aInfo.OldTempURL;
sRemoveURL2 = aInfo.NewTempURL;
@@ -2714,6 +2725,8 @@ void AutoRecovery::implts_markDocumentAsSaved(const css::uno::Reference< css::fr
aInfo.UsedForSaving = false;
+ *pIt = aInfo;
} /* SAFE */