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 <vmiklos@collabora.com>
Tested-by: Jenkins

8 files changed, 247 insertions, 3 deletions

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
--- /dev/null
+++ b/embeddedobj/qa/cppunit/data/ole2.png
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 @@
+<reqif-xhtml:div>
+	<reqif-xhtml:object data="ole2.ole" type="text/rtf">
+		<reqif-xhtml:object data="ole2.png" type="image/png">OLE Object</reqif-xhtml:object>
+	</reqif-xhtml:object>
+</reqif-xhtml:div>
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 <test/bootstrapfixture.hxx>
+#include <unotest/macros_test.hxx>
+#include <test/xmltesttools.hxx>
+
+#include <com/sun/star/frame/Desktop.hpp>
+#include <com/sun/star/embed/XStorage.hpp>
+#include <com/sun/star/frame/XStorable.hpp>
+#include <com/sun/star/packages/zip/ZipFileAccess.hpp>
+
+#include <comphelper/embeddedobjectcontainer.hxx>
+#include <comphelper/propertyvalue.hxx>
+#include <comphelper/scopeguard.hxx>
+#include <comphelper/storagehelper.hxx>
+#include <officecfg/Office/Common.hxx>
+#include <unotools/tempfile.hxx>
+#include <osl/thread.hxx>
+#include <vcl/svapp.hxx>
+#include <tools/debug.hxx>
+#include <unotools/ucbstreamhelper.hxx>
+#include <vcl/outdev.hxx>
+
+using namespace ::com::sun::star;
+
+namespace
+{
+/// Covers embeddedobj/source/msole/ fixes.
+class Test : public test::BootstrapFixture, public unotest::MacrosTest, public XmlTestTools
+{
+private:
+    uno::Reference<lang::XComponent> mxComponent;
+
+public:
+    void setUp() override;
+    void tearDown() override;
+    uno::Reference<lang::XComponent>& 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<lang::XComponent> mxComponent;
+    OUString maURL;
+
+public:
+    OdtExportThread(const uno::Reference<lang::XComponent>& xComponent, const OUString& rURL);
+    virtual void SAL_CALL run() override;
+};
+
+OdtExportThread::OdtExportThread(const uno::Reference<lang::XComponent>& xComponent,
+                                 const OUString& rURL)
+    : mxComponent(xComponent)
+    , maURL(rURL)
+{
+}
+
+void OdtExportThread::run()
+{
+    uno::Reference<frame::XStorable> xStorable(mxComponent, uno::UNO_QUERY);
+    uno::Sequence<beans::PropertyValue> 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<beans::PropertyValue> 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<packages::zip::XZipFileAccess2> xNameAccess
+        = packages::zip::ZipFileAccess::createWithURL(mxComponentContext, aTempFile.GetURL());
+    uno::Reference<io::XInputStream> xInputStream(xNameAccess->getByName("content.xml"),
+                                                  uno::UNO_QUERY);
+    std::unique_ptr<SvStream> 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 <osl/file.hxx>
 #include <rtl/ref.hxx>
 #include <o3tl/char16_t2wchar_t.hxx>
+#include <vcl/threadex.hxx>
 
 #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<METAFILEPICT*>(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);
                     }
                 }