From d5d32eb755c8a53292acbf0648fb82baf6729d8a Mon Sep 17 00:00:00 2001 From: Jan Holesovsky Date: Fri, 10 Feb 2012 14:12:17 +0100 Subject: fdo#40261: Fix crash in XML Form Document. The data structure holding the UI elements in the browser listbox was a terrible mess - it held the items in an unordered_map, but then accessed them via a vector containing iterators to this unordered_map. Fixed the problem (and cleaned all this up) by removing the vector of iterators, and turning the unordered_map into a normal vector. When we need access by name, we just go through all the items; it is always just a handful of them anyway. Signed-off-by: Petr Mladek --- extensions/source/propctrlr/browserlistbox.cxx | 138 ++++++++++++------------- extensions/source/propctrlr/browserlistbox.hxx | 23 ++--- 2 files changed, 73 insertions(+), 88 deletions(-) diff --git a/extensions/source/propctrlr/browserlistbox.cxx b/extensions/source/propctrlr/browserlistbox.cxx index f635ea6bda17..45ef9aaed5a1 100644 --- a/extensions/source/propctrlr/browserlistbox.cxx +++ b/extensions/source/propctrlr/browserlistbox.cxx @@ -470,7 +470,7 @@ namespace pcr UpdateVScroll(); - sal_Bool bNeedScrollbar = m_aOrderedLines.size() > (sal_uInt32)CalcVisibleLines(); + sal_Bool bNeedScrollbar = m_aLines.size() > (sal_uInt32)CalcVisibleLines(); if ( !bNeedScrollbar ) { if ( m_aVScroll.IsVisible() ) @@ -493,7 +493,7 @@ namespace pcr m_aVScroll.SetPosSizePixel( aVScrollPos, aVScrollSize ); } - for ( sal_uInt16 i = 0; i < m_aOrderedLines.size(); ++i ) + for ( sal_uInt16 i = 0; i < m_aLines.size(); ++i ) m_aOutOfDateLines.insert( i ); // repaint @@ -595,15 +595,16 @@ namespace pcr aPos.Y() += _nIndex * m_nRowHeight; - if ( _nIndex < m_aOrderedLines.size() ) + if ( _nIndex < m_aLines.size() ) { - m_aOrderedLines[ _nIndex ]->second.pLine->SetPosSizePixel( aPos, aSize ); + BrowserLinePointer pLine = m_aLines[ _nIndex ].pLine; - m_aOrderedLines[ _nIndex ]->second.pLine->SetTitleWidth( m_nTheNameSize + 2 * FRAME_OFFSET ); + pLine->SetPosSizePixel( aPos, aSize ); + pLine->SetTitleWidth( m_nTheNameSize + 2 * FRAME_OFFSET ); // show the line if necessary - if ( !m_aOrderedLines[ _nIndex ]->second.pLine->IsVisible() ) - m_aOrderedLines[ _nIndex ]->second.pLine->Show(); + if ( !pLine->IsVisible() ) + pLine->Show(); } } @@ -615,8 +616,8 @@ namespace pcr ++aLoop ) { - DBG_ASSERT( *aLoop < m_aOrderedLines.size(), "OBrowserListBox::UpdatePosNSize: invalid line index!" ); - if ( *aLoop < m_aOrderedLines.size() ) + DBG_ASSERT( *aLoop < m_aLines.size(), "OBrowserListBox::UpdatePosNSize: invalid line index!" ); + if ( *aLoop < m_aLines.size() ) PositionLine( *aLoop ); } m_aOutOfDateLines.clear(); @@ -629,10 +630,10 @@ namespace pcr sal_Int32 nLines = CalcVisibleLines(); sal_uInt16 nEnd = (sal_uInt16)(nThumbPos + nLines); - if (nEnd >= m_aOrderedLines.size()) - nEnd = (sal_uInt16)m_aOrderedLines.size()-1; + if (nEnd >= m_aLines.size()) + nEnd = (sal_uInt16)m_aLines.size()-1; - if ( !m_aOrderedLines.empty() ) + if ( !m_aLines.empty() ) { for ( sal_uInt16 i = (sal_uInt16)nThumbPos; i <= nEnd; ++i ) m_aOutOfDateLines.insert( i ); @@ -662,18 +663,21 @@ namespace pcr //------------------------------------------------------------------ void OBrowserListBox::SetPropertyValue(const ::rtl::OUString& _rEntryName, const Any& _rValue, bool _bUnknownValue ) { - ListBoxLines::iterator line = m_aLines.find( _rEntryName ); + ListBoxLines::iterator line = m_aLines.begin(); + for ( ; line != m_aLines.end() && ( line->aName != _rEntryName ); ++line ) + ; + if ( line != m_aLines.end() ) { if ( _bUnknownValue ) { - Reference< XPropertyControl > xControl( line->second.pLine->getControl() ); + Reference< XPropertyControl > xControl( line->pLine->getControl() ); OSL_ENSURE( xControl.is(), "OBrowserListBox::SetPropertyValue: illegal control!" ); if ( xControl.is() ) xControl->setValue( Any() ); } else - impl_setControlAsPropertyValue( line->second, _rValue ); + impl_setControlAsPropertyValue( *line, _rValue ); } } @@ -681,14 +685,14 @@ namespace pcr sal_uInt16 OBrowserListBox::GetPropertyPos( const ::rtl::OUString& _rEntryName ) const { sal_uInt16 nRet = LISTBOX_ENTRY_NOTFOUND; - for ( OrderedListBoxLines::const_iterator linePos = m_aOrderedLines.begin(); - linePos != m_aOrderedLines.end(); + for ( ListBoxLines::const_iterator linePos = m_aLines.begin(); + linePos != m_aLines.end(); ++linePos ) { - if ( (*linePos)->first == _rEntryName ) + if ( linePos->aName == _rEntryName ) { - nRet = (sal_uInt16)( linePos - m_aOrderedLines.begin() ); + nRet = (sal_uInt16)( linePos - m_aLines.begin() ); break; } } @@ -699,9 +703,12 @@ namespace pcr //------------------------------------------------------------------------ bool OBrowserListBox::impl_getBrowserLineForName( const ::rtl::OUString& _rEntryName, BrowserLinePointer& _out_rpLine ) const { - ListBoxLines::const_iterator line = m_aLines.find( _rEntryName ); + ListBoxLines::const_iterator line = m_aLines.begin(); + for ( ; line != m_aLines.end() && ( line->aName != _rEntryName ); ++line ) + ; + if ( line != m_aLines.end() ) - _out_rpLine = line->second.pLine; + _out_rpLine = line->pLine; else _out_rpLine.reset(); return ( NULL != _out_rpLine.get() ); @@ -738,21 +745,21 @@ namespace pcr // create a new line BrowserLinePointer pBrowserLine( new OBrowserLine( _rPropertyData.sName, &m_aLinesPlayground ) ); - ListBoxLine aNewLine( pBrowserLine, _rPropertyData.xPropertyHandler ); - ::std::pair< ListBoxLines::iterator, bool > insertPoint = - m_aLines.insert( ListBoxLines::value_type( _rPropertyData.sName, aNewLine ) ); - OSL_ENSURE( insertPoint.second, "OBrowserListBox::InsertEntry: already have another line for this name!" ); + // check that the name is unique + ListBoxLines::iterator it = m_aLines.begin(); + for ( ; it != m_aLines.end() && ( it->aName != _rPropertyData.sName ); ++it ) + ; + OSL_ENSURE( it == m_aLines.end(), "OBrowserListBox::InsertEntry: already have another line for this name!" ); + ListBoxLine aNewLine( _rPropertyData.sName, pBrowserLine, _rPropertyData.xPropertyHandler ); sal_uInt16 nInsertPos = _nPos; - if ( nInsertPos > m_aOrderedLines.size() ) - nInsertPos = EDITOR_LIST_APPEND; - if ( EDITOR_LIST_APPEND == nInsertPos ) + if ( _nPos >= m_aLines.size() ) { - nInsertPos = (sal_uInt16)m_aOrderedLines.size(); - m_aOrderedLines.push_back( insertPoint.first ); + nInsertPos = static_cast< sal_uInt16 >( m_aLines.size() ); + m_aLines.push_back( aNewLine ); } else - m_aOrderedLines.insert( m_aOrderedLines.begin() + nInsertPos, insertPoint.first ); + m_aLines.insert( m_aLines.begin() + _nPos, aNewLine ); pBrowserLine->SetTitleWidth(m_nTheNameSize); if (m_bUpdate) @@ -766,7 +773,7 @@ namespace pcr // update the positions of possibly affected lines sal_uInt16 nUpdatePos = nInsertPos; - while ( nUpdatePos < m_aOrderedLines.size() ) + while ( nUpdatePos < m_aLines.size() ) m_aOutOfDateLines.insert( nUpdatePos++ ); UpdatePosNSize( ); @@ -799,7 +806,7 @@ namespace pcr //------------------------------------------------------------------ void OBrowserListBox::ShowEntry(sal_uInt16 _nPos) { - if ( _nPos < m_aOrderedLines.size() ) + if ( _nPos < m_aLines.size() ) { sal_Int32 nThumbPos = m_aVScroll.GetThumbPos(); @@ -966,12 +973,10 @@ namespace pcr //------------------------------------------------------------------ sal_uInt16 OBrowserListBox::impl_getControlPos( const Reference< XPropertyControl >& _rxControl ) const { - for ( OrderedListBoxLines::const_iterator search = m_aOrderedLines.begin(); - search != m_aOrderedLines.end(); - ++search - ) - if ( (*search)->second.pLine->getControl().get() == _rxControl.get() ) - return sal_uInt16( search - m_aOrderedLines.begin() ); + for ( ListBoxLines::const_iterator search = m_aLines.begin(); search != m_aLines.end(); ++search ) + if ( search->pLine->getControl().get() == _rxControl.get() ) + return sal_uInt16( search - m_aLines.begin() ); + OSL_FAIL( "OBrowserListBox::impl_getControlPos: invalid control - not part of any of our lines!" ); return (sal_uInt16)-1; } @@ -1006,7 +1011,7 @@ namespace pcr if ( m_pLineListener ) { - const ListBoxLine& rLine = impl_getControlLine( _rxControl ); + const ListBoxLine& rLine = m_aLines[ impl_getControlPos( _rxControl ) ]; m_pLineListener->Commit( rLine.pLine->GetEntryName(), impl_getControlAsPropertyValue( rLine ) @@ -1023,18 +1028,16 @@ namespace pcr // cycle forwards, 'til we've the next control which can grab the focus ++nLine; - while ( (size_t)nLine < m_aOrderedLines.size() ) + while ( static_cast< size_t >( nLine ) < m_aLines.size() ) { - if ( m_aOrderedLines[nLine]->second.pLine->GrabFocus() ) + if ( m_aLines[nLine].pLine->GrabFocus() ) break; ++nLine; } - if ( ( (size_t)nLine >= m_aOrderedLines.size() ) - && ( m_aOrderedLines.size() > 0 ) - ) - // wrap around - m_aOrderedLines[0]->second.pLine->GrabFocus(); + // wrap around? + if ( ( static_cast< size_t >( nLine ) >= m_aLines.size() ) && ( m_aLines.size() > 0 ) ) + m_aLines[0].pLine->GrabFocus(); } //------------------------------------------------------------------ @@ -1062,40 +1065,33 @@ namespace pcr //------------------------------------------------------------------ void OBrowserListBox::Clear() { - for ( ListBoxLines::iterator loop = m_aLines.begin(); - loop != m_aLines.end(); - ++loop - ) + for ( ListBoxLines::iterator loop = m_aLines.begin(); loop != m_aLines.end(); ++loop ) { // hide the line - loop->second.pLine->Hide(); + loop->pLine->Hide(); // reset the listener - lcl_implDisposeControl_nothrow( loop->second.pLine->getControl() ); + lcl_implDisposeControl_nothrow( loop->pLine->getControl() ); } clearContainer( m_aLines ); - clearContainer( m_aOrderedLines ); } //------------------------------------------------------------------ sal_Bool OBrowserListBox::RemoveEntry( const ::rtl::OUString& _rName ) { - sal_uInt16 nPos = GetPropertyPos( _rName ); - if ( nPos == LISTBOX_ENTRY_NOTFOUND ) - return sal_False; + sal_uInt16 nPos = 0; + ListBoxLines::iterator it = m_aLines.begin(); + for ( ; it != m_aLines.end() && ( it->aName != _rName ); ++it, ++nPos ) + ; - OrderedListBoxLines::iterator orderedPos = m_aOrderedLines.begin() + nPos; - BrowserLinePointer pLine = (*orderedPos)->second.pLine; - pLine->Hide(); - lcl_implDisposeControl_nothrow( pLine->getControl() ); + if ( it == m_aLines.end() ) + return sal_False; - m_aLines.erase( *orderedPos ); - m_aOrderedLines.erase( orderedPos ); - m_aOutOfDateLines.erase( (sal_uInt16)m_aOrderedLines.size() ); - // this index *may* have been out of date, which is obsoleted now by m_aOrderedLines shrinking + m_aLines.erase( it ); + m_aOutOfDateLines.erase( (sal_uInt16)m_aLines.size() ); // update the positions of possibly affected lines - while ( nPos < m_aOrderedLines.size() ) + while ( nPos < m_aLines.size() ) m_aOutOfDateLines.insert( nPos++ ); UpdatePosNSize( ); @@ -1112,14 +1108,14 @@ namespace pcr if ( nPos == EDITOR_LIST_REPLACE_EXISTING ) nPos = GetPropertyPos( _rPropertyData.sName ); - if ( nPos < m_aOrderedLines.size() ) + if ( nPos < m_aLines.size() ) { Window* pRefWindow = NULL; if ( nPos > 0 ) - pRefWindow = m_aOrderedLines[nPos-1]->second.pLine->GetRefWindow(); + pRefWindow = m_aLines[nPos-1].pLine->GetRefWindow(); // the current line and control - ListBoxLine& rLine = m_aOrderedLines[nPos]->second; + ListBoxLine& rLine = m_aLines[nPos]; // the old control and some data about it Reference< XPropertyControl > xControl = rLine.pLine->getControl(); @@ -1257,9 +1253,9 @@ namespace pcr nFocusControlPos = (sal_uInt16)nNewThumbPos + CalcVisibleLines() - 1; if ( nFocusControlPos ) { - if ( nFocusControlPos < m_aOrderedLines.size() ) + if ( nFocusControlPos < m_aLines.size() ) { - m_aOrderedLines[ nFocusControlPos ]->second.pLine->GrabFocus(); + m_aLines[ nFocusControlPos ].pLine->GrabFocus(); } else OSL_FAIL( "OBrowserListBox::PreNotify: internal error, invalid focus control position!" ); diff --git a/extensions/source/propctrlr/browserlistbox.hxx b/extensions/source/propctrlr/browserlistbox.hxx index 3f6e4c995e52..c5c46ef4898a 100644 --- a/extensions/source/propctrlr/browserlistbox.hxx +++ b/extensions/source/propctrlr/browserlistbox.hxx @@ -65,19 +65,19 @@ namespace pcr typedef ::boost::shared_ptr< OBrowserLine > BrowserLinePointer; struct ListBoxLine { + ::rtl::OUString aName; BrowserLinePointer pLine; ::com::sun::star::uno::Reference< ::com::sun::star::inspection::XPropertyHandler > xHandler; - ListBoxLine() { } - ListBoxLine( BrowserLinePointer _pLine, const ::com::sun::star::uno::Reference< ::com::sun::star::inspection::XPropertyHandler >& _rxHandler ) - :pLine( _pLine ) - ,xHandler( _rxHandler ) + ListBoxLine( const ::rtl::OUString& rName, BrowserLinePointer _pLine, const ::com::sun::star::uno::Reference< ::com::sun::star::inspection::XPropertyHandler >& _rxHandler ) + : aName( rName ), + pLine( _pLine ), + xHandler( _rxHandler ) { } }; - typedef ::boost::unordered_map< ::rtl::OUString, ListBoxLine, ::rtl::OUStringHash > ListBoxLines; - typedef ::std::vector< ListBoxLines::iterator > OrderedListBoxLines; + typedef ::std::vector< ListBoxLine > ListBoxLines; //======================================================================== //= IControlContext @@ -106,7 +106,6 @@ namespace pcr ::std::auto_ptr< InspectorHelpWindow > m_pHelpWindow; ListBoxLines m_aLines; - OrderedListBoxLines m_aOrderedLines; IPropertyLineListener* m_pLineListener; IPropertyControlObserver* m_pControlObserver; long m_nYOffset; @@ -192,16 +191,6 @@ namespace pcr */ sal_uInt16 impl_getControlPos( const ::com::sun::star::uno::Reference< ::com::sun::star::inspection::XPropertyControl >& _rxControl ) const; - /** retrieves (a reference to) the ->ListBoxLine for a given control - @param _rxControl - The control to lookup. Must denote a control of one of the lines in ->m_aLines - */ - inline const ListBoxLine& - impl_getControlLine( const ::com::sun::star::uno::Reference< ::com::sun::star::inspection::XPropertyControl >& _rxControl ) const - { - return m_aOrderedLines[ impl_getControlPos( _rxControl ) ]->second; - } - /** sets the given property value at the given control, after converting it as necessary @param _rLine The line whose at which the value is to be set. -- cgit v1.2.3