Re: Deletion of vcl::Window's inside ToolBar

[Maxim Monastirsky]

Hi Dennis,

On Wed, Sep 2, 2015 at 6:15 PM, Dennis Francis 
 wrote:
Should I remove the newly added dispose() functions (for FontList 
etc..) as well ?
Sure, given that they're not needed anymore. And we can always 
reintroduce them if anything goes wrong with the current approach. 
Thank you so much for your hard work on this one!


Maxim

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

Re: Deletion of vcl::Window's inside ToolBar

[Dennis Francis]

Hi Maxim, Michael

I have submitted the amended patch. Thanks a lot for the help, I am sure I
learned many things from this discussion.

Thanks,
Dennis

On Fri, Sep 4, 2015 at 2:32 PM, Maxim Monastirsky 
wrote:

> Hi Dennis,
>
> On Wed, Sep 2, 2015 at 6:15 PM, Dennis Francis 
> wrote:
>
>> Should I remove the newly added dispose() functions (for FontList etc..)
>> as well ?
>>
> Sure, given that they're not needed anymore. And we can always reintroduce
> them if anything goes wrong with the current approach. Thank you so much
> for your hard work on this one!
>
> Maxim
>
>
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Deletion of vcl::Window's inside ToolBar

[Dennis Francis]

Hi Maxim

>From the FontList methods, it looks like it uses GetDevFontSize(),
GetMapMode(), SetMapMode() and GetDevFontSizeCount() methods of
the object that is passed to FontList constructor. All these methods are
only defined for OutputDevice class. So it would seem that passing the
parent window should have
the same effect. Valgrind no more shows that memleak if we pass
pBox->GetParent().

Should I remove the newly added dispose() functions (for FontList etc..) as
well ? Seems the above one liner change is enough to kill the memleak.

Thanks,
Dennis

On Tue, Sep 1, 2015 at 2:56 AM, Maxim Monastirsky 
wrote:

> Hi,
>
> On Mon, Aug 31, 2015 at 10:00 PM, Dennis Francis <
> dennisfrancis...@gmail.com> wrote:
>
>> About passing SvxFontNameBox_Impl : I have not yet understood why we pass
>> that, will check as soon as possible.
>>
> I will clarify what I meant. It seems to me that we pass the
> SvxFontNameBox_Impl only to get through it the font list from the
> underlying platform backend. In this case, we can do the same with other
> windows. For example, passing the ToolBox there (via pBox->GetParent())
> seems to eliminate the memleak, while the font box seems (at least at first
> glance) to still work as expected. But you should take this with caution,
> given that I'm not really familiar with the font init stuff.
>
> Maxim
>
>
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Deletion of vcl::Window's inside ToolBar

[Michael Meeks]

Hi Dennis,

On Fri, 2015-08-28 at 09:28 +0530, Dennis Francis wrote:
> I tried calling disposeAndClear() on mpWindow but the object remains
> intact with a lower mnRefCnt of 207. After putting address of
> mnRefCount on gdb watch,

Heh =)

> it was clear that ref counts gets added for each font added to the
> font list box widget while calling the constructor of
> ImplFontListFontInfo.
> In ImplFontListFontInfo the OutputDevice is held inside VclPtr wrapper
> (mpDevice) which is never released.

Right. So - what we need to do is to add code to the 'dispose' method
of whatever is holding these handles to clear the references. The root
problem is that the widget holding those is not clearing up its own
references properly in dispose.

So - there is some deepish nesting here AFAICS - all of which needs
some simple cleanup methods.

ImplFontListFontInfo needs a dispose method that does:
mpDevice.clear();

ImplFontListNameInfo needs a dispose method that walks its
list of ImplFontListFontInfo (via mpFirst I guess)
calling dispose() on those.

FontList needs a dispose method
disposing each of its ptr_vector members
also 'clear()' ing each of it's mpDev, mpDev2 members

And we of course need to dispose the FontList whereever it is
instantiated in a widget: eventually we'll have to hit a 'dispose'
method going up the tree.

Is that feasible ?

Thanks so much for chasing this one Dennis !

ATB,

Michael.

-- 
 michael.me...@collabora.com  <><, Pseudo Engineer, itinerant idiot

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

Re: Deletion of vcl::Window's inside ToolBar

[Dennis Francis]

Hi Maxim

