svtools: fix lost replacement non-rendered graphic when updating it fails

This is similar to commit 8780fa41dcd164af244742461f4e57a4bcf4c7a4
(svtools: fix lost replacement grpahic when updating it via OLE fails,
2018-10-30), but that case was when we already had an old
mpImpl->pGraphic, the updated failed and then we already lost the old
one but didn't have a new one.

Here what happened is that in case tools -> update -> update-all was
used for an OLE object which had bad native data but a good preview,
then the result was bad preview, depending on if you first scrolled to
the OLE object to trigger rendering (good) or not (bad).

The reason for this is that scrolling to the object calls GetGraphic(),
which sets mpImpl->pGraphic using GetReplacement(bUpdate=false), which
works, but svt::EmbeddedObjectRef::UpdateReplacement() calls
GetReplacement(bUpdate=true). That explains why the update breaks the
preview, but not when scrolling to it first.

Fix the problem by improving svt::EmbeddedObjectRef::GetReplacement():
if getting an updated graphic fails, try to get a non-updated graphic.

The result is that GetGraphic() after an UpdateReplacement() not only
always return a non-nullptr Graphic, but also it's no longer of type
None.

Change-Id: I8e5ff4aaaefdc58e032b325bb4001f2a604ccc8a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/130086
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

5 files changed, 151 insertions, 1 deletions

diff --git a/svtools/source/misc/embedhlp.cxx b/svtools/source/misc/embedhlp.cxx
index 4f4a5ce3e364..6e56dc3137e8 100644
--- a/svtools/source/misc/embedhlp.cxx
+++ b/svtools/source/misc/embedhlp.cxx
@@ -471,6 +471,15 @@ void EmbeddedObjectRef::GetReplacement( bool bUpdate )
     }
 
     std::unique_ptr<SvStream> pGraphicStream(GetGraphicStream( bUpdate ));
+    if (!pGraphicStream && aOldGraphic.IsNone())
+    {
+        // We have no old graphic, tried to get an updated one, but that failed. Try to get an old
+        // graphic instead of having no graphic at all.
+        pGraphicStream = GetGraphicStream(false);
+        SAL_WARN("svtools.misc",
+                 "EmbeddedObjectRef::GetReplacement: failed to get updated graphic stream");
+    }
+
     if ( pGraphicStream )
     {
         GraphicFilter& rGF = GraphicFilter::GetGraphicFilter();
@@ -487,7 +496,7 @@ void EmbeddedObjectRef::GetReplacement( bool bUpdate )
         // failed. Go back to the old graphic instead of having no graphic at
         // all.
         mpImpl->pGraphic.reset(new Graphic(aOldGraphic));
-        SAL_WARN("svtools.misc", "EmbeddedObjectRef::GetReplacement: update failed");
+        SAL_WARN("svtools.misc", "EmbeddedObjectRef::GetReplacement: failed to update graphic");
     }
 }
 
