From 6829d4174ba57d752c8d470a55789112db477daf Mon Sep 17 00:00:00 2001 From: Alex Wilson Date: Fri, 30 Jul 2010 02:28:17 +0000 Subject: [PATCH] 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 | 4 +- headers/os/interface/GridLayout.h | 4 +- headers/os/interface/GroupLayout.h | 4 +- headers/os/interface/Layout.h | 6 +-- src/kits/interface/CardLayout.cpp | 7 ++-- src/kits/interface/GridLayout.cpp | 21 +++------- src/kits/interface/GroupLayout.cpp | 22 +++++------ src/kits/interface/Layout.cpp | 62 ++++++++++++++++++++---------- src/kits/interface/SplitLayout.cpp | 45 +++++++++++++--------- src/kits/interface/SplitLayout.h | 4 +- 10 files changed, 97 insertions(+), 82 deletions(-) diff --git a/headers/os/interface/CardLayout.h b/headers/os/interface/CardLayout.h index 422ed33c5a..7e2e70f828 100644 --- a/headers/os/interface/CardLayout.h +++ b/headers/os/interface/CardLayout.h @@ -36,8 +36,8 @@ public: static BArchivable* Instantiate(BMessage* from); protected: - virtual void ItemAdded(BLayoutItem* item); - virtual void ItemRemoved(BLayoutItem* item); + virtual bool ItemAdded(BLayoutItem* item, int32 atIndex); + virtual void ItemRemoved(BLayoutItem* item, int32 fromIndex); private: BSize fMin; diff --git a/headers/os/interface/GridLayout.h b/headers/os/interface/GridLayout.h index 4c4a2880c5..96a38f9000 100644 --- a/headers/os/interface/GridLayout.h +++ b/headers/os/interface/GridLayout.h @@ -63,8 +63,8 @@ public: BLayoutItem* item, int32 index); protected: - virtual void ItemAdded(BLayoutItem* item); - virtual void ItemRemoved(BLayoutItem* item); + virtual bool ItemAdded(BLayoutItem* item, int32 atIndex); + virtual void ItemRemoved(BLayoutItem* item, int32 fromIndex); virtual bool HasMultiColumnItems(); virtual bool HasMultiRowItems(); diff --git a/headers/os/interface/GroupLayout.h b/headers/os/interface/GroupLayout.h index c89712f14e..96123b3240 100644 --- a/headers/os/interface/GroupLayout.h +++ b/headers/os/interface/GroupLayout.h @@ -45,8 +45,8 @@ public: BLayoutItem* item, int32 index); protected: - virtual void ItemAdded(BLayoutItem* item); - virtual void ItemRemoved(BLayoutItem* item); + virtual bool ItemAdded(BLayoutItem* item, int32 atIndex); + virtual void ItemRemoved(BLayoutItem* item, int32 fromIndex); virtual void PrepareItems(enum orientation orientation); diff --git a/headers/os/interface/Layout.h b/headers/os/interface/Layout.h index dc488cdfbe..5a4f799ce3 100644 --- a/headers/os/interface/Layout.h +++ b/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 -// undo the addition, if false. - virtual void ItemAdded(BLayoutItem* item); - virtual void ItemRemoved(BLayoutItem* item); + virtual bool ItemAdded(BLayoutItem* item, int32 atIndex); + virtual void ItemRemoved(BLayoutItem* item, int32 fromIndex); private: friend class BView; diff --git a/src/kits/interface/CardLayout.cpp b/src/kits/interface/CardLayout.cpp index 46ed3386c8..e0eb5bcd21 100644 --- a/src/kits/interface/CardLayout.cpp +++ b/src/kits/interface/CardLayout.cpp @@ -234,15 +234,16 @@ BCardLayout::Instantiate(BMessage* from) } -void -BCardLayout::ItemAdded(BLayoutItem* item) +bool +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; diff --git a/src/kits/interface/GridLayout.cpp b/src/kits/interface/GridLayout.cpp index 0e2d1f5522..03d3bb8945 100644 --- a/src/kits/interface/GridLayout.cpp +++ b/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 -BGridLayout::ItemAdded(BLayoutItem* item) +bool +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; diff --git a/src/kits/interface/GroupLayout.cpp b/src/kits/interface/GroupLayout.cpp index e7c725131e..a9e2d5705b 100644 --- a/src/kits/interface/GroupLayout.cpp +++ b/src/kits/interface/GroupLayout.cpp @@ -13,6 +13,9 @@ #include +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 = - (BGroupLayout::ItemLayoutData*)item->LayoutData(); - - if (!data) // TODO: remove this once ItemAdded() returns a bool - return B_BAD_VALUE; - - return into->AddFloat(kItemWeightField, data->weight); + return into->AddFloat(kItemWeightField, _LayoutDataForItem(item)->weight); } @@ -237,15 +234,16 @@ BGroupLayout::ItemUnarchived(const BMessage* from, } -void -BGroupLayout::ItemAdded(BLayoutItem* item) +bool +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); diff --git a/src/kits/interface/Layout.cpp b/src/kits/interface/Layout.cpp index 639dad9ecb..d846732035 100644 --- a/src/kits/interface/Layout.cpp +++ b/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); - ItemAdded(item); - item->SetLayout(this); - InvalidateLayout(); - - return true; + if (fItems.AddItem(item, index) && ItemAdded(item, index)) { + item->SetLayout(this); + 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(); + 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)) { - ItemAdded(item); - item->SetLayout(this); - err = ItemUnarchived(from, item, i); - } else - err = B_NO_MEMORY; + if (!fItems.AddItem(item, i) || !ItemAdded(item, i)) { + fItems.RemoveItem(i); + return B_ERROR; } + + 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 -BLayout::ItemAdded(BLayoutItem* item) +bool +BLayout::ItemAdded(BLayoutItem* item, int32 atIndex) { + return true; } void -BLayout::ItemRemoved(BLayoutItem* item) +BLayout::ItemRemoved(BLayoutItem* item, int32 fromIndex) { } diff --git a/src/kits/interface/SplitLayout.cpp b/src/kits/interface/SplitLayout.cpp index 9279977c27..171aed8642 100644 --- a/src/kits/interface/SplitLayout.cpp +++ b/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(); - - if (!info) // TODO: remove this check when AddItem() returns a bool - return B_ERROR; + ItemLayoutInfo* info = _ItemLayoutInfo(item); status_t err = into->AddFloat(kItemWeightField, info->weight); if (err == B_OK) @@ -769,30 +769,43 @@ BSplitLayout::ItemUnarchived(const BMessage* from, } -void -BSplitLayout::ItemAdded(BLayoutItem* item) +bool +BSplitLayout::ItemAdded(BLayoutItem* item, int32 atIndex) { + ItemLayoutInfo* itemInfo = new(nothrow) ItemLayoutInfo(); + if (!itemInfo) + return false; + if (CountItems() > 1) { - SplitterItem* splitterItem = new SplitterItem(this); - SetItemWeight(splitterItem, 0); - fSplitterItems.AddItem(splitterItem); + SplitterItem* splitter = new(nothrow) SplitterItem(this); + ItemLayoutInfo* splitterInfo = new(nothrow) ItemLayoutInfo(); + 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(); - if (!info) { - info = new ItemLayoutInfo(); - item->SetLayoutData(info); - } - - return info; + return (ItemLayoutInfo*)item->LayoutData(); } diff --git a/src/kits/interface/SplitLayout.h b/src/kits/interface/SplitLayout.h index 84a6dc5c24..b8b2a50380 100644 --- a/src/kits/interface/SplitLayout.h +++ b/src/kits/interface/SplitLayout.h @@ -98,8 +98,8 @@ public: BLayoutItem* item, int32 index); protected: - virtual void ItemAdded(BLayoutItem* item); - virtual void ItemRemoved(BLayoutItem* item); + virtual bool ItemAdded(BLayoutItem* item, int32 atIndex); + virtual void ItemRemoved(BLayoutItem* item, int32 fromIndex); private: class ItemLayoutInfo;