forms/source/component/Grid.cxx | 9 -- forms/source/component/formcontrolfont.cxx | 120 ++++++++++++++++++----------- forms/source/component/navigationbar.cxx | 9 -- forms/source/inc/formcontrolfont.hxx | 11 ++ forms/source/richtext/richtextmodel.cxx | 9 -- 5 files changed, 97 insertions(+), 61 deletions(-)
New commits: commit 6aefcb6a6f90896754f3432e5ae41403998b7ab0 Author: Michael Stahl <[email protected]> Date: Wed Nov 27 17:08:22 2013 +0100 forms: avoid deadlock when setting FontControlModel properties Deadlock found in forms_unoapi on Windows, with OGridControlModel::setFastPropertyValue_NoBroadcast() calling event handler that tries to lock SolarMutex. FontControlModel::setFastPropertyValue_NoBroadcast() and its callers in 3 other classes must not send events via firePropertyChange() because they are called by OPropertySetHelper::setFastPropertyValues() with its Mutex locked. It is possible (though sadly quite convoluted) to delay the sending of the events by calling setDependentFastPropertyValue() instead. Change-Id: I0c767cfec01fe1bcaeb1236287b5faf81a2e7441 diff --git a/forms/source/component/Grid.cxx b/forms/source/component/Grid.cxx index c81675f..bb52635 100644 --- a/forms/source/component/Grid.cxx +++ b/forms/source/component/Grid.cxx @@ -707,12 +707,9 @@ void OGridControlModel::setFastPropertyValue_NoBroadcast( sal_Int32 nHandle, con default: if ( isFontRelatedProperty( nHandle ) ) { - FontDescriptor aOldFont( getFont() ); - - FontControlModel::setFastPropertyValue_NoBroadcast( nHandle, rValue ); - - if ( isFontAggregateProperty( nHandle ) ) - firePropertyChange( PROPERTY_ID_FONT, makeAny( getFont() ), makeAny( aOldFont ) ); + FontControlModel::setFastPropertyValue_NoBroadcast_impl( + *this, &OGridControlModel::setDependentFastPropertyValue, + nHandle, rValue); } else OControlModel::setFastPropertyValue_NoBroadcast( nHandle, rValue ); diff --git a/forms/source/component/formcontrolfont.cxx b/forms/source/component/formcontrolfont.cxx index 5745802..4b85cae 100644 --- a/forms/source/component/formcontrolfont.cxx +++ b/forms/source/component/formcontrolfont.cxx @@ -20,6 +20,7 @@ #include "formcontrolfont.hxx" #include "property.hrc" #include "property.hxx" +#include <cppuhelper/propshlp.hxx> #include <comphelper/property.hxx> #include <comphelper/types.hxx> #include <tools/color.hxx> @@ -351,104 +352,139 @@ namespace frm } //------------------------------------------------------------------------------ - void FontControlModel::setFastPropertyValue_NoBroadcast( sal_Int32 _nHandle, const Any& _rValue ) throw ( Exception ) + static void setFastPropertyValue_NoBroadcast_implimpl( + FontDescriptor & rFont, + sal_Int32 nHandle, const Any& rValue) throw (Exception) { - switch( _nHandle ) + switch (nHandle) { - case PROPERTY_ID_TEXTCOLOR: - m_aTextColor = _rValue; - break; - - case PROPERTY_ID_TEXTLINECOLOR: - m_aTextLineColor = _rValue; - break; - - case PROPERTY_ID_FONTEMPHASISMARK: - _rValue >>= m_nFontEmphasis; - break; - - case PROPERTY_ID_FONTRELIEF: - _rValue >>= m_nFontRelief; - break; - - case PROPERTY_ID_FONT: - _rValue >>= m_aFont; - break; - case PROPERTY_ID_FONT_NAME: - _rValue >>= m_aFont.Name; + rValue >>= rFont.Name; break; case PROPERTY_ID_FONT_STYLENAME: - _rValue >>= m_aFont.StyleName; + rValue >>= rFont.StyleName; break; case PROPERTY_ID_FONT_FAMILY: - _rValue >>= m_aFont.Family; + rValue >>= rFont.Family; break; case PROPERTY_ID_FONT_CHARSET: - _rValue >>= m_aFont.CharSet; + rValue >>= rFont.CharSet; break; case PROPERTY_ID_FONT_CHARWIDTH: - _rValue >>= m_aFont.CharacterWidth; + rValue >>= rFont.CharacterWidth; break; case PROPERTY_ID_FONT_KERNING: - _rValue >>= m_aFont.Kerning; + rValue >>= rFont.Kerning; break; case PROPERTY_ID_FONT_ORIENTATION: - _rValue >>= m_aFont.Orientation; + rValue >>= rFont.Orientation; break; case PROPERTY_ID_FONT_PITCH: - _rValue >>= m_aFont.Pitch; + rValue >>= rFont.Pitch; break; case PROPERTY_ID_FONT_TYPE: - _rValue >>= m_aFont.Type; + rValue >>= rFont.Type; break; case PROPERTY_ID_FONT_WIDTH: - _rValue >>= m_aFont.Width; + rValue >>= rFont.Width; break; case PROPERTY_ID_FONT_HEIGHT: { float nHeight = 0; - _rValue >>= nHeight; - m_aFont.Height = (sal_Int16)nHeight; + rValue >>= nHeight; + rFont.Height = (sal_Int16)nHeight; } break; case PROPERTY_ID_FONT_WEIGHT: - _rValue >>= m_aFont.Weight; + rValue >>= rFont.Weight; break; case PROPERTY_ID_FONT_SLANT: - _rValue >>= m_aFont.Slant; + rValue >>= rFont.Slant; break; case PROPERTY_ID_FONT_UNDERLINE: - _rValue >>= m_aFont.Underline; + rValue >>= rFont.Underline; break; case PROPERTY_ID_FONT_STRIKEOUT: - _rValue >>= m_aFont.Strikeout; + rValue >>= rFont.Strikeout; break; case PROPERTY_ID_FONT_WORDLINEMODE: { sal_Bool bWordLineMode = sal_False; - _rValue >>= bWordLineMode; - m_aFont.WordLineMode = bWordLineMode; + rValue >>= bWordLineMode; + rFont.WordLineMode = bWordLineMode; } break; default: - OSL_FAIL( "FontControlModel::setFastPropertyValue_NoBroadcast: invalid property!" ); + assert(false); // isFontAggregateProperty + } + } + + void FontControlModel::setFastPropertyValue_NoBroadcast_impl( + ::cppu::OPropertySetHelper & rBase, + void (::cppu::OPropertySetHelper::*pSet)(sal_Int32, Any const&), + sal_Int32 nHandle, const Any& rValue) throw (Exception) + { + if (isFontAggregateProperty(nHandle)) + { + // need to fire a event for PROPERTY_ID_FONT too apparently, so: + FontDescriptor font(getFont()); + + // first set new value on backup copy + setFastPropertyValue_NoBroadcast_implimpl(font, nHandle, rValue); + + // then set that as the actual property - will eventually call + // this method recursively again... + (rBase.*pSet)(PROPERTY_ID_FONT, makeAny(font)); +#ifndef NDEBUG + // verify that the nHandle property has the new value + Any tmp; + getFastPropertyValue(tmp, nHandle); + assert(tmp == rValue || PROPERTY_ID_FONT_HEIGHT == nHandle /*rounded*/); +#endif + } + else + { + switch (nHandle) + { + case PROPERTY_ID_TEXTCOLOR: + m_aTextColor = rValue; + break; + + case PROPERTY_ID_TEXTLINECOLOR: + m_aTextLineColor = rValue; + break; + + case PROPERTY_ID_FONTEMPHASISMARK: + rValue >>= m_nFontEmphasis; + break; + + case PROPERTY_ID_FONTRELIEF: + rValue >>= m_nFontRelief; + break; + + case PROPERTY_ID_FONT: + rValue >>= m_aFont; + break; + + default: + SAL_WARN("forms.component", "invalid property: " << nHandle); + } } } diff --git a/forms/source/component/navigationbar.cxx b/forms/source/component/navigationbar.cxx index 9baeea7..0aa5311 100644 --- a/forms/source/component/navigationbar.cxx +++ b/forms/source/component/navigationbar.cxx @@ -403,12 +403,9 @@ namespace frm } else if ( isFontRelatedProperty( _nHandle ) ) { - FontDescriptor aOldFont( getFont() ); - - FontControlModel::setFastPropertyValue_NoBroadcast( _nHandle, _rValue ); - - if ( isFontAggregateProperty( _nHandle ) ) - firePropertyChange( PROPERTY_ID_FONT, makeAny( getFont() ), makeAny( aOldFont ) ); + FontControlModel::setFastPropertyValue_NoBroadcast_impl( + *this, &ONavigationBarModel::setDependentFastPropertyValue, + _nHandle, _rValue); } else { diff --git a/forms/source/inc/formcontrolfont.hxx b/forms/source/inc/formcontrolfont.hxx index 4e802ba..e29d93d 100644 --- a/forms/source/inc/formcontrolfont.hxx +++ b/forms/source/inc/formcontrolfont.hxx @@ -25,6 +25,10 @@ #include <com/sun/star/beans/Property.hpp> #include <com/sun/star/lang/IllegalArgumentException.hpp> +namespace cppu { + class OPropertySetHelper; +} + //......................................................................... namespace frm { @@ -74,7 +78,12 @@ namespace frm void getFastPropertyValue ( ::com::sun::star::uno::Any& _rValue, sal_Int32 _nHandle ) const; sal_Bool convertFastPropertyValue ( ::com::sun::star::uno::Any& _rConvertedValue, ::com::sun::star::uno::Any& _rOldValue, sal_Int32 _nHandle, const ::com::sun::star::uno::Any& _rValue ) throw( ::com::sun::star::lang::IllegalArgumentException ); - void setFastPropertyValue_NoBroadcast( sal_Int32 _nHandle, const ::com::sun::star::uno::Any& _rValue ) throw ( ::com::sun::star::uno::Exception ); + void setFastPropertyValue_NoBroadcast_impl( + ::cppu::OPropertySetHelper & rBase, + void (::cppu::OPropertySetHelper::*pSet)( + sal_Int32, css::uno::Any const&), + sal_Int32 nHandle, const ::com::sun::star::uno::Any& rValue) + throw ( ::com::sun::star::uno::Exception ); ::com::sun::star::uno::Any getPropertyDefaultByHandle ( sal_Int32 _nHandle ) const; diff --git a/forms/source/richtext/richtextmodel.cxx b/forms/source/richtext/richtextmodel.cxx index 226badd..592b0d2 100644 --- a/forms/source/richtext/richtextmodel.cxx +++ b/forms/source/richtext/richtextmodel.cxx @@ -391,12 +391,9 @@ namespace frm } else if ( isFontRelatedProperty( _nHandle ) ) { - FontDescriptor aOldFont( getFont() ); - - FontControlModel::setFastPropertyValue_NoBroadcast( _nHandle, _rValue ); - - if ( isFontAggregateProperty( _nHandle ) ) - firePropertyChange( PROPERTY_ID_FONT, makeAny( getFont() ), makeAny( aOldFont ) ); + FontControlModel::setFastPropertyValue_NoBroadcast_impl( + *this, &ORichTextModel::setDependentFastPropertyValue, + _nHandle, _rValue); } else { commit e521a803c914e0d3718ae795976948aabbb9c76c Author: Michael Stahl <[email protected]> Date: Wed Nov 27 17:06:34 2013 +0100 FontControlModel::convertFastPropertyValue: fix bad cast of Kerning Casting sal_Bool to integer lets the comparison always fail. Change-Id: I33cf9e9b6a65f81166870bdfe32e9a97101501df diff --git a/forms/source/component/formcontrolfont.cxx b/forms/source/component/formcontrolfont.cxx index cd351e5..5745802 100644 --- a/forms/source/component/formcontrolfont.cxx +++ b/forms/source/component/formcontrolfont.cxx @@ -301,7 +301,7 @@ namespace frm break; case PROPERTY_ID_FONT_KERNING: - bModified = tryPropertyValue( _rConvertedValue, _rOldValue, _rValue, (sal_Int16)m_aFont.Kerning ); + bModified = tryPropertyValue( _rConvertedValue, _rOldValue, _rValue, m_aFont.Kerning ); break; case PROPERTY_ID_FONT_ORIENTATION: _______________________________________________ Libreoffice-commits mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