Thanks for pointing out, I have taken out my call of disposeAndClear().
About passing SvxFontNameBox_Impl : I have not yet understood why we pass
that, will check as soon as possible.

Hi Michael

Thanks for the suggestions. I have attempted to do what you suggested in
the latest patch uploaded at gerrit (
https://gerrit.libreoffice.org/#/c/18073/ )
I used opengrok to get a list of all usages of the token FontList, and
tried to figure out which all objects instantiate FontList, there are
places where I
am not sure of the correctness of my patch. At least the unit tests did not
fail and the original memleak is also gone :) But I am scared if introduced
crashes in some
untested work flows.

Could you please review and suggest changes ?


Thanks,
Dennis


On Mon, Aug 31, 2015 at 12:42 PM, Michael Meeks 
wrote:
>
> And we of course need to dispose the FontList whereever it is
> instantiated in a widget: eventually we'll have to hit a 'dispose'
> method going up the tree.
>
> Is that feasible ?
>
> Thanks so much for chasing this one Dennis !
>
> ATB,
>
> Michael.
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Deletion of vcl::Window's inside ToolBar

[Maxim Monastirsky]

Hi,

On Mon, Aug 31, 2015 at 10:00 PM, Dennis Francis 
 wrote:
About passing SvxFontNameBox_Impl : I have not yet understood why we 
pass that, will check as soon as possible.
I will clarify what I meant. It seems to me that we pass the 
SvxFontNameBox_Impl only to get through it the font list from the 
underlying platform backend. In this case, we can do the same with 
other windows. For example, passing the ToolBox there (via 
pBox->GetParent()) seems to eliminate the memleak, while the font box 
seems (at least at first glance) to still work as expected. But you 
should take this with caution, given that I'm not really familiar with 
the font init stuff.


Maxim

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

Re: Deletion of vcl::Window's inside ToolBar

[Maxim Monastirsky]

On Fri, Aug 28, 2015 at 6:58 AM, Dennis Francis 
dennisfrancis...@gmail.com wrote:

Please suggest any better methods that I may have missed.
Another suggestion: It seems that we can pass any window as argument to 
the FontList ctor, and get the same results. If this is true (which I'm 
not sure, so should be double checked), the question is why do we pass 
there (in lcl_GetDocFontList) SvxFontNameBox_Impl itself in the first 
place?


Maxim

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

Re: Deletion of vcl::Window's inside ToolBar

[Maxim Monastirsky]

Hi Dennis,

On Fri, Aug 28, 2015 at 6:58 AM, Dennis Francis 
dennisfrancis...@gmail.com wrote:

I tried calling disposeAndClear() on mpWindow
JFYI there is already a call to disposeAndClear() in 
SfxToolBoxControl::dispose().


it was clear that ref counts gets added for each font added to the 
font list box widget while calling the constructor of 
ImplFontListFontInfo.

Good finding.


Please suggest any better methods that I may have missed.
Well, for me adding a manual m_aOwnFontList.reset() to 
SvxFontNameBox_Impl::dispose() seems to do the job.


Maxim

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

Re: Deletion of vcl::Window's inside ToolBar

[Dennis Francis]

Hi Michael and all,

I tried calling disposeAndClear() on mpWindow but the object remains intact
with a lower mnRefCnt of 207. After putting address of mnRefCount on gdb
watch,
it was clear that ref counts gets added for each font added to the font
list box widget while calling the constructor of ImplFontListFontInfo.
In ImplFontListFontInfo the OutputDevice is held inside VclPtr wrapper
(mpDevice) which is never released.
Changing mpDevice  to ordinary pointer, reduced the residual refcount of
the window from 207 to 1 after disposeAndClear()
The remaining one reference was held in FontList object whose destructor
call depends on SvxFontNameBox_Impl dtor call, which is our original
problem.
After converting mpDev and mpDev2 in FontList to ordinary pointers,
mpWindow.disposeAndClear() results in calling of
SvxFontNameBox_Impl dtor and now valgrind does not show such a leak.

I also tried to think of a way to do .clear() on these VclPtr wrapped
OutputDevice objects in FontList and ImplFontListFontInfo without having to
change them to
ordinary pointers, but the catch to catch situation of calling
SvxFontNameBox_Impl dtor prevailed. Please suggest any better methods that
I may have missed.

I have submitted an initial patch in gerrit containing these changes.
https://gerrit.libreoffice.org/#/c/18073/

