framework/source/services/autorecovery.cxx | 13 +++++++++++++
1 file changed, 13 insertions(+)
New commits:
commit 9e28b2f9ac3f205db223352cb88eb3546a8e1c0e
Author: anwilli5 <anwil...@ncsu.edu>
Date: Sun Jun 5 23:06:05 2016 -0400
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: https://gerrit.libreoffice.org/25948
Tested-by: Jenkins <c...@libreoffice.org>
Reviewed-by: jan iversen <j...@documentfoundation.org>
diff --git a/framework/source/services/autorecovery.cxx
b/framework/source/services/autorecovery.cxx
index 0a1655b..cf97b6d 100644
--- a/framework/source/services/autorecovery.cxx
+++ b/framework/source/services/autorecovery.cxx
@@ -2667,11 +2667,22 @@ void AutoRecovery::implts_markDocumentAsSaved(const
css::uno::Reference< css::fr
return;
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;
aInfo.OldTempURL.clear();
@@ -2692,6 +2703,8 @@ void AutoRecovery::implts_markDocumentAsSaved(const
css::uno::Reference< css::fr
aInfo.UsedForSaving = false;
+ *pIt = aInfo;
+
} /* SAFE */
implts_flushConfigItem(aInfo);
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
