From d5cd62164d32273a25913c93aa04be9f7f3a4073 Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Fri, 7 May 2021 12:31:24 +0200 Subject: embeddedobj: handle getting the visible area on a thread The Windows OLE2 implementation of the embedded object interface assumes that the same thread is used for loading and saving the embedded objects. This means that in case the main thread is used for loading (e.g. from remote UNO, but with OnMainThead=true), but a thread is used for storing (without an explicit OnMainThead=true), then the underlying win32 API call failed and we returned a fixed size in EmbeddedObjectRef::GetSize(). Fix the problem by explicitly checking for RPC_E_WRONG_THREAD and adding error handling for that case. Change-Id: Icf1e7722d33a809fa671d1505b2a0155af040c71 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115236 Reviewed-by: Miklos Vajna Tested-by: Jenkins --- embeddedobj/CppunitTest_embeddedobj_msole.mk | 48 +++++++++ embeddedobj/Module_embeddedobj.mk | 6 ++ embeddedobj/qa/cppunit/data/ole2.ole | 19 ++++ embeddedobj/qa/cppunit/data/ole2.png | Bin 0 -> 766 bytes embeddedobj/qa/cppunit/data/reqif-ole2.xhtml | 5 + embeddedobj/qa/cppunit/msole.cxx | 142 +++++++++++++++++++++++++++ embeddedobj/source/msole/olecomponent.cxx | 21 ++++ embeddedobj/source/msole/olevisual.cxx | 9 +- 8 files changed, 247 insertions(+), 3 deletions(-) create mode 100644 embeddedobj/CppunitTest_embeddedobj_msole.mk create mode 100644 embeddedobj/qa/cppunit/data/ole2.ole create mode 100644 embeddedobj/qa/cppunit/data/ole2.png create mode 100644 embeddedobj/qa/cppunit/data/reqif-ole2.xhtml create mode 100644 embeddedobj/qa/cppunit/msole.cxx (limited to 'embeddedobj') diff --git a/embeddedobj/CppunitTest_embeddedobj_msole.mk b/embeddedobj/CppunitTest_embeddedobj_msole.mk new file mode 100644 index 000000000000..2ffb7f64d829 --- /dev/null +++ b/embeddedobj/CppunitTest_embeddedobj_msole.mk @@ -0,0 +1,48 @@ +# -*- Mode: makefile-gmake; tab-width: 4; indent-tabs-mode: t -*- +#************************************************************************* +# +# This file is part of the LibreOffice project. +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +#************************************************************************* + +$(eval $(call gb_CppunitTest_CppunitTest,embeddedobj_msole)) + +$(eval $(call gb_CppunitTest_use_externals,embeddedobj_msole,\ + boost_headers \ + libxml2 \ +)) + +$(eval $(call gb_CppunitTest_add_exception_objects,embeddedobj_msole, \ + embeddedobj/qa/cppunit/msole \ +)) + +$(eval $(call gb_CppunitTest_use_libraries,embeddedobj_msole, \ + comphelper \ + cppu \ + embobj \ + sal \ + test \ + unotest \ + utl \ + vcl \ + tl \ +)) + +$(eval $(call gb_CppunitTest_use_sdk_api,embeddedobj_msole)) + +$(eval $(call gb_CppunitTest_use_ure,embeddedobj_msole)) +$(eval $(call gb_CppunitTest_use_vcl,embeddedobj_msole)) + +$(eval $(call gb_CppunitTest_use_rdb,embeddedobj_msole,services)) + +$(eval $(call gb_CppunitTest_use_custom_headers,embeddedobj_msole,\ + officecfg/registry \ +)) + +$(eval $(call gb_CppunitTest_use_configuration,embeddedobj_msole)) + +# vim: set noet sw=4 ts=4: diff --git a/embeddedobj/Module_embeddedobj.mk b/embeddedobj/Module_embeddedobj.mk index 394112a13b7d..659b406b9ea9 100644 --- a/embeddedobj/Module_embeddedobj.mk +++ b/embeddedobj/Module_embeddedobj.mk @@ -21,4 +21,10 @@ $(eval $(call gb_Module_add_slowcheck_targets,embeddedobj,\ )) endif +ifeq ($(OS),WNT) +$(eval $(call gb_Module_add_slowcheck_targets,embeddedobj,\ + CppunitTest_embeddedobj_msole \ +)) +endif + # vim: set noet sw=4 ts=4: diff --git a/embeddedobj/qa/cppunit/data/ole2.ole b/embeddedobj/qa/cppunit/data/ole2.ole new file mode 100644 index 000000000000..c0013db40113 --- /dev/null +++ b/embeddedobj/qa/cppunit/data/ole2.ole @@ -0,0 +1,19 @@ +{\object\objemb\objw240\objh240{\*\objclass PBrush}{\*\objdata 01050000020000000700000050427275736800000000000000000040030000 +424d36030000000000003600000028000000100000001000000001001800000000000003000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff +ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff +ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff +ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff +ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff +ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff +ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0000000000000000000001050000050000000d0000004d45544146494c455049435400a701000059feffffe40000000800a701a7010000 +0100090000036e00000000004500000000000400000003010800050000000b0200000000050000000c0211001100030000001e000400000007010400040000000701040045000000410b2000cc00100010000000000010001000000000002800000010000000100000000100010000000000000000000000000000000000 +000000000000000000000000ffffff00ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101040000002701ffff030000000000}{\result {\rtlch\fcs1 \af31507 \ltrch\fcs0 \insrsid793696 +{\*\shppict{\pict{\*\picprop\shplid1027{\sp{\sn shapeType}{\sv 75}}{\sp{\sn fFlipH}{\sv 0}}{\sp{\sn fFlipV}{\sv 0}}{\sp{\sn fLockAspectRatio}{\sv 1}}{\sp{\sn pictureGray}{\sv 0}} +{\sp{\sn pictureBiLevel}{\sv 0}}{\sp{\sn pictureActive}{\sv 0}}{\sp{\sn fRecolorFillAsPicture}{\sv 0}}{\sp{\sn fUseShapeAnchor}{\sv 0}}{\sp{\sn fFilled}{\sv 0}}{\sp{\sn fHitTestFill}{\sv 1}} +{\sp{\sn fillShape}{\sv 1}}{\sp{\sn fillUseRect}{\sv 0}}{\sp{\sn fNoFillHitTest}{\sv 0}}{\sp{\sn fLine}{\sv 0}}{\sp{\sn fPreferRelativeResize}{\sv 1}}{\sp{\sn fReallyHidden}{\sv 0}} +{\sp{\sn fScriptAnchor}{\sv 0}}{\sp{\sn fFakeMaster}{\sv 0}}{\sp{\sn fCameFromImgDummy}{\sv 0}}{\sp{\sn fLayoutInCell}{\sv 1}}}\picscalex100\picscaley100\piccropl0\piccropr0\piccropt0\piccropb0 +\picw423\pich423\picwgoal240\pichgoal240\pngblip\bliptag602933164{\*\blipuid 23f007ac3ac2aed53753eaea27c13e03}89504e470d0a1a0a0000000d494844520000001000000010080200000090916836000000017352474200aece1ce9000000097048597300000ec700000ec70138 +922f760000001f49444154384f63fcffff3f0329808914c520b5a31a8809b1d1501a1ca10400556d031d6ec895ac0000000049454e44ae426082}}{\nonshppict{\pict\picscalex100\picscaley100\piccropl0\piccropr0\piccropt0\piccropb0 +\picw423\pich423\picwgoal240\pichgoal240\wmetafile8\bliptag602933164\blipupi96{\*\blipuid 23f007ac3ac2aed53753eaea27c13e03}0100090000036e00000000004500000000000400000003010800050000000b0200000000050000000c0211001100030000001e00040000000701040004000000 +0701040045000000410b2000cc001000100000000000100010000000000028000000100000001000000001000100000000000000000000000000000000000000 +00000000000000000000ffffff00ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101ffff0101040000002701ffff030000000000}}}}} diff --git a/embeddedobj/qa/cppunit/data/ole2.png b/embeddedobj/qa/cppunit/data/ole2.png new file mode 100644 index 000000000000..fdad35484e7c Binary files /dev/null and b/embeddedobj/qa/cppunit/data/ole2.png differ diff --git a/embeddedobj/qa/cppunit/data/reqif-ole2.xhtml b/embeddedobj/qa/cppunit/data/reqif-ole2.xhtml new file mode 100644 index 000000000000..716ecd1bda63 --- /dev/null +++ b/embeddedobj/qa/cppunit/data/reqif-ole2.xhtml @@ -0,0 +1,5 @@ + + + OLE Object + + diff --git a/embeddedobj/qa/cppunit/msole.cxx b/embeddedobj/qa/cppunit/msole.cxx new file mode 100644 index 000000000000..6cd768ead146 --- /dev/null +++ b/embeddedobj/qa/cppunit/msole.cxx @@ -0,0 +1,142 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include +#include +#include + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace ::com::sun::star; + +namespace +{ +/// Covers embeddedobj/source/msole/ fixes. +class Test : public test::BootstrapFixture, public unotest::MacrosTest, public XmlTestTools +{ +private: + uno::Reference mxComponent; + +public: + void setUp() override; + void tearDown() override; + uno::Reference& getComponent() { return mxComponent; } + void registerNamespaces(xmlXPathContextPtr& pXmlXpathCtx) override; +}; +} + +void Test::setUp() +{ + test::BootstrapFixture::setUp(); + + mxDesktop.set(frame::Desktop::create(mxComponentContext)); +} + +void Test::tearDown() +{ + if (mxComponent.is()) + mxComponent->dispose(); + + test::BootstrapFixture::tearDown(); +} + +void Test::registerNamespaces(xmlXPathContextPtr& pXmlXpathCtx) +{ + XmlTestTools::registerODFNamespaces(pXmlXpathCtx); +} + +class OdtExportThread : public osl::Thread +{ + uno::Reference mxComponent; + OUString maURL; + +public: + OdtExportThread(const uno::Reference& xComponent, const OUString& rURL); + virtual void SAL_CALL run() override; +}; + +OdtExportThread::OdtExportThread(const uno::Reference& xComponent, + const OUString& rURL) + : mxComponent(xComponent) + , maURL(rURL) +{ +} + +void OdtExportThread::run() +{ + uno::Reference xStorable(mxComponent, uno::UNO_QUERY); + uno::Sequence aStoreProperties = { + comphelper::makePropertyValue("FilterName", OUString("writer8")), + }; + xStorable->storeToURL(maURL, aStoreProperties); +} + +CPPUNIT_TEST_FIXTURE(Test, testSaveOnThread) +{ + // Given an embedded object which hosts mspaint data: + if (Application::GetDefaultDevice()->GetDPIX() != 96) + { + return; + } + + DBG_TESTSOLARMUTEX(); + OUString aURL = m_directories.getURLFromSrc(u"embeddedobj/qa/cppunit/data/reqif-ole2.xhtml"); + uno::Sequence aLoadProperties = { + comphelper::makePropertyValue("FilterName", OUString("HTML (StarWriter)")), + comphelper::makePropertyValue("FilterOptions", OUString("xhtmlns=reqif-xhtml")), + }; + getComponent().set(loadFromDesktop(aURL, "com.sun.star.text.TextDocument", aLoadProperties)); + + // When saving that document on a thread: + utl::TempFile aTempFile; + aTempFile.EnableKillingFile(); + OdtExportThread aThread(getComponent(), aTempFile.GetURL()); + aThread.create(); + { + SolarMutexReleaser r; + while (aThread.isRunning()) + { + SolarMutexGuard g; + Application::Reschedule(/*bHandleAllCurrentEvents=*/true); + } + } + + // Then make sure its visible area's width is correct. + uno::Reference xNameAccess + = packages::zip::ZipFileAccess::createWithURL(mxComponentContext, aTempFile.GetURL()); + uno::Reference xInputStream(xNameAccess->getByName("content.xml"), + uno::UNO_QUERY); + std::unique_ptr pStream(utl::UcbStreamHelper::CreateStream(xInputStream, true)); + xmlDocUniquePtr pXmlDoc = parseXmlStream(pStream.get()); + // 16 pixels, assuming 96 DPI. + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 0.1665in + // - Actual : 1.9685in + // i.e. we wrote a hardcoded 5cm width, not the real one. + assertXPath(pXmlDoc, "//style:graphic-properties", "visible-area-width", "0.1665in"); +} + +CPPUNIT_PLUGIN_IMPLEMENT(); + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/embeddedobj/source/msole/olecomponent.cxx b/embeddedobj/source/msole/olecomponent.cxx index 9843e4999f44..19818bea342e 100644 --- a/embeddedobj/source/msole/olecomponent.cxx +++ b/embeddedobj/source/msole/olecomponent.cxx @@ -38,6 +38,7 @@ #include #include #include +#include #include "graphconvert.hxx" #include "olecomponent.hxx" @@ -1131,6 +1132,18 @@ awt::Size OleComponent::GetExtent( sal_Int64 nAspect ) aFormat.dwAspect = nMSAspect; hr = pDataObject->GetData( &aFormat, &aMedium ); + + if (hr == RPC_E_WRONG_THREAD) + { + // Assume that the OLE object was loaded on the main thread. + vcl::solarthread::syncExecute([this, &hr, &pDataObject, &aFormat, &aMedium]() { + // Make sure that the current state is embed::EmbedStates::RUNNING. + RunObject(); + // Now try again on the correct thread. + hr = pDataObject->GetData(&aFormat, &aMedium); + }); + } + if ( SUCCEEDED( hr ) && aMedium.tymed == TYMED_MFPICT ) // Win Metafile { METAFILEPICT* pMF = static_cast(GlobalLock( aMedium.hMetaFilePict )); @@ -1179,6 +1192,10 @@ awt::Size OleComponent::GetExtent( sal_Int64 nAspect ) OSL_FAIL( "Unexpected size is provided!" ); } } + else if (!SUCCEEDED(hr)) + { + SAL_WARN("embeddedobj.ole", " OleComponent::GetExtent: GetData() failed"); + } // i113605, to release storage medium if ( SUCCEEDED( hr ) ) ::ReleaseStgMedium(&aMedium); @@ -1211,6 +1228,7 @@ awt::Size OleComponent::GetCachedExtent( sal_Int64 nAspect ) //else // throw io::IOException(); // TODO + SAL_WARN("embeddedobj.ole", " OleComponent::GetCachedExtent: GetExtent() failed"); throw lang::IllegalArgumentException(); } @@ -1227,7 +1245,10 @@ awt::Size OleComponent::GetRecommendedExtent( sal_Int64 nAspect ) SIZEL aSize; HRESULT hr = m_pNativeImpl->m_pOleObject->GetExtent( nMSAspect, &aSize ); if ( FAILED( hr ) ) + { + SAL_WARN("embeddedobj.ole", " OleComponent::GetRecommendedExtent: GetExtent() failed"); throw lang::IllegalArgumentException(); + } return awt::Size( aSize.cx, aSize.cy ); } diff --git a/embeddedobj/source/msole/olevisual.cxx b/embeddedobj/source/msole/olevisual.cxx index 19084209e7d3..b56357ddbb29 100644 --- a/embeddedobj/source/msole/olevisual.cxx +++ b/embeddedobj/source/msole/olevisual.cxx @@ -225,8 +225,9 @@ awt::Size SAL_CALL OleEmbeddedObject::getVisualAreaSize( sal_Int64 nAspect ) aSize = m_pOleComponent->GetExtent( nAspect ); // will throw an exception in case of failure bSuccess = true; } - catch( const uno::Exception& ) + catch( const uno::Exception& rException ) { + SAL_WARN("embeddedobj.ole", "OleEmbeddedObject::getVisualAreaSize: GetExtent() failed: " << rException); } if (bBackToLoaded) @@ -249,8 +250,9 @@ awt::Size SAL_CALL OleEmbeddedObject::getVisualAreaSize( sal_Int64 nAspect ) aSize = m_pOleComponent->GetCachedExtent( nAspect ); // will throw an exception in case of failure bSuccess = true; } - catch( const uno::Exception& ) + catch( const uno::Exception& rException ) { + SAL_WARN("embeddedobj.ole", "OleEmbeddedObject::getVisualAreaSize: GetCachedExtent() failed: " << rException); } } @@ -262,8 +264,9 @@ awt::Size SAL_CALL OleEmbeddedObject::getVisualAreaSize( sal_Int64 nAspect ) aSize = m_pOleComponent->GetRecommendedExtent( nAspect ); // will throw an exception in case of failure bSuccess = true; } - catch( const uno::Exception& ) + catch( const uno::Exception& rException ) { + SAL_WARN("embeddedobj.ole", "OleEmbeddedObject::getVisualAreaSize: GetRecommendedExtent() failed: " << rException); } } -- cgit