Thanks,
Dennis

On Wed, Aug 26, 2015 at 6:20 PM, Michael Meeks michael.me...@collabora.com
wrote:

 Hi Dennis,

 On Wed, 2015-08-26 at 17:59 +0530, Dennis Francis wrote:
  After entering ToolBarManager::RemoveControllers, I put a break point
  on SvxFontNameBox_Impl::~SvxFontNameBox_Impl and then did
  mpWindow.clear() in gdb (where mpWindow is the SvxFontNameBox_Impl
  object wrapped in VclPtr).

 Interesting. Is it possible that there should be a disposeAndClear
 there ?

  But gdb did not show a call on the dtor ~SvxFontNameBox_Impl. After
  further digging, I found that the mnRefCnt of the mpWindow in question
  was 219 just before the VclPtr was reset.

 So - for a given un-disposed window - I'd expect lots of VclPtr
 references - but its unusual to have that many.

  Now I wonder who else increased the ref count of this mpWindow object,
  why the value is so high and how to proceed from here ?

 VCL itself maintains a whole slew of these - eg. the impl. of a
 window
 points to the next window, the previous window, top-levels are
 maintained in lists left  right etc. parents hold references to
 children and vv. (depends how many children it has).

 In order to (hopefully) break all these referencing cycles - it is
 necessary to call 'disposeOnce' or 'disposeAndClear' on the VclPtr -
 which stats the process of tidying up all the references.

 I miss the context but reading vcl/README.lifecycle would
 probably be helpful.

 HTH,

 Michael.

 --
  michael.me...@collabora.com  , Pseudo Engineer, itinerant idiot


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

Re: Deletion of vcl::Window's inside ToolBar

[Dennis Francis]

Hi Caolán and Maxim

Thanks for your replies, My reply is inline.

On Tue, Aug 25, 2015 at 7:38 PM, Caolán McNamara caol...@redhat.com wrote:

 Digging into the other examples, I guess that SetItemWindow was called
 on the toolbar with the SvxFontNameBox_Impl window as an argument ?

yes.


 If that's the case then the pattern appears to be that what object
 called SetItemWindow on the toolbar should call SetItemWindow(id, 0) to
 remove the window from the toolbar and then call disposeAndClear on the
 window.

 i.e. what entity put the item into the toolbar is the same entity that
 has to remove it and dispose it.

 What's the reproducing scenario here, is it the fontname widget in the
 standard toolbar, e.g. just start and exit writer to reproduce ?


I think it should work with writer too, I ran soffice --calc and opened a
new spreadsheet and then closed it.


 C.


Here is what I observe in gdb :
===
breakpoints were set on :
SvxFontNameBox_Impl::SvxFontNameBox_Impl()
SvxFontNameBox_Impl::FillList()
SvxFontNameBox_Impl::~SvxFontNameBox_Impl()

Order of calls observed from gdb each with address of SvxFontNameBox_Impl
object :
1. (SvxFontNameBox_Impl * const) 0x1cd4290,
SvxFontNameBox_Impl::SvxFontNameBox_Impl - from toolbar code
2. (SvxFontNameBox_Impl * const) 0x1cd4290, SvxFontNameBox_Impl::FillList
3. (SvxFontNameBox_Impl * const) 0x1e60cc0,
SvxFontNameBox_Impl::SvxFontNameBox_Impl  - from sidebar::ControllerFactory
4. (SvxFontNameBox_Impl * const) 0x1e60cc0,
SvxFontNameBox_Impl::~SvxFontNameBox_Impl - sidebar
5. (SvxFontNameBox_Impl * const) 0x1e60cc0,
SvxFontNameBox_Impl::~SvxFontNameBox_Impl - sidebar


No call of destructor SvxFontNameBox_Impl::~SvxFontNameBox_Impl() for the
instance 0x1cd4290
==

I tried setting break point on framework::ToolBarManager::RemoveControllers
and it calls SetItemWindow(id, 0) for all items :
m_pToolBar-SetItemWindow(nItemId, 0);

But in ToolBox::SetItemWindow() method, I think the pItem-mpWindow is just
overwritten and not getting deleted. Is the deletion code missing here ?
Is it ok to add

 if (pItem-mpWindow)
delete pItem-mpWindow;

