From edb4c8244cfe8ed3801ef4854d45c03324c96510 Mon Sep 17 00:00:00 2001
From: Alex Wilson <yourpalal2@gmail.com>
Date: Tue, 6 Sep 2011 14:03:49 -0600
Subject: [PATCH] Add a method to BView::Private that calls
 BView::_RemoveSelf(). Add a struct to kits/interface/Layout.cpp that uses
 BView::Private to provide RAII mechanics for the removal of views (calling
 BView::Private::RemoveSelf()). Use the new struct to simplify
 BLayout::AddItem().

---
 headers/private/interface/ViewPrivate.h |  5 +++
 src/kits/interface/Layout.cpp           | 57 ++++++++++++++++++-------
 2 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/headers/private/interface/ViewPrivate.h b/headers/private/interface/ViewPrivate.h
index d81a8c2a6a..27474a3152 100644
--- a/headers/private/interface/ViewPrivate.h
+++ b/headers/private/interface/ViewPrivate.h
@@ -66,6 +66,11 @@ public:
 	bool	WillLayout();
 	bool	MinMaxValid();
 
+	bool RemoveSelf()
+	{
+		return fView->_RemoveSelf();
+	}
+
 	BView* fView;
 };
 
diff --git a/src/kits/interface/Layout.cpp b/src/kits/interface/Layout.cpp
index ad3a2bf226..a4e974cb24 100644
--- a/src/kits/interface/Layout.cpp
+++ b/src/kits/interface/Layout.cpp
@@ -40,6 +40,31 @@ namespace {
 		= B_LAYOUT_INVALID | B_LAYOUT_IN_PROGRESS;
 
 	const char* const kLayoutItemField = "BLayout:items";
+
+
+	struct ViewRemover {
+		ViewRemover(BView* view) : fView(view) {};
+
+		~ViewRemover()
+		{
+			if (fView)
+				BView::Private(fView).RemoveSelf();
+		}
+
+		void SetTo(BView* view)
+		{
+			if (fView)
+				BView::Private(fView).RemoveSelf();
+			fView = view;
+		}
+
+		void Detach()
+		{
+			fView = NULL;
+		}
+
+		BView* fView;
+	};
 }
 
 
@@ -148,31 +173,31 @@ 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 != fTarget
-		&& !(addedView = fTarget->_AddChild(view, NULL)))
+	ViewRemover remover(view);
+	if (view && view->fParent != fTarget && !fTarget->_AddChild(view, NULL)) {
+		remover.Detach(); // view wasn't added
 		return false;
+	}
 
 	// validate the index
 	if (index < 0 || index > fItems.CountItems())
 		index = fItems.CountItems();
 
-	if (fItems.AddItem(item, index) && ItemAdded(item, index)) {
-		item->SetLayout(this);
-		if (!fAncestorsVisible)
-			item->AncestorVisibilityChanged(fAncestorsVisible);
-		InvalidateLayout();
-		return true;
-	} else {
-		// 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();
+	if (!fItems.AddItem(item, index))
+		return false;
+
+	if (!ItemAdded(item, index)) {
+		fItems.RemoveItem(index);
 		return false;
 	}
+
+	item->SetLayout(this);
+	if (!fAncestorsVisible)
+		item->AncestorVisibilityChanged(fAncestorsVisible);
+	InvalidateLayout();
+	remover.Detach();
+	return true;
 }