[Libreoffice-commits] core.git: Branch 'libreoffice-4-4' - 4 commits - chart2/source editeng/qa editeng/source sd/qa xmloff/source
Michael Stahl
mstahl at redhat.com
Mon Dec 1 14:33:54 PST 2014
chart2/source/controller/main/ChartController.cxx | 16 ++++
chart2/source/controller/main/ControllerCommandDispatch.cxx | 9 ++
editeng/qa/unit/core-test.cxx | 39 ++++++++++++
editeng/source/editeng/editdoc.cxx | 23 ++++---
editeng/source/editeng/editobj.cxx | 5 +
sd/qa/unit/data/fdo84043.odp |binary
sd/qa/unit/export-tests.cxx | 16 ++++
xmloff/source/style/impastpl.cxx | 14 ++++
8 files changed, 111 insertions(+), 11 deletions(-)
New commits:
commit 91892803ebb5c99a972fd5ef37b77634a9138062
Author: Michael Stahl <mstahl at redhat.com>
Date: Mon Dec 1 19:25:13 2014 +0100
fdo#85496: add some asserts to detect this sort of problem
Change-Id: Iff787c8d2a71bc3082192cc98e3d916badee65dd
(cherry picked from commit 7a242b463132d67a4a2d6e69319e0da367145cc0)
diff --git a/editeng/source/editeng/editobj.cxx b/editeng/source/editeng/editobj.cxx
index b100bd4..332e71e 100644
--- a/editeng/source/editeng/editobj.cxx
+++ b/editeng/source/editeng/editobj.cxx
@@ -977,6 +977,11 @@ void EditTextObjectImpl::GetAllSections( std::vector<editeng::Section>& rAttrs )
for (; itCurAttr != aAttrs.end() && itCurAttr->mnParagraph == nPara && itCurAttr->mnEnd <= nEnd; ++itCurAttr)
{
editeng::Section& rSecAttr = *itCurAttr;
+ // serious bug: will cause duplicate attributes to be exported
+ assert(rSecAttr.maAttributes.end() == std::find_if(
+ rSecAttr.maAttributes.begin(), rSecAttr.maAttributes.end(),
+ [&pItem](SfxPoolItem const*const pIt)
+ { return pIt->Which() == pItem->Which(); }));
rSecAttr.maAttributes.push_back(pItem);
}
}
diff --git a/xmloff/source/style/impastpl.cxx b/xmloff/source/style/impastpl.cxx
index 1888407..365fd5e 100644
--- a/xmloff/source/style/impastpl.cxx
+++ b/xmloff/source/style/impastpl.cxx
@@ -247,6 +247,20 @@ XMLAutoStylePoolProperties::XMLAutoStylePoolProperties( XMLAutoStyleFamily& rFam
}
while (rFamilyData.maNameSet.find(msName) != rFamilyData.maNameSet.end());
}
+
+#if OSL_DEBUG_LEVEL > 0
+ std::set<sal_Int32> DebugProperties;
+ for (size_t i = 0; i < maProperties.size(); ++i)
+ {
+ sal_Int32 const property(maProperties[i].mnIndex);
+ // serious bug: will cause duplicate attributes to be exported
+ assert(DebugProperties.find(property) == DebugProperties.end());
+ if (-1 != property)
+ {
+ DebugProperties.insert(property);
+ }
+ }
+#endif
}
bool operator<( const XMLAutoStyleFamily& r1, const XMLAutoStyleFamily& r2)
commit 8fccff42616ac434c3d641a76c37f3a135cec97e
Author: Michael Stahl <mstahl at redhat.com>
Date: Mon Dec 1 19:21:49 2014 +0100
fdo#85496: editeng: do not add multiple 0-length attributes...
... at the same position.
Since commit 0d57434180db6c8eda8c5b9b704f8a1c18b371df these will be
exported by the ODF filter as duplicate attributes.
Change-Id: I8befe55f61c59ab968409fa03359540c300f9198
(cherry picked from commit 846b56b6b99e334dfa44f1a24640aa3158509854)
diff --git a/editeng/qa/unit/core-test.cxx b/editeng/qa/unit/core-test.cxx
index 4090db4..ae4a946 100644
--- a/editeng/qa/unit/core-test.cxx
+++ b/editeng/qa/unit/core-test.cxx
@@ -573,6 +573,45 @@ void Test::testSectionAttributes()
CPPUNIT_ASSERT_EQUAL(0, (int)pSecAttr->mnEnd);
CPPUNIT_ASSERT_MESSAGE("Attribute array should be empty.", pSecAttr->maAttributes.empty());
}
+
+
+ {
+ aEngine.Clear();
+ aEngine.SetText("one\ntwo");
+ CPPUNIT_ASSERT_EQUAL(2, aEngine.GetParagraphCount());
+
+ // embolden 2nd paragraph
+ pSet.reset(new SfxItemSet(aEngine.GetEmptyItemSet()));
+ pSet->Put(aBold);
+ aEngine.QuickSetAttribs(*pSet, ESelection(1,0,1,3));
+ // disboldify 1st paragraph
+ SvxWeightItem aNotSoBold(WEIGHT_NORMAL, EE_CHAR_WEIGHT);
+ pSet->Put(aNotSoBold);
+ aEngine.QuickSetAttribs(*pSet, ESelection(0,0,0,3));
+
+ // now delete & join the paragraphs - this is fdo#85496 scenario
+ aEngine.QuickDelete(ESelection(0,0,1,3));
+ CPPUNIT_ASSERT_EQUAL(1, aEngine.GetParagraphCount());
+
+ boost::scoped_ptr<EditTextObject> pEditText(aEngine.CreateTextObject());
+ CPPUNIT_ASSERT_MESSAGE("Failed to create text object.", pEditText.get());
+ std::vector<editeng::Section> aAttrs;
+ pEditText->GetAllSections(aAttrs);
+
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), aAttrs.size());
+
+ const editeng::Section* pSecAttr = &aAttrs[0];
+ CPPUNIT_ASSERT_EQUAL(0, (int)pSecAttr->mnParagraph);
+ CPPUNIT_ASSERT_EQUAL(0, (int)pSecAttr->mnStart);
+ CPPUNIT_ASSERT_EQUAL(0, (int)pSecAttr->mnEnd);
+ std::set<sal_uInt16> whiches;
+ for (size_t i = 0; i < pSecAttr->maAttributes.size(); ++i)
+ {
+ sal_uInt16 const nWhich(pSecAttr->maAttributes[i]->Which());
+ CPPUNIT_ASSERT_MESSAGE("duplicate item in text portion attributes",
+ whiches.insert(nWhich).second);
+ }
+ }
}
CPPUNIT_TEST_SUITE_REGISTRATION(Test);
diff --git a/editeng/source/editeng/editdoc.cxx b/editeng/source/editeng/editdoc.cxx
index e293425..54cdb90 100644
--- a/editeng/source/editeng/editdoc.cxx
+++ b/editeng/source/editeng/editdoc.cxx
@@ -1579,14 +1579,23 @@ void ContentNode::AppendAttribs( ContentNode* pNextNode )
{
if ( pTmpAttrib->GetEnd() == nNewStart )
{
- if ( ( pTmpAttrib->Which() == pAttrib->Which() ) &&
- ( *(pTmpAttrib->GetItem()) == *(pAttrib->GetItem() ) ) )
+ if (pTmpAttrib->Which() == pAttrib->Which())
{
- pTmpAttrib->GetEnd() =
- pTmpAttrib->GetEnd() + pAttrib->GetLen();
- rNextAttribs.erase(rNextAttribs.begin()+nAttr);
- // Unsubscribe from the pool?!
- bMelted = true;
+ // prevent adding 2 0-length attributes at same position
+ if ((*(pTmpAttrib->GetItem()) == *(pAttrib->GetItem()))
+ || (0 == pAttrib->GetLen()))
+ {
+ pTmpAttrib->GetEnd() =
+ pTmpAttrib->GetEnd() + pAttrib->GetLen();
+ rNextAttribs.erase(rNextAttribs.begin()+nAttr);
+ // Unsubscribe from the pool?!
+ bMelted = true;
+ }
+ else if (0 == pTmpAttrib->GetLen())
+ {
+ aCharAttribList.Remove(nTmpAttr);
+ --nTmpAttr; // to cancel later increment...
+ }
}
}
++nTmpAttr;
commit 8141936ccdb0aa71d7489ce232d954da89e5174b
Author: Michael Stahl <mstahl at redhat.com>
Date: Fri Nov 28 18:39:52 2014 +0100
chart2: ChartController: fix CommandDispatchContainer access locking
crashes on concurrent setModel() and getDispatchForURL() in
JunitTest_chart2_unoapi:
Thread 13:
7 in (anonymous namespace)::Frame::isActionLocked (this=0x2b66c0e4a090) at /home/tinderbox/master/framework/source/services/frame.cxx:2596
8 in (anonymous namespace)::Frame::close (this=0x2b66c0e4a090, bDeliverOwnership=0 '\000') at /home/tinderbox/master/framework/source/services/frame.cxx:1772
9 in chart::ChartController::notifyClosing (this=0x2b668ae41058, rSource=...) at /home/tinderbox/master/chart2/source/controller/main/ChartController.cxx:872
10 in apphelper::CloseableLifeTimeManager::impl_doClose (this=0x2b669a0b1d08) at /home/tinderbox/master/chart2/source/tools/LifeTime.cxx:360
11 in apphelper::CloseableLifeTimeManager::g_close_endTryClose_doClose (this=0x2b669a0b1d08) at /home/tinderbox/master/chart2/source/tools/LifeTime.cxx:311
12 in chart::ChartModel::close (this=0x2b669a0b1c28, bDeliverOwnership=1 '\001') at /home/tinderbox/master/chart2/source/model/main/ChartModel.cxx:659
Thread 1:
4 in std::__debug::map<rtl::OUString, com::sun::star::uno::Reference<com::sun::star::frame::XDispatch>, std::less<rtl::OUString>, std::allocator<std::pair<rtl::OUString const, com::sun::star::uno::Reference<com::sun::star::frame::XDispatch> > > >::find (this=0x2b668ae41248, __x=...) at /usr/include/c++/4.8.3/debug/map.h:382
No locals.
5 in chart::CommandDispatchContainer::getDispatchForURL (this=0x2b668ae41248, rURL=...) at /home/tinderbox/master/chart2/source/controller/main/CommandDispatchContainer.cxx:80
6 in chart::ChartController::queryDispatch (this=0x2b668ae41058, rURL=..., rTargetFrameName=...) at /home/tinderbox/master/chart2/source/controller/main/ChartController.cxx:1003
No locals.
7 in framework::DispatchProvider::implts_queryFrameDispatch (this=0x2b66a3fd21e8, xFrame=..., aURL=..., sTargetFrameName=..., nSearchFlags=0) at /home/tinderbox/master/framework/source/dispatch/dispatchprovider.cxx:374
8 in framework::DispatchProvider::queryDispatch (this=0x2b66a3fd21e8, aURL=..., sTargetFrameName=..., nSearchFlags=0) at /home/tinderbox/master/framework/source/dispatch/dispatchprovider.cxx:111
9 in framework::InterceptionHelper::queryDispatch (this=0x2b668a61bc08, aURL=..., sTargetFrameName=..., nSearchFlags=0) at /home/tinderbox/master/framework/source/dispatch/interceptionhelper.cxx:78
10 in (anonymous namespace)::Frame::queryDispatch (this=0x2b66c0e4a090, aURL=..., sTargetFrameName=..., nSearchFlags=0) at /home/tinderbox/master/framework/source/services/frame.cxx:2227
11 in svt::ToolboxController::bindListener (this=0x2b66c0f72740) at /home/tinderbox/master/svtools/source/uno/toolboxcontroller.cxx:529
12 in svt::ToolboxController::update (this=0x2b66c0f72740) at /home/tinderbox/master/svtools/source/uno/toolboxcontroller.cxx:232
13 in framework::ToolBarManager::UpdateControllers (this=0x2b669a0a1728) at /home/tinderbox/master/framework/source/uielement/toolbarmanager.cxx:440
14 in framework::ToolBarManager::AsyncUpdateControllersHdl (this=0x2b669a0a1728) at /home/tinderbox/master/framework/source/uielement/toolbarmanager.cxx:2110
15 in framework::ToolBarManager::LinkStubAsyncUpdateControllersHdl (pThis=0x2b669a0a1728, pCaller=0x2b669a0a1890) at /home/tinderbox/master/framework/source/uielement/toolbarmanager.cxx:2097
16 in Link::Call (this=0x2b669a0a18b0, pCaller=0x2b669a0a1890) at /home/tinderbox/master/include/tools/link.hxx:139
17 in Timer::Timeout (this=0x2b669a0a1890) at /home/tinderbox/master/vcl/source/app/timer.cxx:276
Change-Id: I17ef63db8f7c288460e00031e8e8a5c3e4d086b3
(cherry picked from commit 426ef2847cbdc74c068531915efb852a727cd3ee)
diff --git a/chart2/source/controller/main/ChartController.cxx b/chart2/source/controller/main/ChartController.cxx
index a9ff0c6..7accfe9 100644
--- a/chart2/source/controller/main/ChartController.cxx
+++ b/chart2/source/controller/main/ChartController.cxx
@@ -491,10 +491,10 @@ sal_Bool SAL_CALL ChartController::attachModel( const uno::Reference< frame::XMo
//is called to attach the controller to a new model.
//return true if attach was successfully, false otherwise (e.g. if you do not work with a model)
- SolarMutexClearableGuard aClearableGuard;
+ SolarMutexResettableGuard aGuard;
if( impl_isDisposedOrSuspended() ) //@todo? allow attaching a new model while suspended?
return sal_False; //behave passive if already disposed or suspended
- aClearableGuard.clear();
+ aGuard.clear();
TheModelRef aNewModelRef( new TheModel( xModel), m_aModelMutex);
TheModelRef aOldModelRef(m_aModel,m_aModelMutex);
@@ -519,6 +519,7 @@ sal_Bool SAL_CALL ChartController::attachModel( const uno::Reference< frame::XMo
//--handle relations to the new model
aNewModelRef->addListener( this );
+ aGuard.reset(); // lock for m_aDispatchContainer access
// set new model at dispatchers
m_aDispatchContainer.setModel( aNewModelRef->getModel());
ControllerCommandDispatch * pDispatch = new ControllerCommandDispatch( m_xCC, this, &m_aDispatchContainer );
@@ -542,6 +543,7 @@ sal_Bool SAL_CALL ChartController::attachModel( const uno::Reference< frame::XMo
pShapeController->initialize();
m_aDispatchContainer.setShapeController( pShapeController );
}
+ aGuard.clear();
#ifdef TEST_ENABLE_MODIFY_LISTENER
uno::Reference< util::XModifyBroadcaster > xMBroadcaster( aNewModelRef->getModel(),uno::UNO_QUERY );
@@ -565,7 +567,7 @@ sal_Bool SAL_CALL ChartController::attachModel( const uno::Reference< frame::XMo
//the frameloader is responsible to call xModel->connectController
{
- SolarMutexGuard aGuard;
+ SolarMutexGuard aGuard2;
if( m_pChartWindow )
m_pChartWindow->Invalidate();
}
@@ -784,6 +786,7 @@ void SAL_CALL ChartController::dispose()
//// @todo integrate specialized implementation
//e.g. release further resources and references
+ SolarMutexGuard g;
m_aDispatchContainer.DisposeAndClear();
}
catch( const uno::Exception & ex )
@@ -894,7 +897,10 @@ bool ChartController::impl_releaseThisModel(
}
}
if( bReleaseModel )
+ {
+ SolarMutexGuard g;
m_aDispatchContainer.setModel( 0 );
+ }
return bReleaseModel;
}
@@ -997,6 +1003,8 @@ uno::Reference<frame::XDispatch> SAL_CALL
sal_Int32 /* nSearchFlags */)
throw(uno::RuntimeException, std::exception)
{
+ SolarMutexGuard aGuard;
+
if ( !m_aLifeTimeManager.impl_isDisposed() && getModel().is() )
{
if( !rTargetFrameName.isEmpty() && rTargetFrameName == "_self" )
@@ -1010,6 +1018,8 @@ uno::Sequence<uno::Reference<frame::XDispatch > >
const uno::Sequence<frame::DispatchDescriptor>& xDescripts )
throw(uno::RuntimeException, std::exception)
{
+ SolarMutexGuard g;
+
if ( !m_aLifeTimeManager.impl_isDisposed() )
{
return m_aDispatchContainer.getDispatchesForURLs( xDescripts );
diff --git a/chart2/source/controller/main/ControllerCommandDispatch.cxx b/chart2/source/controller/main/ControllerCommandDispatch.cxx
index 7f4f4c1..e28fbe8 100644
--- a/chart2/source/controller/main/ControllerCommandDispatch.cxx
+++ b/chart2/source/controller/main/ControllerCommandDispatch.cxx
@@ -32,6 +32,8 @@
#include "StatisticsHelper.hxx"
#include "ShapeController.hxx"
+#include <vcl/svapp.hxx>
+
#include <com/sun/star/util/XModifyBroadcaster.hpp>
#include <com/sun/star/frame/XStorable.hpp>
#include <com/sun/star/chart2/XChartDocument.hpp>
@@ -698,7 +700,12 @@ bool ControllerCommandDispatch::commandAvailable( const OUString & rCommand )
bool ControllerCommandDispatch::isShapeControllerCommandAvailable( const OUString& rCommand )
{
- ShapeController* pShapeController = ( m_pDispatchContainer ? m_pDispatchContainer->getShapeController() : NULL );
+ ShapeController* pShapeController(0);
+ {
+ SolarMutexGuard g;
+ if (m_pDispatchContainer)
+ pShapeController = m_pDispatchContainer->getShapeController();
+ }
if ( pShapeController )
{
FeatureState aState( pShapeController->getState( rCommand ) );
commit 17d4a05bb8757da6384d0df8385deb86b8080856
Author: Michael Stahl <mstahl at redhat.com>
Date: Fri Nov 28 17:37:01 2014 +0100
fdo#84043: add a test for the bug
Change-Id: Ic9ad86bfc2d4d6b02afe99a34ded27fe94f54fab
(cherry picked from commit 12ab10e5824bb5efff27123aecfdefa1a16d5223)
diff --git a/sd/qa/unit/data/fdo84043.odp b/sd/qa/unit/data/fdo84043.odp
new file mode 100644
index 0000000..eed9e79
Binary files /dev/null and b/sd/qa/unit/data/fdo84043.odp differ
diff --git a/sd/qa/unit/export-tests.cxx b/sd/qa/unit/export-tests.cxx
index dfa2edc..189dbf2 100644
--- a/sd/qa/unit/export-tests.cxx
+++ b/sd/qa/unit/export-tests.cxx
@@ -71,6 +71,7 @@ public:
void testN828390_5();
void testMediaEmbedding();
void testFdo71961();
+ void testFdo84043();
void testN828390();
void testBnc880763();
void testBnc862510_5();
@@ -87,6 +88,7 @@ public:
CPPUNIT_TEST(testN828390_5);
CPPUNIT_TEST(testMediaEmbedding);
CPPUNIT_TEST(testFdo71961);
+ CPPUNIT_TEST(testFdo84043);
CPPUNIT_TEST(testN828390);
CPPUNIT_TEST(testBnc880763);
CPPUNIT_TEST(testBnc862510_5);
@@ -325,6 +327,20 @@ void SdExportTest::testMediaEmbedding()
xDocShRef->DoClose();
}
+void SdExportTest::testFdo84043()
+{
+ ::sd::DrawDocShellRef xDocShRef = loadURL(getURLFromSrc("/sd/qa/unit/data/fdo84043.odp"), ODP);
+ xDocShRef = saveAndReload( xDocShRef, ODP );
+
+ // the bug was duplicate attributes, causing crash in a build with asserts
+ SdDrawDocument const* pDoc = xDocShRef->GetDoc();
+ CPPUNIT_ASSERT_MESSAGE("no document", pDoc != nullptr);
+ SdrPage const* pPage = pDoc->GetPage(1);
+ CPPUNIT_ASSERT_MESSAGE("no page", pPage != nullptr);
+ SdrObject const* pShape = pPage->GetObj(1);
+ CPPUNIT_ASSERT_MESSAGE("no shape", pShape != nullptr);
+}
+
void SdExportTest::testFdo71961()
{
::sd::DrawDocShellRef xDocShRef = loadURL(getURLFromSrc("/sd/qa/unit/data/fdo71961.odp"), ODP);
More information about the Libreoffice-commits
mailing list