before assigning pNewWindow to it ?


   1296 void ToolBox::SetItemWindow( sal_uInt16 nItemId, vcl::Window*
pNewWindow )
   1297 {
   1298 sal_uInt16 nPos = GetItemPos( nItemId );
   1299
   1300 if ( nPos != TOOLBOX_ITEM_NOTFOUND )
   1301 {
   1302 ImplToolItem* pItem = mpData-m_aItems[nPos];
   1303* pItem-mpWindow = pNewWindow;*
   1304 if ( pNewWindow )
   1305 pNewWindow-Hide();
   1306 ImplInvalidate( true );
   1307 CallEventListeners( VCLEVENT_TOOLBOX_ITEMWINDOWCHANGED,
reinterpret_cast void* ( nPos ) );
   1308 }
   1309 }

Thanks,
Dennis

http://www.ldcs.co.in
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Deletion of vcl::Window's inside ToolBar

[Dennis Francis]

Sorry, I understand now that pItem-mpWindow is a smart pointer (VclPtr).
So the issue may be elsewhere.

Thanks,
Dennis

On Wed, Aug 26, 2015 at 3:42 PM, Dennis Francis dennisfrancis...@gmail.com
wrote:

 Hi Caolán and Maxim

 Thanks for your replies, My reply is inline.

 On Tue, Aug 25, 2015 at 7:38 PM, Caolán McNamara caol...@redhat.com
 wrote:

 Digging into the other examples, I guess that SetItemWindow was called
 on the toolbar with the SvxFontNameBox_Impl window as an argument ?

 yes.


 If that's the case then the pattern appears to be that what object
 called SetItemWindow on the toolbar should call SetItemWindow(id, 0) to
 remove the window from the toolbar and then call disposeAndClear on the
 window.

 i.e. what entity put the item into the toolbar is the same entity that
 has to remove it and dispose it.

 What's the reproducing scenario here, is it the fontname widget in the
 standard toolbar, e.g. just start and exit writer to reproduce ?


 I think it should work with writer too, I ran soffice --calc and opened a
 new spreadsheet and then closed it.


 C.


 Here is what I observe in gdb :
 ===
 breakpoints were set on :
 SvxFontNameBox_Impl::SvxFontNameBox_Impl()
 SvxFontNameBox_Impl::FillList()
 SvxFontNameBox_Impl::~SvxFontNameBox_Impl()

 Order of calls observed from gdb each with address of SvxFontNameBox_Impl
 object :
 1. (SvxFontNameBox_Impl * const) 0x1cd4290,
 SvxFontNameBox_Impl::SvxFontNameBox_Impl - from toolbar code
 2. (SvxFontNameBox_Impl * const) 0x1cd4290, SvxFontNameBox_Impl::FillList
 3. (SvxFontNameBox_Impl * const) 0x1e60cc0,
 SvxFontNameBox_Impl::SvxFontNameBox_Impl  - from sidebar::ControllerFactory
 4. (SvxFontNameBox_Impl * const) 0x1e60cc0,
 SvxFontNameBox_Impl::~SvxFontNameBox_Impl - sidebar
 5. (SvxFontNameBox_Impl * const) 0x1e60cc0,
 SvxFontNameBox_Impl::~SvxFontNameBox_Impl - sidebar


 No call of destructor SvxFontNameBox_Impl::~SvxFontNameBox_Impl() for the
 instance 0x1cd4290

 ==

 I tried setting break point on
 framework::ToolBarManager::RemoveControllers and it calls SetItemWindow(id,
 0) for all items :
 m_pToolBar-SetItemWindow(nItemId, 0);

 But in ToolBox::SetItemWindow() method, I think the pItem-mpWindow is
 just overwritten and not getting deleted. Is the deletion code missing here
 ?
 Is it ok to add

  if (pItem-mpWindow)
 delete pItem-mpWindow;

 before assigning pNewWindow to it ?


1296 void ToolBox::SetItemWindow( sal_uInt16 nItemId, vcl::Window*
 pNewWindow )
1297 {
1298 sal_uInt16 nPos = GetItemPos( nItemId );
1299
1300 if ( nPos != TOOLBOX_ITEM_NOTFOUND )
1301 {
1302 ImplToolItem* pItem = mpData-m_aItems[nPos];
1303* pItem-mpWindow = pNewWindow;*
1304 if ( pNewWindow )
1305 pNewWindow-Hide();
1306 ImplInvalidate( true );
1307 CallEventListeners( VCLEVENT_TOOLBOX_ITEMWINDOWCHANGED,
 reinterpret_cast void* ( nPos ) );
1308 }
1309 }

 Thanks,
 Dennis

 http://www.ldcs.co.in

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

