Fix a memory leak I introduced with my modification of BLayout::RemoveItem() and add comments so that it doesn't happen again. Also resolve some TODOs on the appropriate timing of calls to hook methods in BLayoutItem/BLayout.
This commit is contained in:
@@ -226,19 +226,17 @@ BLayout::RemoveItem(int32 index)
|
|||||||
return NULL;
|
return NULL;
|
||||||
|
|
||||||
BLayoutItem* item = (BLayoutItem*)fItems.RemoveItem(index);
|
BLayoutItem* item = (BLayoutItem*)fItems.RemoveItem(index);
|
||||||
// If this is the last item in use that refers to a certain BView,
|
|
||||||
// that BView now needs to be removed.
|
|
||||||
BView* view = item->View();
|
|
||||||
if (view && BView::Private(view).CountLayoutItems() == 0)
|
|
||||||
view->_RemoveSelf();
|
|
||||||
|
|
||||||
// TODO: Is this the right place/order to call these hooks?
|
|
||||||
// view->Parent() could be NULL, maybe that's not a problem..
|
|
||||||
|
|
||||||
ItemRemoved(item, index);
|
ItemRemoved(item, index);
|
||||||
item->SetLayout(NULL);
|
item->SetLayout(NULL);
|
||||||
InvalidateLayout();
|
|
||||||
|
|
||||||
|
// If this is the last item in use that refers to its BView,
|
||||||
|
// that BView now needs to be removed. UNLESS fTarget is NULL,
|
||||||
|
// in which case we leave the view as is. (See SetTarget() for more info)
|
||||||
|
BView* view = item->View();
|
||||||
|
if (fTarget && view && BView::Private(view).CountLayoutItems() == 0)
|
||||||
|
view->_RemoveSelf();
|
||||||
|
|
||||||
|
InvalidateLayout();
|
||||||
return item;
|
return item;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -624,8 +622,10 @@ void
|
|||||||
BLayout::SetTarget(BView* target)
|
BLayout::SetTarget(BView* target)
|
||||||
{
|
{
|
||||||
if (fTarget != target) {
|
if (fTarget != target) {
|
||||||
|
/* With fTarget NULL, RemoveItem() will not remove the views from their
|
||||||
|
* parent. This ensures that the views are not lost to the void.
|
||||||
|
*/
|
||||||
fTarget = NULL;
|
fTarget = NULL;
|
||||||
// only remove items, not views
|
|
||||||
|
|
||||||
// remove and delete all items
|
// remove and delete all items
|
||||||
for (int32 i = CountItems() - 1; i >= 0; i--)
|
for (int32 i = CountItems() - 1; i >= 0; i--)
|
||||||
|
|||||||
@@ -164,17 +164,16 @@ BLayoutItem::SetLayout(BLayout* layout)
|
|||||||
if (layout == fLayout)
|
if (layout == fLayout)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
std::swap(fLayout, layout);
|
BLayout* oldLayout = fLayout;
|
||||||
if (layout)
|
fLayout = layout;
|
||||||
|
|
||||||
|
if (oldLayout)
|
||||||
DetachedFromLayout(layout);
|
DetachedFromLayout(layout);
|
||||||
|
|
||||||
// TODO: is this the right place to do this?
|
|
||||||
// at this point, this->Layout() will return not exactly truthful
|
|
||||||
// values... this could cause problems in DetachedFromLayout();
|
|
||||||
if (BView* view = View()) {
|
if (BView* view = View()) {
|
||||||
if (layout && !fLayout) {
|
if (oldLayout && !fLayout) {
|
||||||
BView::Private(view).DeregisterLayoutItem(this);
|
BView::Private(view).DeregisterLayoutItem(this);
|
||||||
} else if (fLayout && !layout) {
|
} else if (fLayout && !oldLayout) {
|
||||||
BView::Private(view).RegisterLayoutItem(this);
|
BView::Private(view).RegisterLayoutItem(this);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user