Aren/haiku
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

BView: Fix destruction order of layout items.

Browse Source 
Because of the virtual hooks a BLayout must never be destroyed while it
still has layout items. If these items are only removed from the layout
in its destructor, the subclass version of hooks like ItemRemoved() are
not called anymore. This lead to leaks because many BLayout subclasses
use the ItemRemoved() hook to clean up associated data (as is suggested
explicitly in the BLayout documentation).

In the same line of thought, a BLayoutItem must never be deleted when it
is still attached to a layout, as it similarly has virtual hooks like
DetachedFromLayout() that can not be called at this point anymore.

The destructors of BLayout and BLayoutItem now have debugger calls in
case these conditions are not met which should help to avoid
accidentally introducing such hard to debug issues.

To ensure the correct destruction order the sequence is now:

* Destroy the child views first. This cleans up their layout items while
  the layout tree is still intact.
* Unset the view layout before removing layout items so it can properly
  detach from the layout instead of just deleting it.
This commit is contained in:
Michael Lotz 2015-04-14 23:54:14 +02:00
parent 0cc8c71bba
commit e837ee8bc6
3 changed files with 15 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/kits/interface/Layout.cpp
View File

@ -94,9 +94,10 @@ BLayout::~BLayout()
if (fOwner && this == fOwner->GetLayout())
fOwner->_LayoutLeft(this);
// removes and deletes all items
if (fTarget)
SetTarget(NULL);
if (CountItems() > 0) {
debugger("Deleting a BLayout that still has items. Subclass hooks "
"will not be called");
}
}

6
src/kits/interface/LayoutItem.cpp
View File

@ -35,8 +35,10 @@ BLayoutItem::BLayoutItem(BMessage* from)
BLayoutItem::~BLayoutItem()
{
if (fLayout)
fLayout->RemoveItem(this);
if (fLayout != NULL) {
debugger("Deleting a BLayoutItem that is still attached to a layout. "
"Call RemoveSelf first.");
}
}

13
src/kits/interface/View.cpp
View File

@ -704,9 +704,6 @@ BView::~BView()
"Call RemoveSelf first.");
}
_RemoveLayoutItemsFromLayout(true);
_RemoveSelf();
if (fToolTip != NULL)
fToolTip->ReleaseReference();
@ -720,10 +717,13 @@ BView::~BView()
child = nextChild;
}
// delete the layout and the layout data
delete fLayoutData->fLayout;
SetLayout(NULL);
_RemoveLayoutItemsFromLayout(true);
delete fLayoutData;
_RemoveSelf();
if (fVerScroller)
fVerScroller->SetTarget((BView*)NULL);
if (fHorScroller)
@ -4198,7 +4198,7 @@ BView::_RemoveLayoutItemsFromLayout(bool deleteItems)
int32 index = fLayoutData->fLayoutItems.CountItems();
while (index-- > 0) {
BLayoutItem* item = fLayoutData->fLayoutItems.ItemAt(index);
item->Layout()->RemoveItem(item);
item->RemoveSelf();
// Removes item from fLayoutItems list
if (deleteItems)
delete item;
@ -4802,6 +4802,7 @@ BView::SetLayout(BLayout* layout)
// unset and delete the old layout
if (fLayoutData->fLayout) {
fLayoutData->fLayout->RemoveSelf();
fLayoutData->fLayout->SetOwner(NULL);
delete fLayoutData->fLayout;
}