Re: Deletion of vcl::Window's inside ToolBar

[Dennis Francis]

Hi

After entering ToolBarManager::RemoveControllers, I put a break point on
SvxFontNameBox_Impl::~SvxFontNameBox_Impl and then did mpWindow.clear() in
gdb (where mpWindow is the SvxFontNameBox_Impl object wrapped in VclPtr).
But gdb did not show a call on the dtor ~SvxFontNameBox_Impl. After further
digging, I found that the mnRefCnt of the mpWindow in question was *219*
just before the VclPtr was reset.
Now I wonder who else increased the ref count of this mpWindow object, why
the value is so high and how to proceed from here ?

Thanks,
Dennis

On Wed, Aug 26, 2015 at 4:23 PM, Dennis Francis dennisfrancis...@gmail.com
wrote:

 Sorry, I understand now that pItem-mpWindow is a smart pointer (VclPtr).
 So the issue may be elsewhere.

 Thanks,
 Dennis

 On Wed, Aug 26, 2015 at 3:42 PM, Dennis Francis 
 dennisfrancis...@gmail.com wrote:

 Hi Caolán and Maxim

 Thanks for your replies, My reply is inline.

 On Tue, Aug 25, 2015 at 7:38 PM, Caolán McNamara caol...@redhat.com
 wrote:

 Digging into the other examples, I guess that SetItemWindow was called
 on the toolbar with the SvxFontNameBox_Impl window as an argument ?

 yes.


 If that's the case then the pattern appears to be that what object
 called SetItemWindow on the toolbar should call SetItemWindow(id, 0) to
 remove the window from the toolbar and then call disposeAndClear on the
 window.

 i.e. what entity put the item into the toolbar is the same entity that
 has to remove it and dispose it.

 What's the reproducing scenario here, is it the fontname widget in the
 standard toolbar, e.g. just start and exit writer to reproduce ?


 I think it should work with writer too, I ran soffice --calc and opened a
 new spreadsheet and then closed it.


 C.


 Here is what I observe in gdb :

 ===
 breakpoints were set on :
 SvxFontNameBox_Impl::SvxFontNameBox_Impl()
 SvxFontNameBox_Impl::FillList()
 SvxFontNameBox_Impl::~SvxFontNameBox_Impl()

 Order of calls observed from gdb each with address of SvxFontNameBox_Impl
 object :
 1. (SvxFontNameBox_Impl * const) 0x1cd4290,
 SvxFontNameBox_Impl::SvxFontNameBox_Impl - from toolbar code
 2. (SvxFontNameBox_Impl * const) 0x1cd4290, SvxFontNameBox_Impl::FillList
 3. (SvxFontNameBox_Impl * const) 0x1e60cc0,
 SvxFontNameBox_Impl::SvxFontNameBox_Impl  - from sidebar::ControllerFactory
 4. (SvxFontNameBox_Impl * const) 0x1e60cc0,
 SvxFontNameBox_Impl::~SvxFontNameBox_Impl - sidebar
 5. (SvxFontNameBox_Impl * const) 0x1e60cc0,
 SvxFontNameBox_Impl::~SvxFontNameBox_Impl - sidebar


 No call of destructor SvxFontNameBox_Impl::~SvxFontNameBox_Impl() for the
 instance 0x1cd4290

 ==

 I tried setting break point on
 framework::ToolBarManager::RemoveControllers and it calls SetItemWindow(id,
 0) for all items :
 m_pToolBar-SetItemWindow(nItemId, 0);

 But in ToolBox::SetItemWindow() method, I think the pItem-mpWindow is
 just overwritten and not getting deleted. Is the deletion code missing here
 ?
 Is it ok to add

  if (pItem-mpWindow)
 delete pItem-mpWindow;

 before assigning pNewWindow to it ?


1296 void ToolBox::SetItemWindow( sal_uInt16 nItemId, vcl::Window*
 pNewWindow )
