hopefully this fixed the strange autorecovery related dataloss fdo#42899

also this is a fix for bnc#817477
Disabling the optimisation of copying the library container storage
to target storage for the moment ( hopefully after some rework
it might make some sense to re-enable this code ) The problem here is
there is a tragic flaw in the api implementation. In the implementation
the library in-memory model state reflects that the library model
has been saved to storage but not the library container storage
as you ( or at least I ) would expect but actually any storage.
So to illustrate the problem, during autorecovery when the
basic library containers are stored to the autorecovery file the
library pImplLib->implIsModified() is set to false, any subsequent save
attempt will think the library is not modified and will attempt
to the librarycontainer storage to the target one. However, in this case the
source (library container) storage has never been updated with the changes
from memory.
Can't we simply only update the 'implIsModified' state only if the library
container's own storage and the storage to store to are the same ?
Sounds like a good idea, unfortunately this is not possible due to the way
that sfx spaghetti code uses temporary storages for even own copies and
also because it sets the new root storage for the library container
after the library copy happens. ( some stuff in dbaccess appears to
depend on this as well )
AFAICT for any document save/saveas etc. operation the librarycontainer's
own storage and the storage we save to are *always* different.
So for the moment it seems best to *always* write the storage from the
in-memory model.

Change-Id: Ia24e7a6119558497d901370dbc0986101bde4de9
Reviewed-on: https://gerrit.libreoffice.org/3832
Reviewed-by: Fridrich Strba <fridrich@documentfoundation.org>
Tested-by: Fridrich Strba <fridrich@documentfoundation.org>

1 files changed, 14 insertions, 0 deletions

diff --git a/basic/source/uno/namecont.cxx b/basic/source/uno/namecont.cxx
index c9a67b64e627..1515b372ba77 100644
--- a/basic/source/uno/namecont.cxx
+++ b/basic/source/uno/namecont.cxx
@@ -1926,6 +1926,19 @@ void SfxLibraryContainer::storeLibraries_Impl( const uno::Reference< embed::XSto
 
         if( pImplLib->implIsModified() || bComplete )
         {
+// For the moment don't copy storage (as an optimisation )
+// but instead always write to storage from memory.
+// Testing pImplLib->implIsModified() is not reliable,
+// IMHO the value of pImplLib->implIsModified() should
+// reflect whether the library ( in-memory ) model
+// is in sync with the library container's own storage. Currently
+// whenever the library model is written to *any* storage
+// pImplLib->implSetModified( sal_False ) is called
+// The way the code works, especially the way that sfx uses
+// temp storage when saving ( and later sets the root storage of the
+// library container ) and similar madness in dbaccess means some surgery
+// is required to make it possible to successfully use this optimisation
+#if 0
             // Can we simply copy the storage?
             if( !mbOldInfoFormat && !pImplLib->implIsModified() && !mbOasis2OOoFormat && xSourceLibrariesStor.is() )
             {
@@ -1940,6 +1953,7 @@ void SfxLibraryContainer::storeLibraries_Impl( const uno::Reference< embed::XSto
                 }
             }
             else
+#endif
             {
                 uno::Reference< embed::XStorage > xLibraryStor;
                 if( bStorage )