[PATCH libreoffice-4-0] hopefully this fixed the strange autorecovery related datalo...

[Noel Power (via Code Review)]

Hi,

I have submitted a patch for review:

    https://gerrit.libreoffice.org/3832

To pull it, you can do:

    git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/32/3832/1

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
---
M basic/source/uno/namecont.cxx
1 file changed, 14 insertions(+), 0 deletions(-)



diff --git a/basic/source/uno/namecont.cxx b/basic/source/uno/namecont.cxx
index c9a67b6..1515b37 100644
--- a/basic/source/uno/namecont.cxx
+++ b/basic/source/uno/namecont.cxx
@@ -1926,6 +1926,19 @@
 
         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 @@
                 }
             }
             else
+#endif
             {
                 uno::Reference< embed::XStorage > xLibraryStor;
                 if( bStorage )

-- 
To view, visit https://gerrit.libreoffice.org/3832
To unsubscribe, visit https://gerrit.libreoffice.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia24e7a6119558497d901370dbc0986101bde4de9
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: libreoffice-4-0
Gerrit-Owner: Noel Power <noel.power at suse.com>