Make replacement graphic management more atomic

Commit 8872f7121b4ae4dd0b51820366d3510a88f7aac2 (crashtesting: crash
on exporting kde274105-6.docx to .rtf, 2024-03-27) added some safety
code in EmbeddedObjectRef::GetReplacement. It mentioned, that there
are likely some bugs in the management of the graphic.

This tries to fix this management, avoiding the intermediate states,
and only changing the graphic when all the data is ready. This also
reverts the changes of the mentioned commit, obsoleted now; and of
commit 8780fa41dcd164af244742461f4e57a4bcf4c7a4 (svtools: fix lost
replacement grpahic when updating it via OLE fails, 2018-10-30);
but keeps commit 24231f2a336c50eac5e6a1621c57e60ebe1e1cf4 (svtools:
fix lost replacement non-rendered graphic when updating it fails,
2022-02-17).

This has revealed that the second part of unit test for tdf#143222
("Check export of embedded worksheet in slide"), introduced in
commit 92a407b7f90a98704a238c5ffa3a3491eaf3263a (tdf143222 Handle
alternate content of graphicData element., 2021-07-08), has never
really worked: the "pGraphic != nullptr" check would never fail;
in fact, that used to always return an empty graphic. The problem
was filed as tdf#163064, and the test was modified accordingly.

Commit 5d997c029e53c31a3651a08f5012645097cec48f (sw XHTML export:
improve dummy OLE object handling, 2018-08-30) made ReqIF export
handle missing replacement graphic. However, it had assumed that
SwOLENode::GetGraphic always returns a valid pointer even for the
missing data. That is fixed here in OutHTML_FrameFormatOLENodeGrf.

Other places, where the pointer was dereferenced unconditionally,
were fixed (keeping current behavior).

Change-Id: Ica97a691ecc11b856bdb003f89467ea3392684bd
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/173716
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

6 files changed, 19 insertions, 17 deletions

diff --git a/sw/qa/core/view/view.cxx b/sw/qa/core/view/view.cxx
index 2c6655a659b7..b130827deb8d 100644
--- a/sw/qa/core/view/view.cxx
+++ b/sw/qa/core/view/view.cxx
@@ -60,10 +60,11 @@ CPPUNIT_TEST_FIXTURE(Test, testUpdateOleObjectPreviews)
         SwOLENode* pOleNode = pNode->GetOLENode();
         CPPUNIT_ASSERT(pOleNode);
         SwOLEObj& rOleObj = pOleNode->GetOLEObj();
-        svt::EmbeddedObjectRef& rObject = rOleObj.GetObject();
+        const Graphic* pGraphic = rOleObj.GetObject().GetGraphic();
         // Without the accompanying fix in place, this test would have failed, the update broke the
         // preview of the second embedded object.
-        CPPUNIT_ASSERT(!rObject.GetGraphic()->IsNone());
+        CPPUNIT_ASSERT(pGraphic);
+        CPPUNIT_ASSERT(!pGraphic->IsNone());
     }
 }
 
diff --git a/sw/source/core/docnode/ndnotxt.cxx b/sw/source/core/docnode/ndnotxt.cxx
index d044f4e90dd3..0fdb56751023 100644
--- a/sw/source/core/docnode/ndnotxt.cxx
+++ b/sw/source/core/docnode/ndnotxt.cxx
@@ -231,7 +231,8 @@ Graphic SwNoTextNode::GetGraphic() const
     else
     {
         OSL_ENSURE( GetOLENode(), "new type of Node?" );
-        aRet = *const_cast<SwOLENode*>(static_cast<const SwOLENode*>(this))->SwOLENode::GetGraphic();
+        if (const Graphic* pGraphic = const_cast<SwOLENode*>(static_cast<const SwOLENode*>(this))->SwOLENode::GetGraphic())
+            aRet = *pGraphic;
     }
     return aRet;
 }