diff --git a/sw/CppunitTest_sw_core_view.mk b/sw/CppunitTest_sw_core_view.mk
new file mode 100644
index 000000000000..c1846e3c91ee
--- /dev/null
+++ b/sw/CppunitTest_sw_core_view.mk
@@ -0,0 +1,70 @@
+# -*- 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,sw_core_view))
+
+$(eval $(call gb_CppunitTest_use_common_precompiled_header,sw_core_view))
+
+$(eval $(call gb_CppunitTest_add_exception_objects,sw_core_view, \
+    sw/qa/core/view/view \
+))
+
+$(eval $(call gb_CppunitTest_use_libraries,sw_core_view, \
+    cppu \
+    cppuhelper \
+    sal \
+    svl \
+    svt \
+    sw \
+    swqahelper \
+    test \
+    unotest \
+    utl \
+    vcl \
+))
+
+$(eval $(call gb_CppunitTest_use_externals,sw_core_view,\
+    boost_headers \
+    libxml2 \
+))
+
+$(eval $(call gb_CppunitTest_set_include,sw_core_view,\
+    -I$(SRCDIR)/sw/inc \
+    -I$(SRCDIR)/sw/source/core/inc \
+    -I$(SRCDIR)/sw/source/uibase/inc \
+    -I$(SRCDIR)/sw/qa/inc \
+    $$(INCLUDE) \
+))
+
+$(eval $(call gb_CppunitTest_use_api,sw_core_view,\
+    udkapi \
+    offapi \
+    oovbaapi \
+))
+
+$(eval $(call gb_CppunitTest_use_ure,sw_core_view))
+$(eval $(call gb_CppunitTest_use_vcl,sw_core_view))
+
+$(eval $(call gb_CppunitTest_use_rdb,sw_core_view,services))
+
+$(eval $(call gb_CppunitTest_use_custom_headers,sw_core_view,\
+    officecfg/registry \
+))
+
+$(eval $(call gb_CppunitTest_use_configuration,sw_core_view))
+
+$(eval $(call gb_CppunitTest_use_uiconfigs,sw_core_view, \
+    modules/swriter \
+))
+
+$(eval $(call gb_CppunitTest_use_more_fonts,sw_core_view))
+
+# vim: set noet sw=4 ts=4:
diff --git a/sw/Module_sw.mk b/sw/Module_sw.mk
index da25613e7ace..baaafcbc5b76 100644
--- a/sw/Module_sw.mk
+++ b/sw/Module_sw.mk
@@ -140,6 +140,7 @@ $(eval $(call gb_Module_add_slowcheck_targets,sw,\
     CppunitTest_sw_core_draw \
     CppunitTest_sw_core_edit \
     CppunitTest_sw_uibase_fldui \
+    CppunitTest_sw_core_view \
 ))
 
 ifneq ($(DISABLE_GUI),TRUE)
diff --git a/sw/qa/core/view/data/update-ole-object-previews.odt b/sw/qa/core/view/data/update-ole-object-previews.odt
new file mode 100644
index 000000000000..e6004afcb8ad
--- /dev/null
+++ b/sw/qa/core/view/data/update-ole-object-previews.odt
diff --git a/sw/qa/core/view/view.cxx b/sw/qa/core/view/view.cxx
new file mode 100644
index 000000000000..3ff399a0f8ea
--- /dev/null
+++ b/sw/qa/core/view/view.cxx
@@ -0,0 +1,70 @@
+/* -*- 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 <swmodeltestbase.hxx>
+
+#include <svtools/embedhlp.hxx>
+#include <vcl/graph.hxx>
+
+#include <doc.hxx>
+#include <docsh.hxx>
+#include <fmtcntnt.hxx>
+#include <frameformats.hxx>
+#include <frmfmt.hxx>
+#include <ndarr.hxx>
+#include <ndindex.hxx>
+#include <ndole.hxx>
+#include <node.hxx>
+#include <nodeoffset.hxx>
+#include <wrtsh.hxx>
+
+constexpr OUStringLiteral DATA_DIRECTORY = u"/sw/qa/core/view/data/";
+
+namespace
+{
+/// Covers sw/source/core/view/ fixes.
+class Test : public SwModelTestBase
+{
+};
+}
+
+CPPUNIT_TEST_FIXTURE(Test, testUpdateOleObjectPreviews)
+{
+    // Given a document with two embedded objects, both with broken native data:
+    SwDoc* pDoc = createSwDoc(DATA_DIRECTORY, "update-ole-object-previews.odt");
+    SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell();
+
+    // When updating the previews of those embedded objects (right after document load, before
+    // painting the OLE objects):
+    pWrtShell->UpdateOleObjectPreviews();
+
+    // Then make sure that the working preview of those objects are not lost:
+    const SwFrameFormats* pFormats = pDoc->GetSpzFrameFormats();
+    CPPUNIT_ASSERT(pFormats);
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), pFormats->size());
+    for (size_t i = 0; i < pFormats->size(); ++i)
+    {
+        SwFrameFormat* pFormat = (*pFormats)[i];
+
+        const SwNodeIndex* pNodeIndex = pFormat->GetContent().GetContentIdx();
+        CPPUNIT_ASSERT(pNodeIndex);
+        SwNode* pNode = pDoc->GetNodes()[pNodeIndex->GetIndex() + 1];
+        SwOLENode* pOleNode = pNode->GetOLENode();
+        CPPUNIT_ASSERT(pOleNode);
+        SwOLEObj& rOleObj = pOleNode->GetOLEObj();
+        svt::EmbeddedObjectRef& rObject = rOleObj.GetObject();
+        // 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_PLUGIN_IMPLEMENT();
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */