include/sfx2/docfile.hxx | 2 + sfx2/source/doc/docfile.cxx | 11 +++++++ sfx2/source/doc/objstor.cxx | 2 + sw/qa/extras/uiwriter/data/tdf117225.odt |binary sw/qa/extras/uiwriter/uiwriter.cxx | 43 ++++++++++++++++++++++++++++++- 5 files changed, 57 insertions(+), 1 deletion(-)
New commits: commit c1676204447df942e766c0780c1580e1f0427b73 Author: Miklos Vajna <vmik...@collabora.co.uk> Date: Mon May 14 21:47:10 2018 +0200 tdf#117225 sfx2: fix leftover temp file when saving doc with embedded objects Regression from 27938e1bbd5f3405c47b9933be7489eeb03920f3 (sfx2 store: create temp files next to local files (storage case), 2018-01-17), the optimization to store temp files during save next to the destination document (so that it can be renamed and not copied after writing the data successfully) causes problems when we have embedded objects. Avoid the problem by disabling this new optimization when the document has embedded objects. How to fix the actual root cause is not clear to me, I see that: - the SfxMedium::GetOutputStorage() call in SfxObjectShell::SaveTo_Impl() create a temp file - the SfxMedium::Commit() call in SfxObjectShell::SaveTo_Impl() tries to remove the file, which fails on Windows as there is an open file handle to that file - SfxObjectShell::SwitchChildrenPersistance() would close the storage, owning that open file handle, but it's too late So just go back to the previous behavior for now. Change-Id: I37259100d1ddf963c1f2d3b7bd9f460bc995815c Reviewed-on: https://gerrit.libreoffice.org/54340 Tested-by: Jenkins <c...@libreoffice.org> Reviewed-by: Miklos Vajna <vmik...@collabora.co.uk> diff --git a/include/sfx2/docfile.hxx b/include/sfx2/docfile.hxx index 06a7f9623764..ce73db5342f1 100644 --- a/include/sfx2/docfile.hxx +++ b/include/sfx2/docfile.hxx @@ -270,6 +270,8 @@ public: SAL_DLLPRIVATE SignatureState GetCachedSignatureState_Impl(); SAL_DLLPRIVATE void SetCachedSignatureState_Impl( SignatureState nState ); + void SetHasEmbeddedObjects(bool bHasEmbeddedObjects); + static css::uno::Sequence < css::util::RevisionTag > GetVersionList( const css::uno::Reference< css::embed::XStorage >& xStorage ); static OUString CreateTempCopyWithExt( const OUString& aURL ); diff --git a/sfx2/source/doc/docfile.cxx b/sfx2/source/doc/docfile.cxx index cd257837ef30..a589d1134d90 100644 --- a/sfx2/source/doc/docfile.cxx +++ b/sfx2/source/doc/docfile.cxx @@ -288,6 +288,8 @@ public: // in this case the member will hold this information SignatureState m_nSignatureState; + bool m_bHasEmbeddedObjects = false; + util::DateTime m_aDateTime; explicit SfxMedium_Impl(); @@ -3525,6 +3527,10 @@ OUString GetLogicBase(std::unique_ptr<SfxMedium_Impl> const & pImpl) aLogicBase.clear(); } + if (pImpl->m_bHasEmbeddedObjects) + // Embedded objects would mean a special base, ignore that. + aLogicBase.clear(); + return aLogicBase; } } @@ -3828,6 +3834,11 @@ void SfxMedium::SetCachedSignatureState_Impl( SignatureState nState ) pImpl->m_nSignatureState = nState; } +void SfxMedium::SetHasEmbeddedObjects(bool bHasEmbeddedObjects) +{ + pImpl->m_bHasEmbeddedObjects = bHasEmbeddedObjects; +} + bool SfxMedium::HasStorage_Impl() const { return pImpl->xStorage.is(); diff --git a/sfx2/source/doc/objstor.cxx b/sfx2/source/doc/objstor.cxx index 02b5d7bcb0c5..244de5869a34 100644 --- a/sfx2/source/doc/objstor.cxx +++ b/sfx2/source/doc/objstor.cxx @@ -1239,7 +1239,9 @@ bool SfxObjectShell::SaveTo_Impl // if the url is not provided ( means the document is based on a stream ) this code is not // reachable. rMedium.CloseAndRelease(); + rMedium.SetHasEmbeddedObjects(GetEmbeddedObjectContainer().HasEmbeddedObjects()); rMedium.GetOutputStorage(); + rMedium.SetHasEmbeddedObjects(false); } } else if ( !bStorageBasedSource && !bStorageBasedTarget ) diff --git a/sw/qa/extras/uiwriter/data/tdf117225.odt b/sw/qa/extras/uiwriter/data/tdf117225.odt new file mode 100644 index 000000000000..9e31eb6b2090 Binary files /dev/null and b/sw/qa/extras/uiwriter/data/tdf117225.odt differ diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx index 735bbe8f7cd0..2d4aa322db4c 100644 --- a/sw/qa/extras/uiwriter/uiwriter.cxx +++ b/sw/qa/extras/uiwriter/uiwriter.cxx @@ -120,7 +120,29 @@ #include <iodetect.hxx> #include <wrthtml.hxx> -static char const DATA_DIRECTORY[] = "/sw/qa/extras/uiwriter/data/"; +namespace +{ +char const DATA_DIRECTORY[] = "/sw/qa/extras/uiwriter/data/"; + +int CountFilesInDirectory(const OUString &rURL) +{ + int nRet = 0; + + osl::Directory aDir(rURL); + CPPUNIT_ASSERT_EQUAL(osl::FileBase::E_None, aDir.open()); + + osl::DirectoryItem aItem; + osl::FileStatus aFileStatus(osl_FileStatus_Mask_FileURL|osl_FileStatus_Mask_Type); + while (aDir.getNextItem(aItem) == osl::FileBase::E_None) + { + aItem.getFileStatus(aFileStatus); + if (aFileStatus.getFileType() != osl::FileStatus::Directory) + ++nRet; + } + + return nRet; +} +} class SwUiWriterTest : public SwModelTestBase, public HtmlTestTools { @@ -319,6 +341,7 @@ public: void testTdf116403(); void testHtmlCopyImages(); void testTdf116789(); + void testTdf117225(); CPPUNIT_TEST_SUITE(SwUiWriterTest); CPPUNIT_TEST(testReplaceForward); @@ -511,6 +534,7 @@ public: CPPUNIT_TEST(testTdf116403); CPPUNIT_TEST(testHtmlCopyImages); CPPUNIT_TEST(testTdf116789); + CPPUNIT_TEST(testTdf117225); CPPUNIT_TEST_SUITE_END(); private: @@ -6191,6 +6215,23 @@ void SwUiWriterTest::testTdf116789() CPPUNIT_ASSERT_EQUAL(xText1, xText2); } +void SwUiWriterTest::testTdf117225() +{ + // Test that saving a document with an embedded object does not leak + // tempfiles in the directory of the target file. + OUString aTargetDirectory = m_directories.getURLFromWorkdir("/CppunitTest/sw_uiwriter.test.user/"); + OUString aTargetFile = aTargetDirectory + "tdf117225.odt"; + OUString aSourceFile = m_directories.getURLFromSrc(DATA_DIRECTORY) + "tdf117225.odt"; + osl::File::copy(aSourceFile, aTargetFile); + mxComponent = loadFromDesktop(aTargetFile); + uno::Reference<frame::XStorable> xStorable(mxComponent, uno::UNO_QUERY); + int nExpected = CountFilesInDirectory(aTargetDirectory); + xStorable->store(); + int nActual = CountFilesInDirectory(aTargetDirectory); + // nActual was nExpected + 1, i.e. we leaked a tempfile. + CPPUNIT_ASSERT_EQUAL(nExpected, nActual); +} + CPPUNIT_TEST_SUITE_REGISTRATION(SwUiWriterTest); CPPUNIT_PLUGIN_IMPLEMENT(); _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits