Update BLayout::ItemAdded() and BLayout::ItemRemoved() to take an int32 index parameter, and ItemAdded() to return a bool indicating success or failure. Update BLayout::AddItem() to take the result of ItemAdded() into account. Also update BLayout::AllUnarchived() to take this into account. In either situation, if ItemAdded() returns false, the item does not get added to the layout. Removed various TODOs about code that could be removed once this change was made. In BSplitLayout, an item's LayoutData was lazy-initialized, but would always be created during ItemAdded(). Now we create the LayoutData explicitly, so that we can catch failed memory allocations. Closes #5524.

git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@37817 a95241bf-73f2-0310-859d-f6bbb57e9c96

headers/os/interface/CardLayout.h

@@ -36,8 +36,8 @@ public:
 	static	BArchivable*		Instantiate(BMessage* from);
 
 protected:
-	virtual	void				ItemAdded(BLayoutItem* item);
+	virtual	bool				ItemAdded(BLayoutItem* item, int32 atIndex);
-	virtual	void				ItemRemoved(BLayoutItem* item);
+	virtual	void				ItemRemoved(BLayoutItem* item, int32 fromIndex);
 
 private:
 			BSize				fMin;

headers/os/interface/GridLayout.h

@@ -63,8 +63,8 @@ public:
 									BLayoutItem* item, int32 index);
 
 protected:	
-	virtual	void				ItemAdded(BLayoutItem* item);
+	virtual	bool				ItemAdded(BLayoutItem* item, int32 atIndex);
-	virtual	void				ItemRemoved(BLayoutItem* item);
+	virtual	void				ItemRemoved(BLayoutItem* item, int32 fromIndex);
 
 	virtual	bool				HasMultiColumnItems();
 	virtual	bool				HasMultiRowItems();

headers/os/interface/GroupLayout.h

@@ -45,8 +45,8 @@ public:
 									BLayoutItem* item, int32 index);
 
 protected:	
-	virtual	void				ItemAdded(BLayoutItem* item);
+	virtual	bool				ItemAdded(BLayoutItem* item, int32 atIndex);
-	virtual	void				ItemRemoved(BLayoutItem* item);
+	virtual	void				ItemRemoved(BLayoutItem* item, int32 fromIndex);
 
 	virtual	void				PrepareItems(enum orientation orientation);
 	

headers/os/interface/Layout.h

@@ -60,10 +60,8 @@ public:
 	virtual	status_t			ItemUnarchived(const BMessage* from,
 									BLayoutItem* item, int32 index);
 protected:
-// TODO: Since memory allocations can fail, we should return a bool and
+	virtual	bool				ItemAdded(BLayoutItem* item, int32 atIndex);
-// undo the addition, if false.
+	virtual	void				ItemRemoved(BLayoutItem* item, int32 fromIndex);
-	virtual	void				ItemAdded(BLayoutItem* item);
-	virtual	void				ItemRemoved(BLayoutItem* item);
 
 private:
 			friend class BView;

src/kits/interface/CardLayout.cpp

@@ -234,15 +234,16 @@ BCardLayout::Instantiate(BMessage* from)
 }
 	
 
-void
+bool
-BCardLayout::ItemAdded(BLayoutItem* item)
+BCardLayout::ItemAdded(BLayoutItem* item, int32 atIndex)
 {
 	item->SetVisible(false);
+	return true;
 }
 
 
 void
