fdo#80924: writerfilter: RTF import: fix shapeType PictureFrame import

For shapeType PictureFrame, a default CustomShape was created and then
RTFDocumentImpl::resolvePict() crashes because it actually operates on a
previous shape, because in this special case RTFSdrImport::m_xShape
is never actually set to the new shape, so contains the previous one.

Also the new shape needs to be added to the draw-page, at least
otherwise the assertion of the supported service fails because
some SvxShape::mpObj weak-reference is dead?

This essentially reverts commit 3cab1adf19d553663685e8198f0ec3f258a37c36
(except for the added testcase, which was slightly different because
it did not have a "pib" property).

(regression from ba9b63d8101197d3fd8612193b1ca188271dfc1a)

(cherry picked from commit 083b2b1471fff9e75f4b27a8769f95b8a0ebf5a1)

Conflicts:
	writerfilter/source/rtftok/rtfdocumentimpl.cxx

Change-Id: I6539c4286850dff2d8564006487cc765f1d117be
Reviewed-on: https://gerrit.libreoffice.org/10351
Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>
Tested-by: Miklos Vajna <vmiklos@collabora.co.uk>

3 files changed, 22 insertions, 12 deletions

diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.cxx b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
index befbb4204a89..c53174e2487e 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.cxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
@@ -705,7 +705,8 @@ void RTFDocumentImpl::resolve(Stream& rMapper)
     }
 }
 
-int RTFDocumentImpl::resolvePict(bool bInline)
+int RTFDocumentImpl::resolvePict(bool const bInline,
+        uno::Reference<drawing::XShape> const& i_xShape)
 {
     SvMemoryStream aStream;
     SvStream* pStream = 0;
@@ -778,16 +779,13 @@ int RTFDocumentImpl::resolvePict(bool bInline)
     }
 
     // Wrap it in an XShape.
-    uno::Reference<drawing::XShape> xShape;
-    xShape = m_pSdrImport->getCurrentShape();
+    uno::Reference<drawing::XShape> xShape(i_xShape);
     if (xShape.is())
     {
         uno::Reference<lang::XServiceInfo> xSI(xShape, uno::UNO_QUERY_THROW);
-        if(!xSI->supportsService("com.sun.star.drawing.GraphicObjectShape"))
-            xShape.clear();
+        assert(xSI->supportsService("com.sun.star.drawing.GraphicObjectShape"));
     }
-
-    if (!xShape.is())
+    else
     {
         if (m_xModelFactory.is())
             xShape.set(m_xModelFactory->createInstance(
@@ -4944,7 +4942,7 @@ int RTFDocumentImpl::popState()
         Mapper().props(lcl_getBookmarkProperties(m_aBookmarks[m_aStates.top().aDestinationText.makeStringAndClear()]));
         break;
     case DESTINATION_PICT:
-        resolvePict(true);
+        resolvePict(true, m_pSdrImport->getCurrentShape());
         break;
     case DESTINATION_FORMFIELDNAME:
     {
diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.hxx b/writerfilter/source/rtftok/rtfdocumentimpl.hxx
index b86c54f950ae..1e8a46e34bab 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.hxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.hxx
@@ -356,7 +356,8 @@ public:
     bool isInBackground();
     void setDestinationText(OUString& rString);
     /// Resolve a picture: If not inline, then anchored.
-    int resolvePict(bool bInline);
+    int resolvePict(bool bInline,
+            css::uno::Reference<css::drawing::XShape> const& xShape);
 
     /// If this is the first run of the document, starts the initial paragraph.
     void checkFirstRun();
diff --git a/writerfilter/source/rtftok/rtfsdrimport.cxx b/writerfilter/source/rtftok/rtfsdrimport.cxx
index a47731d9c7a0..5edf4254c5d7 100644
--- a/writerfilter/source/rtftok/rtfsdrimport.cxx
+++ b/writerfilter/source/rtftok/rtfsdrimport.cxx
@@ -269,6 +269,9 @@ void RTFSdrImport::resolve(RTFShape& rShape, bool bClose,
             nType = i->second.toInt32();
             switch (nType)
             {
+            case ESCHER_ShpInst_PictureFrame:
+                createShape("com.sun.star.drawing.GraphicObjectShape", xShape, xPropertySet);
+                break;
             case ESCHER_ShpInst_Line:
                 createShape("com.sun.star.drawing.LineShape", xShape, xPropertySet);
                 break;
@@ -653,15 +656,23 @@ void RTFSdrImport::resolve(RTFShape& rShape, bool bClose,
             xPropertySet->setPropertyValue("TextWritingMode", uno::makeAny(eWritingMode));
     }
 
+    if (m_aParents.size() && m_aParents.top().is() && !m_bTextFrame)
+        m_aParents.top()->add(xShape);
+
     if (nType == ESCHER_ShpInst_PictureFrame) // picture frame
     {
+        assert(!m_bTextFrame);
         if (bPib)
-            m_rImport.resolvePict(false);
+        {
+            m_rImport.resolvePict(false, xShape);
+        }
+        else // ??? not sure if the early return should be removed on else?
+        {
+            m_xShape = xShape; // store it for later resolvePict call
+        }
         return;
     }
 
-    if (m_aParents.size() && m_aParents.top().is() && !m_bTextFrame)
-        m_aParents.top()->add(xShape);
     if (bCustom && xShape.is())
     {
         uno::Reference<drawing::XEnhancedCustomShapeDefaulter> xDefaulter(xShape, uno::UNO_QUERY);