diff --git a/sw/source/filter/html/htmlplug.cxx b/sw/source/filter/html/htmlplug.cxx
index 3e594bbf8b43..238b09dbc2dc 100644
--- a/sw/source/filter/html/htmlplug.cxx
+++ b/sw/source/filter/html/htmlplug.cxx
@@ -1649,13 +1649,10 @@ SwHTMLWriter& OutHTML_FrameFormatOLENodeGrf( SwHTMLWriter& rWrt, const SwFrameFo
     if (TrySaveFormulaAsPDF(rWrt, rFrameFormat, pOLENd, bWriteReplacementGraphic, bInCntnr))
         return rWrt;
 
-    if ( !pOLENd->GetGraphic() )
-    {
-        SAL_WARN("sw.html", "Unexpected missing OLE fallback graphic");
-        return rWrt;
-    }
-
-    Graphic aGraphic( *pOLENd->GetGraphic() );
+    // Missing fallback graphic must not give up exporting the document, and also must produce
+    // valid output (see OutHTMLGraphic)
+    const Graphic* pFallbackGraphic = pOLENd->GetGraphic();
+    Graphic aGraphic(pFallbackGraphic ? *pFallbackGraphic : Graphic());
 
     SwDocShell* pDocSh = rWrt.m_pDoc->GetDocShell();
     bool bObjectOpened = false;
diff --git a/sw/source/filter/html/htmlreqifreader.cxx b/sw/source/filter/html/htmlreqifreader.cxx
index d0c059037231..9493be474282 100644
--- a/sw/source/filter/html/htmlreqifreader.cxx
+++ b/sw/source/filter/html/htmlreqifreader.cxx
@@ -223,7 +223,8 @@ OString InsertOLE1HeaderFromOle10NativeStream(const rtl::Reference<SotStorage>&
         return aClassName;
     }
 
-    const Graphic& rGraphic = *rOLENode.GetGraphic();
+    const Graphic* pGraphic = rOLENode.GetGraphic();
+    const Graphic rGraphic = pGraphic ? *pGraphic : Graphic();
     Size aSize = rOLENode.GetTwipSize();
     SvMemoryStream aGraphicStream;
     if (GraphicConverter::Export(aGraphicStream, rGraphic, ConvertDataFormat::WMF) != ERRCODE_NONE)
diff --git a/sw/source/filter/ww8/docxattributeoutput.cxx b/sw/source/filter/ww8/docxattributeoutput.cxx
index 60646c0f2a72..6b38d40f1ad5 100644
--- a/sw/source/filter/ww8/docxattributeoutput.cxx
+++ b/sw/source/filter/ww8/docxattributeoutput.cxx
@@ -5226,8 +5226,8 @@ void DocxAttributeOutput::FlyFrameGraphic( const SwGrfNode* pGrfNode, const Size
         Graphic aGraphic;
         if (pGrfNode)
             aGraphic = pGrfNode->GetGrf();
-        else
-            aGraphic = *pOLENode->GetGraphic();
+        else if (const Graphic* pGraphic = pOLENode->GetGraphic())
+            aGraphic = *pGraphic;
 
         m_rDrawingML.SetFS(m_pSerializer); // to be sure that we write to the right stream
         auto pGraphicExport = m_rDrawingML.createGraphicExport();
@@ -5876,8 +5876,9 @@ void DocxAttributeOutput::WriteOLE( SwOLENode& rNode, const Size& rSize, const S
 
     // write preview image
     const Graphic* pGraphic = rNode.GetGraphic();
+    Graphic aGraphic = pGraphic ? *pGraphic : Graphic();
     m_rDrawingML.SetFS(m_pSerializer);
-    OUString sImageId = m_rDrawingML.writeGraphicToStorage(*pGraphic);
+    OUString sImageId = m_rDrawingML.writeGraphicToStorage(aGraphic);
 
     if ( sDrawAspect == "Content" )
     {
diff --git a/sw/source/filter/ww8/rtfattributeoutput.cxx b/sw/source/filter/ww8/rtfattributeoutput.cxx
index 2a910cc9aced..dd0d581257fa 100644
--- a/sw/source/filter/ww8/rtfattributeoutput.cxx
+++ b/sw/source/filter/ww8/rtfattributeoutput.cxx
@@ -4389,12 +4389,13 @@ void RtfAttributeOutput::FlyFrameOLEReplacement(const SwFlyFrameFormat* pFlyFram
     aRendered.setWidth(rSize.Width());
     aRendered.setHeight(rSize.Height());
     const Graphic* pGraphic = rOLENode.GetGraphic();
-    Size aMapped(pGraphic->GetPrefSize());
+    Graphic aGraphic = pGraphic ? *pGraphic : Graphic();
+    Size aMapped(aGraphic.GetPrefSize());
     auto& rCr = rOLENode.GetAttr(RES_GRFATR_CROPGRF);
     const char* pBLIPType = OOO_STRING_SVTOOLS_RTF_PNGBLIP;
     const sal_uInt8* pGraphicAry = nullptr;
     SvMemoryStream aStream;
-    if (GraphicConverter::Export(aStream, *pGraphic, ConvertDataFormat::PNG) != ERRCODE_NONE)
+    if (GraphicConverter::Export(aStream, aGraphic, ConvertDataFormat::PNG) != ERRCODE_NONE)
         SAL_WARN("sw.rtf", "failed to export the graphic");
     sal_uInt64 nSize = aStream.TellEnd();
     pGraphicAry = static_cast<sal_uInt8 const*>(aStream.GetData());
@@ -4404,7 +4405,7 @@ void RtfAttributeOutput::FlyFrameOLEReplacement(const SwFlyFrameFormat* pFlyFram
     m_aRunText->append("{" OOO_STRING_SVTOOLS_RTF_NONSHPPICT);
     pBLIPType = OOO_STRING_SVTOOLS_RTF_WMETAFILE;
     SvMemoryStream aWmfStream;
-    if (GraphicConverter::Export(aWmfStream, *pGraphic, ConvertDataFormat::WMF) != ERRCODE_NONE)
+    if (GraphicConverter::Export(aWmfStream, aGraphic, ConvertDataFormat::WMF) != ERRCODE_NONE)
         SAL_WARN("sw.rtf", "failed to export the graphic");
     nSize = aWmfStream.TellEnd();
     pGraphicAry = static_cast<sal_uInt8 const*>(aWmfStream.GetData());