Re: fdo#46808, Convert awt::UnoControlDialogModel to new style problem

[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