[had inadvertently dropped the ML]

On 05/24/2013 05:46 PM, Stephan Bergmann wrote:
On 05/24/2013 01:30 PM, Noel Grandin wrote:
OK, so it turns out that my change
     fdo#46808, Convert awt::UnoControlDialogModel to new style
http://cgit.freedesktop.org/libreoffice/core/commit/?id=6c61b20a8d4a6dcac28801cde82a211fb7e30654


has been causing some problems, notably around getting and setting
properties.
[...]
The problem is that after my change, this:
       Reference< beans::XPropertySet > xDlgPSet( xDialogModel,
UNO_QUERY )
       Any aStringResourceManagerAny;
       aStringResourceManagerAny <<= xStringResourceManager;
       xDlgPSet->setPropertyValue( aResourceResolverPropName,
aStringResourceManagerAny );
is not equivalent to this:
      Any aStringResourceManagerAny;
      aStringResourceManagerAny <<= xStringResourceManager;
      xDialogModel->setPropertyValue( aResourceResolverPropName,
aStringResourceManagerAny );

For context the type hierarchy looks like this:
    UnoControlDialogModel --> ControlModelContainerBase -->
UnoControlModel --> ::cppu::OPropertySetHelper

So when my new code calls setPropertyValue, it ends up here:
     // overrides to resolve ambiguity
     virtual void  UnoControlDialogModel::setPropertyValue(const
OUString& p1, Any& p2)  throw (some stuff)
     {
[...]
     }
but if we first cast to Reference<XPropertySet>, and then do the call,
we end up in
     cppu::OPropertySetHelper::setPropertyValue()
and all is well.

The problem is that the implementation of the
css.awt.UnoControlDialogModel involves UNO aggregation
(IMPL_CREATE_INSTANCE_WITH_GEOMETRY(UnoControlDialogModel) in
toolkit/soruce/helper/registerservices.cxx creating a
OGeometryControlModel<UnoControlDialogModel> instance that aggregates a
UnoControlDialogModel instance).  That means that queryInterface can
return a reference to something that is technically a different object,
and that's what's happening here, and explains why calling
setPropertyValue in two different ways on what logically appears to be a
single object can end up calling two different implementations (of two
different physical objects).  (UNO aggregation is known to be broken and
should not be used.  Nevertheless, there's still code that does---code
that is a horrible mess and hard to clean up.)

That all this worked as intended in the past is just sheer luck, but any
way of substantially touching it is asking for trouble.  I'm going to
revert 6c61b20a8d4a6dcac28801cde82a211fb7e30654 again.

Stephan

_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Reply via email to