-BCardLayout::ItemRemoved(BLayoutItem* item)
+BCardLayout::ItemRemoved(BLayoutItem* item, int32 fromIndex)
 {
 	if (fVisibleItem == item) {
 		BLayoutItem* newVisibleItem = NULL;

src/kits/interface/GridLayout.cpp

@@ -493,8 +493,6 @@ status_t
 BGridLayout::ItemArchived(BMessage* into, BLayoutItem* item, int32 index) const
 {
 	ItemLayoutData* data =	_LayoutDataForItem(item);
-	if (!data) // TODO: remove this check once AddItem() returns a bool
-		return B_ERROR;
 
 	status_t err = into->AddInt32(kItemDimensionsField, data->dimensions.x);
 	if (err == B_OK)
@@ -513,12 +511,8 @@ BGridLayout::ItemUnarchived(const BMessage* from,
 	BLayoutItem* item, int32 index)
 {
 	ItemLayoutData* data = _LayoutDataForItem(item);
-	if (!data) { // TODO: remove this check once AddItem() returns a bool
-		data = new ItemLayoutData();
-		item->SetLayoutData(data);
-	}
-
 	Dimensions& dimensions = data->dimensions;
+
 	index *= 4;
 		// each item stores 4 int32s into kItemDimensionsField
 	status_t err = from->FindInt32(kItemDimensionsField, index, &dimensions.x);
@@ -552,21 +546,18 @@ BGridLayout::ItemUnarchived(const BMessage* from,
 }
 
 
-void
+bool
-BGridLayout::ItemAdded(BLayoutItem* item)
+BGridLayout::ItemAdded(BLayoutItem* item, int32 atIndex)
 {
-	item->SetLayoutData(new ItemLayoutData);
+	item->SetLayoutData(new(nothrow) ItemLayoutData);
+	return item->LayoutData() != NULL;
 }
 
 
 void
-BGridLayout::ItemRemoved(BLayoutItem* item)
+BGridLayout::ItemRemoved(BLayoutItem* item, int32 fromIndex)
 {
 	ItemLayoutData* data = _LayoutDataForItem(item);
-// TODO: Once ItemAdded() returns a bool, we can remove this check.
-	if (!data)
-		return;
-
 	Dimensions itemDimensions = data->dimensions;
 	item->SetLayoutData(NULL);
 	delete data;

src/kits/interface/GroupLayout.cpp

@@ -13,6 +13,9 @@
 #include <new>
 
 
+using std::nothrow;
+
+
 namespace {
 	const char* const kItemWeightField = "BGroupLayout:item:weight";
 	const char* const kVerticalField = "BGroupLayout:vertical";
@@ -204,7 +207,7 @@ BArchivable*
 BGroupLayout::Instantiate(BMessage* from)
 {
 	if (validate_instantiation(from, "BGroupLayout"))
-		return new(std::nothrow) BGroupLayout(from);
+		return new(nothrow) BGroupLayout(from);
 	return NULL;
 }
 
@@ -213,13 +216,7 @@ status_t
 BGroupLayout::ItemArchived(BMessage* into,
 	BLayoutItem* item, int32 index) const
 {
-	BGroupLayout::ItemLayoutData* data =
+	return into->AddFloat(kItemWeightField, _LayoutDataForItem(item)->weight);
-		(BGroupLayout::ItemLayoutData*)item->LayoutData();
-
-	if (!data) // TODO: remove this once ItemAdded() returns a bool
-		return B_BAD_VALUE;
-
-	return into->AddFloat(kItemWeightField, data->weight);
 }
 
 
@@ -237,15 +234,16 @@ BGroupLayout::ItemUnarchived(const BMessage* from,
 }
 
 
-void
+bool
-BGroupLayout::ItemAdded(BLayoutItem* item)
+BGroupLayout::ItemAdded(BLayoutItem* item, int32 atIndex)
 {
-	item->SetLayoutData(new(std::nothrow) ItemLayoutData);
+	item->SetLayoutData(new(nothrow) ItemLayoutData);
+	return item->LayoutData() != NULL;
 }
 
 
 void
-BGroupLayout::ItemRemoved(BLayoutItem* item)
+BGroupLayout::ItemRemoved(BLayoutItem* item, int32 fromIndex)
 {
 	if (ItemLayoutData* data = _LayoutDataForItem(item)) {
 		item->SetLayoutData(NULL);

src/kits/interface/Layout.cpp

@@ -18,6 +18,7 @@
 
 using std::nothrow;
 
+
 namespace {
 	const char* const kLayoutItemField = "BLayout:items";
 }
@@ -93,20 +94,29 @@ BLayout::AddItem(int32 index, BLayoutItem* item)
 
 	// if the item refers to a BView, we make sure it is added to the parent
 	// view
+	bool addedView = false;
 	BView* view = item->View();
-	if (view && view->fParent != fView && !fView->_AddChild(view, NULL))
+	if (view && view->fParent != fView
+		&& !(addedView = fView->_AddChild(view, NULL)))
 		return false;
 
 	// validate the index
 	if (index < 0 || index > fItems.CountItems())
 		index = fItems.CountItems();
 
-	fItems.AddItem(item, index);
+	if (fItems.AddItem(item, index) && ItemAdded(item, index)) {
-	ItemAdded(item);
+		item->SetLayout(this);
-	item->SetLayout(this);
+		InvalidateLayout();
-	InvalidateLayout();
+		return true;
-
+	} else {
-	return true;
+		// this check is necessary so that if an addition somewhere other
+		// than the end of the list fails, we don't remove the wrong item
+		if (fItems.ItemAt(index) == item)
+			fItems.RemoveItem(index);
+		if (addedView)
+			view->_RemoveSelf();
+		return false;
+	}
 }
 
 
@@ -154,7 +164,7 @@ BLayout::RemoveItem(int32 index)
 		view->_RemoveSelf();
 
 	item->SetLayout(NULL);
-	ItemRemoved(item);
+	ItemRemoved(item, index);
 	InvalidateLayout();
 
 	return item;
@@ -233,7 +243,6 @@ BLayout::AllUnarchived(const BMessage* from)
 {
 	BUnarchiver unarchiver(from);
 	status_t err = BArchivable::AllUnarchived(from);
-
 	if (err != B_OK)
 		return err;
 
@@ -241,24 +250,34 @@ BLayout::AllUnarchived(const BMessage* from)
 	unarchiver.ArchiveMessage()->GetInfo(kLayoutItemField, NULL, &itemCount);
 	for (int32 i = 0; i < itemCount && err == B_OK; i++) {
 		BLayoutItem* item;
-		err = unarchiver.FindObject(kLayoutItemField, i, item);
+		err = unarchiver.FindObject(kLayoutItemField,
+			i, BUnarchiver::B_DONT_ASSUME_OWNERSHIP, item);
+		if (err != B_OK)
+			return err;
 
-		if (err == B_OK && item) {
+		if (!fItems.AddItem(item, i) || !ItemAdded(item, i)) {
-			if (fItems.AddItem(item)) {
+			fItems.RemoveItem(i);
-				ItemAdded(item);
+			return B_ERROR;
-				item->SetLayout(this);
-				err = ItemUnarchived(from, item, i);
-			} else
-				err = B_NO_MEMORY;
 		}
+
+		err = ItemUnarchived(from, item, i);
+		if (err != B_OK) {
+			fItems.RemoveItem(i);
+			ItemRemoved(item, i);	
+			return err;
+		}
+
+		item->SetLayout(this);
+		unarchiver.AssumeOwnership(item);
 	}
 
+	InvalidateLayout();
 	return err;
 }
 
 
 status_t
-BLayout::ItemArchived(BMessage* into, BLayoutItem* of, int32 index) const
+BLayout::ItemArchived(BMessage* into, BLayoutItem* item, int32 index) const
 {
 	return B_OK;
 }
@@ -271,14 +290,15 @@ BLayout::ItemUnarchived(const BMessage* from, BLayoutItem* item, int32 index)
 }
 
 
-void
+bool
-BLayout::ItemAdded(BLayoutItem* item)
+BLayout::ItemAdded(BLayoutItem* item, int32 atIndex)
 {
+	return true;
 }
 
 
 void
-BLayout::ItemRemoved(BLayoutItem* item)
+BLayout::ItemRemoved(BLayoutItem* item, int32 fromIndex)
 {
 }
 

src/kits/interface/SplitLayout.cpp

@@ -18,6 +18,9 @@
 #include "SimpleLayouter.h"
 
 
+using std::nothrow;
+
+
 // archivng constants
 namespace {
 	const char* const kItemCollapsibleField = "BSplitLayout:item:collapsible";
@@ -741,10 +744,7 @@ BSplitLayout::Instantiate(BMessage* from)
 status_t
 BSplitLayout::ItemArchived(BMessage* into, BLayoutItem* item, int32 index) const
 {
-	ItemLayoutInfo* info = (ItemLayoutInfo*)item->LayoutData();
+	ItemLayoutInfo* info = _ItemLayoutInfo(item);
-
-	if (!info) // TODO: remove this check when AddItem() returns a bool
-		return B_ERROR;
 
 	status_t err = into->AddFloat(kItemWeightField, info->weight);
 	if (err == B_OK)
@@ -769,30 +769,43 @@ BSplitLayout::ItemUnarchived(const BMessage* from,
 }
 
 
-void
+bool
-BSplitLayout::ItemAdded(BLayoutItem* item)
+BSplitLayout::ItemAdded(BLayoutItem* item, int32 atIndex)
 {
+	ItemLayoutInfo* itemInfo = new(nothrow) ItemLayoutInfo();
+	if (!itemInfo)
+		return false;
+
 	if (CountItems() > 1) {
-		SplitterItem* splitterItem = new SplitterItem(this);
+		SplitterItem* splitter = new(nothrow) SplitterItem(this);
-		SetItemWeight(splitterItem, 0);
+		ItemLayoutInfo* splitterInfo = new(nothrow) ItemLayoutInfo();
-		fSplitterItems.AddItem(splitterItem);
+		if (!splitter || !splitterInfo || !fSplitterItems.AddItem(splitter)) {
+			delete itemInfo;
+			delete splitter;
+			delete splitterInfo;
+			return false;
+		}
+		splitter->SetLayoutData(splitterInfo);
+		SetItemWeight(splitter, 0);
 	}
 
+	item->SetLayoutData(itemInfo);
 	SetItemWeight(item, 1);
+	return true;
 }
 
 
 void
-BSplitLayout::ItemRemoved(BLayoutItem* item)
+BSplitLayout::ItemRemoved(BLayoutItem* item, int32 atIndex)
 {
 	if (fSplitterItems.CountItems() > 0) {
 		SplitterItem* splitterItem = (SplitterItem*)fSplitterItems.RemoveItem(
 			fSplitterItems.CountItems() - 1);
-		delete (ItemLayoutInfo*)splitterItem->LayoutData();
+		delete _ItemLayoutInfo(splitterItem);
 		delete splitterItem;
 	}
 
-	delete (ItemLayoutInfo*)item->LayoutData();
+	delete _ItemLayoutInfo(item);
 	item->SetLayoutData(NULL);
 }
 
@@ -1184,13 +1197,7 @@ BSplitLayout::_SetSplitterValue(int32 index, int32 value)
 BSplitLayout::ItemLayoutInfo*
 BSplitLayout::_ItemLayoutInfo(BLayoutItem* item) const
 {
-	ItemLayoutInfo* info = (ItemLayoutInfo*)item->LayoutData();
+	return (ItemLayoutInfo*)item->LayoutData();
-	if (!info) {
-		info = new ItemLayoutInfo();
-		item->SetLayoutData(info);
-	}
-
-	return info;
 }
 
 

src/kits/interface/SplitLayout.h

@@ -98,8 +98,8 @@ public:
 									BLayoutItem* item, int32 index);
 
 protected:
-	virtual	void				ItemAdded(BLayoutItem* item);
+	virtual	bool				ItemAdded(BLayoutItem* item, int32 atIndex);
-	virtual	void				ItemRemoved(BLayoutItem* item);
+	virtual	void				ItemRemoved(BLayoutItem* item, int32 fromIndex);
 
 private:
 			class ItemLayoutInfo;