1297 {
1298 sal_uInt16 nPos = GetItemPos( nItemId );
1299
1300 if ( nPos != TOOLBOX_ITEM_NOTFOUND )
1301 {
1302 ImplToolItem* pItem = mpData-m_aItems[nPos];
1303* pItem-mpWindow = pNewWindow;*
1304 if ( pNewWindow )
1305 pNewWindow-Hide();
1306 ImplInvalidate( true );
1307 CallEventListeners( VCLEVENT_TOOLBOX_ITEMWINDOWCHANGED,
 reinterpret_cast void* ( nPos ) );
1308 }
1309 }

 Thanks,
 Dennis

 http://www.ldcs.co.in



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

Re: Deletion of vcl::Window's inside ToolBar

[Michael Meeks]

Hi Dennis,

On Wed, 2015-08-26 at 17:59 +0530, Dennis Francis wrote:
 After entering ToolBarManager::RemoveControllers, I put a break point
 on SvxFontNameBox_Impl::~SvxFontNameBox_Impl and then did
 mpWindow.clear() in gdb (where mpWindow is the SvxFontNameBox_Impl
 object wrapped in VclPtr).

Interesting. Is it possible that there should be a disposeAndClear
there ?

 But gdb did not show a call on the dtor ~SvxFontNameBox_Impl. After
 further digging, I found that the mnRefCnt of the mpWindow in question
 was 219 just before the VclPtr was reset.

So - for a given un-disposed window - I'd expect lots of VclPtr
references - but its unusual to have that many.

 Now I wonder who else increased the ref count of this mpWindow object,
 why the value is so high and how to proceed from here ?

VCL itself maintains a whole slew of these - eg. the impl. of a window
points to the next window, the previous window, top-levels are
maintained in lists left  right etc. parents hold references to
children and vv. (depends how many children it has).

In order to (hopefully) break all these referencing cycles - it is
necessary to call 'disposeOnce' or 'disposeAndClear' on the VclPtr -
which stats the process of tidying up all the references.

I miss the context but reading vcl/README.lifecycle would
probably be helpful.

HTH,

Michael.

-- 
 michael.me...@collabora.com  , Pseudo Engineer, itinerant idiot

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

Re: Deletion of vcl::Window's inside ToolBar

[Maxim Monastirsky]

Hi,

On Tue, Aug 25, 2015 at 5:08 PM, Caolán McNamara caol...@redhat.com 
wrote:

i.e. what entity put the item into the toolbar is the same entity that
has to remove it and dispose it.

Just to add some specific code pointers:

The window items are added to the toolbar in 
ToolBarManager::CreateControllers, and removed in 
ToolBarManager::RemoveControllers. It also calls the dispose method of 
the (XToolbarController based) controller, which in turn calls 
disposeAndClear of that window. In case of the font name box, it's in 
SfxToolBoxControl::dispose.


Maxim

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

Re: Deletion of vcl::Window's inside ToolBar

[Caolán McNamara]

On Fri, 2015-08-21 at 23:51 +0530, Dennis Francis wrote:
 Hi all,
 
 I am trying to understand where the SvxFontNameBox_Impl objects are 
 deallocated. With help from 
 gdb and opengrok, I found that it is held in ImplToolItem::mpWindow. 
 But in ImplToolItem destructor it says clearly not to remove mpWindow 
 as a comment.

 
 ImplToolItem::~ImplToolItem() { 
 // don't dispose mpWindow - we get copied around.
 }

Digging into the other examples, I guess that SetItemWindow was called
on the toolbar with the SvxFontNameBox_Impl window as an argument ?

If that's the case then the pattern appears to be that what object
called SetItemWindow on the toolbar should call SetItemWindow(id, 0) to
remove the window from the toolbar and then call disposeAndClear on the
window.

i.e. what entity put the item into the toolbar is the same entity that
has to remove it and dispose it.

What's the reproducing scenario here, is it the fontname widget in the
standard toolbar, e.g. just start and exit writer to reproduce ?

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

Deletion of vcl::Window's inside ToolBar

[Dennis Francis]

Hi all,

I am trying to understand where the SvxFontNameBox_Impl objects are
deallocated. With help from
gdb and opengrok, I found that it is held in ImplToolItem::mpWindow. But in
ImplToolItem destructor it says clearly not to remove mpWindow as a comment.

ImplToolItem::~ImplToolItem() {
// don't dispose mpWindow - we get copied around.
}


Now I am stuck, as I cannot find any object that gets hold of mpWindow and
own it. It would be really helpful if somebody give me a hint on this.

Thanks,
Dennis

http://www.ldcs.co.in
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice