[Libreoffice-commits] core.git: Branch 'distro/collabora/cp-6.4' - include/tools sd/qa
Tomaž Vajngerl (via logerrit)
logerrit at kemper.freedesktop.org
Thu Jun 11 07:21:52 UTC 2020
include/tools/weakbase.h | 5 ++-
include/tools/weakbase.hxx | 9 ++++-
sd/qa/unit/tiledrendering/LOKitSearchTest.cxx | 39 ++++++++++++++++++++++++
sd/qa/unit/tiledrendering/data/OnePDFObject.odg |binary
4 files changed, 51 insertions(+), 2 deletions(-)
New commits:
commit 35677f1d11ff26fda86ddc9d6d73c3ee2b52fe2b
Author: Tomaž Vajngerl <tomaz.vajngerl at collabora.co.uk>
AuthorDate: Wed Jun 10 11:08:36 2020 +0200
Commit: Andras Timar <andras.timar at collabora.com>
CommitDate: Thu Jun 11 09:21:16 2020 +0200
fix reseting WeakReference when moving a ref to another ref.
In ViewIteratorImpl::Reverse we std::move a reference, to a local
variable, however the reference was not reset correctly, which
caused a crash.
The issue is that a mpWeakConnection in WeakReference always
needs to be non-null as shown in reset(reference_type* pReference)
method. In the move constructor of WeakReference, we just std::move
the reference like:
mpWeakConnection = std::move(rWeakRef.mpWeakConnection);
This means rWeakRef.mpWeakConnection will be reset to null, which
is not what is expected, so what is missing is to instantiate
rWeakRef.mpWeakConnection by calling reset(nullptr) or using the
added reset() method.
This also adds a test for LOKit PDF search as this is the case
where the crash has been reproduced. It happens because of a
call to:
... std::move(maPosition.mxObject);
in the method ViewIteratorImpl::Reverse()
Change-Id: I43692554ba4f6231d00091269b7c902ab5cbc11f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/95995
Tested-by: Jenkins
Reviewed-by: Tomaž Vajngerl <quikee at gmail.com>
(cherry picked from commit 8ca89dc44aa5905d8d189be765ad2a1a613d31f4)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/96070
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Andras Timar <andras.timar at collabora.com>
diff --git a/include/tools/weakbase.h b/include/tools/weakbase.h
index 458381e47fff..fbd49d2ecf90 100644
--- a/include/tools/weakbase.h
+++ b/include/tools/weakbase.h
@@ -82,7 +82,7 @@ public:
/** constructs a reference from another reference */
inline WeakReference( const WeakReference< reference_type >& rWeakRef );
- /** constructs a reference from another reference */
+ /** move a reference from another reference */
inline WeakReference( WeakReference< reference_type >&& rWeakRef );
/** returns true if the reference object is not null and still alive */
@@ -94,6 +94,9 @@ public:
/** sets this reference to the given object or null */
inline void reset( reference_type* pReference );
+ /** resets this reference to null */
+ inline void reset();
+
/** returns the pointer to the reference object or null */
inline reference_type * operator->() const;
diff --git a/include/tools/weakbase.hxx b/include/tools/weakbase.hxx
index fa16edd2f147..8bc4611abb0c 100644
--- a/include/tools/weakbase.hxx
+++ b/include/tools/weakbase.hxx
@@ -49,6 +49,7 @@ template< class reference_type >
inline WeakReference< reference_type >::WeakReference( WeakReference< reference_type >&& rWeakRef )
{
mpWeakConnection = std::move(rWeakRef.mpWeakConnection);
+ rWeakRef.reset();
}
template< class reference_type >
@@ -73,7 +74,13 @@ inline void WeakReference< reference_type >::reset( reference_type* pReference )
if( pReference )
mpWeakConnection = pReference->getWeakConnection();
else
- mpWeakConnection = new WeakConnection;
+ reset();
+}
+
+template< class reference_type >
+inline void WeakReference< reference_type >::reset()
+{
+ mpWeakConnection = new WeakConnection;
}
template< class reference_type >
diff --git a/sd/qa/unit/tiledrendering/LOKitSearchTest.cxx b/sd/qa/unit/tiledrendering/LOKitSearchTest.cxx
index 91b5ecbc8473..ac2695441575 100644
--- a/sd/qa/unit/tiledrendering/LOKitSearchTest.cxx
+++ b/sd/qa/unit/tiledrendering/LOKitSearchTest.cxx
@@ -57,6 +57,7 @@ public:
void testDontSearchInMasterPages();
void testSearchInPDFNonExisting();
void testSearchInPDF();
+ void testSearchInPDFOnePDFObject();
void testSearchInPDFInMultiplePages();
void testSearchInPDFInMultiplePagesBackwards();
void testSearchIn2MixedObjects();
@@ -71,6 +72,7 @@ public:
CPPUNIT_TEST(testDontSearchInMasterPages);
CPPUNIT_TEST(testSearchInPDFNonExisting);
CPPUNIT_TEST(testSearchInPDF);
+ CPPUNIT_TEST(testSearchInPDFOnePDFObject);
CPPUNIT_TEST(testSearchInPDFInMultiplePages);
CPPUNIT_TEST(testSearchInPDFInMultiplePagesBackwards);
CPPUNIT_TEST(testSearchIn2MixedObjects);
@@ -342,6 +344,43 @@ void LOKitSearchTest::testSearchInPDF()
#endif
}
+void LOKitSearchTest::testSearchInPDFOnePDFObject()
+{
+#if HAVE_FEATURE_PDFIUM
+ SdXImpressDocument* pXImpressDocument = createDoc("OnePDFObject.odg");
+ sd::ViewShell* pViewShell = pXImpressDocument->GetDocShell()->GetViewShell();
+ CPPUNIT_ASSERT(pViewShell);
+ mpCallbackRecorder->registerCallbacksFor(pViewShell->GetViewShellBase());
+
+ SdPage* pPage = pViewShell->GetActualPage();
+ CPPUNIT_ASSERT(pPage);
+
+ SdrObject* pObject = pPage->GetObj(0);
+ CPPUNIT_ASSERT(pObject);
+
+ SdrGrafObj* pGraphicObject = dynamic_cast<SdrGrafObj*>(pObject);
+ CPPUNIT_ASSERT(pGraphicObject);
+
+ Graphic aGraphic = pGraphicObject->GetGraphic();
+ auto const& pVectorGraphicData = aGraphic.getVectorGraphicData();
+ CPPUNIT_ASSERT(pVectorGraphicData);
+ CPPUNIT_ASSERT_EQUAL(VectorGraphicDataType::Pdf,
+ pVectorGraphicData->getVectorGraphicDataType());
+
+ // Search down
+ lcl_search("ABC", false, false);
+
+ CPPUNIT_ASSERT_EQUAL(true, mpCallbackRecorder->m_bFound);
+ CPPUNIT_ASSERT_EQUAL(1, mpCallbackRecorder->m_nSearchResultCount);
+
+ // Search up
+ lcl_search("ABC", false, true); // This caused a crash
+
+ CPPUNIT_ASSERT_EQUAL(true, mpCallbackRecorder->m_bFound);
+ CPPUNIT_ASSERT_EQUAL(2, mpCallbackRecorder->m_nSearchResultCount);
+#endif
+}
+
void LOKitSearchTest::testSearchInPDFInMultiplePages()
{
#if HAVE_FEATURE_PDFIUM
diff --git a/sd/qa/unit/tiledrendering/data/OnePDFObject.odg b/sd/qa/unit/tiledrendering/data/OnePDFObject.odg
new file mode 100644
index 000000000000..225741c7b50b
Binary files /dev/null and b/sd/qa/unit/tiledrendering/data/OnePDFObject.odg differ
More information about the Libreoffice-commits
mailing list