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

[Stephan Bergmann]

[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