From ff1ade6b3d1195d3065cd2ecd1b6064e06f9e17f Mon Sep 17 00:00:00 2001 From: ejakowatz Date: Thu, 22 Aug 2002 03:43:06 +0000 Subject: [PATCH] More tests and fixes for bugs exposed by them. Also removed spurious instantiation of BRoster from BArchivable, line 323, as per Tyler's mention. =) git-svn-id: file:///srv/svn/repos/haiku/trunk/current@852 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/kits/app/Looper.cpp | 162 ++++++++++++++++-- src/kits/app/LooperList.cpp | 8 +- src/kits/support/Archivable.cpp | 1 - src/tests/kits/app/Jamfile | 1 + src/tests/kits/app/blooper/LooperTest.cpp | 2 + .../app/blooper/SetCommonFilterListTest.cpp | 102 +++++++++++ .../app/blooper/SetCommonFilterListTest.h | 44 +++++ 7 files changed, 301 insertions(+), 19 deletions(-) create mode 100644 src/tests/kits/app/blooper/SetCommonFilterListTest.cpp create mode 100644 src/tests/kits/app/blooper/SetCommonFilterListTest.h diff --git a/src/kits/app/Looper.cpp b/src/kits/app/Looper.cpp index 49290e4b6e..4e869cd0c4 100644 --- a/src/kits/app/Looper.cpp +++ b/src/kits/app/Looper.cpp @@ -137,10 +137,35 @@ BLooper::~BLooper() Lock(); -// TODO: delete fLastMessage; -// In case the looper thread calls Quit() fLast message is not deleted. + // In case the looper thread calls Quit() fLastMessage is not deleted. + if (fLastMessage) + { + delete fLastMessage; + fLastMessage = NULL; + } -// TODO: Close the message port and read and reply to the remaining messages. + // Close the message port and read and reply to the remaining messages. + if (fMsgPort > 0) + { + close_port(fMsgPort); + } + + BMessage* msg; + // Clear the queue so our call to IsMessageWaiting() below doesn't give + // us bogus info + while ((msg = fQueue->NextMessage())) + { + delete msg; // msg will automagically post generic reply + } + + do + { + msg = ReadMessageFromPort(0); + if (msg) + { + delete msg; // msg will automagically post generic reply + } + } while (IsMessageWaiting()); // bonefish: Killing the looper thread doesn't work very well with // BApplication. In fact here it doesn't work too well either. When the @@ -157,6 +182,18 @@ BLooper::~BLooper() BObjectLocker ListLock(gLooperList); RemoveHandler(this); + + // Remove all the "child" handlers + BHandler* child; + while (CountHandlers()) + { + child = HandlerAt(0); + if (child) + { + RemoveHandler(child); + } + } + RemoveLooper(this); UnlockFully(); @@ -645,7 +682,7 @@ team_id BLooper::Team() const //------------------------------------------------------------------------------ BLooper* BLooper::LooperForThread(thread_id tid) { - BObjectLocker ListLock (gLooperList); + BObjectLocker ListLock(gLooperList); if (ListLock.IsLocked()) { return gLooperList.LooperForThread(tid); @@ -809,13 +846,28 @@ bool BLooper::RemoveCommonFilter(BMessageFilter* filter) //------------------------------------------------------------------------------ void BLooper::SetCommonFilterList(BList* filters) { + // We have a somewhat serious problem here. It is entirely possible in R5 + // to assign a given list of filters to *two* BLoopers simultaneously. This + // becomes problematic when the loopers are destroyed: the last looper + // destroyed will have a problem when it tries to delete a filter list that + // has already been deleted. In R5, this results in a general protection + // fault; here it ends in a segment violation. + if (!IsLocked()) + { + debugger("Owning Looper must be locked before calling " + "SetCommonFilterList"); + return; + } + if (fCommonFilters) { for (int32 i = 0; i < fCommonFilters->CountItems(); ++i) { delete fCommonFilters->ItemAt(i); } - fCommonFilters->MakeEmpty(); + + delete fCommonFilters; + fCommonFilters = NULL; } // Per the BeBook, we take ownership of the list @@ -1388,23 +1440,105 @@ bool BLooper::AssertLocked() const return true; } //------------------------------------------------------------------------------ -BHandler* BLooper::top_level_filter(BMessage* msg, BHandler* t) +BHandler* BLooper::top_level_filter(BMessage* msg, BHandler* target) { - // TODO: implement - // return the supplied handler for now - return t; + if (msg) + { + // Apply the common filters first + target = apply_filters(CommonFilterList(), msg, target); + if (target) + { + if (target->Looper() != this) + { + // TODO: debugger message? + target = NULL; + } + else + { + // Now apply handler-specific filters + target = handler_only_filter(msg, target); + } + } + } + + return target; } //------------------------------------------------------------------------------ -BHandler* BLooper::handler_only_filter(BMessage* msg, BHandler* t) +BHandler* BLooper::handler_only_filter(BMessage* msg, BHandler* target) { - // TODO: implement - return NULL; + // Keep running filters until our handler is NULL, or until the filtering + // handler returns itself as the designated handler + BHandler* oldTarget = NULL; + while (target && (target != oldTarget)) + { + oldTarget = target; + target = apply_filters(oldTarget->FilterList(), msg, oldTarget); + if (target && (target->Looper() != this)) + { + // TODO: debugger message? + target = NULL; + } + } + + return target; } //------------------------------------------------------------------------------ BHandler* BLooper::apply_filters(BList* list, BMessage* msg, BHandler* target) { - // TODO: implement - return NULL; + // This is where the action is! + // Check the parameters + if (!list || !msg) + { + return target; + } + + // For each filter in the provided list + BMessageFilter* filter = NULL; + for (int32 i = 0; i < list->CountItems(); ++i) + { + filter = (BMessageFilter*)list->ItemAt(i); + + // Check command conditions + if (filter->FiltersAnyCommand() || (filter->Command() == msg->what)) + { + // Check delivery conditions + message_delivery delivery = filter->MessageDelivery(); + bool dropped = msg->WasDropped(); + if (delivery == B_ANY_DELIVERY || + ((delivery == B_DROPPED_DELIVERY) && dropped) || + ((delivery == B_PROGRAMMED_DELIVERY) && !dropped)) + { + // Check source conditions + message_source source = filter->MessageSource(); + bool remote = msg->IsSourceRemote(); + if (source == B_ANY_SOURCE || + ((source == B_REMOTE_SOURCE) && remote) || + ((source == B_LOCAL_SOURCE) && !remote)) + { + filter_result result; + // Are we using an "external" function? + filter_hook func = filter->FilterFunction(); + if (func) + { + result = func(msg, &target, filter); + } + else + { + result = filter->Filter(msg, &target); + } + + // Is further processing allowed? + if (result == B_SKIP_MESSAGE) + { + // No; time to bail out + return NULL; + } + } + } + } + } + + return target; } //------------------------------------------------------------------------------ void BLooper::check_lock() diff --git a/src/kits/app/LooperList.cpp b/src/kits/app/LooperList.cpp index 06cfe8548b..ee4ec4b3d9 100644 --- a/src/kits/app/LooperList.cpp +++ b/src/kits/app/LooperList.cpp @@ -241,22 +241,22 @@ BLooperList::LooperData::operator=(const LooperData& rhs) //------------------------------------------------------------------------------ bool BLooperList::FindLooperPred::operator()(BLooperList::LooperData& Data) { - return looper == Data.looper; + return Data.looper && (looper == Data.looper); } //------------------------------------------------------------------------------ bool BLooperList::FindThreadPred::operator()(LooperData& Data) { - return thread == Data.looper->Thread(); + return Data.looper && (thread == Data.looper->Thread()); } //------------------------------------------------------------------------------ bool BLooperList::FindNamePred::operator()(LooperData& Data) { - return strcmp(name, Data.looper->Name()) == 0; + return Data.looper && (strcmp(name, Data.looper->Name()) == 0); } //------------------------------------------------------------------------------ bool BLooperList::FindPortPred::operator()(LooperData& Data) { - return port == _get_looper_port_(Data.looper); + return Data.looper && (port == _get_looper_port_(Data.looper)); } //------------------------------------------------------------------------------ diff --git a/src/kits/support/Archivable.cpp b/src/kits/support/Archivable.cpp index 4b108cde9e..1804404130 100644 --- a/src/kits/support/Archivable.cpp +++ b/src/kits/support/Archivable.cpp @@ -320,7 +320,6 @@ instantiation_func find_instantiation_func(const char* class_name, return NULL; } - BRoster Roster; instantiation_func theFunc = NULL; BString funcName; diff --git a/src/tests/kits/app/Jamfile b/src/tests/kits/app/Jamfile index e45ffde25b..1a83f2d4ee 100644 --- a/src/tests/kits/app/Jamfile +++ b/src/tests/kits/app/Jamfile @@ -49,6 +49,7 @@ CommonTestLib libapptest.so AddCommonFilterTest.cpp RemoveCommonFilterTest.cpp LooperSizeTest.cpp + SetCommonFilterListTest.cpp # BMessageQueue MessageQueueTest.cpp diff --git a/src/tests/kits/app/blooper/LooperTest.cpp b/src/tests/kits/app/blooper/LooperTest.cpp index 3fa1c83c5a..14efe2b0b0 100644 --- a/src/tests/kits/app/blooper/LooperTest.cpp +++ b/src/tests/kits/app/blooper/LooperTest.cpp @@ -12,6 +12,7 @@ #include "AddCommonFilterTest.h" #include "RemoveCommonFilterTest.h" #include "LooperSizeTest.h" +#include "SetCommonFilterListTest.h" Test* LooperTestSuite() { @@ -29,6 +30,7 @@ Test* LooperTestSuite() tests->addTest(TAddCommonFilterTest::Suite()); tests->addTest(TRemoveCommonFilterTest::Suite()); tests->addTest(TLooperSizeTest::Suite()); + tests->addTest(TSetCommonFilterListTest::Suite()); return tests; } diff --git a/src/tests/kits/app/blooper/SetCommonFilterListTest.cpp b/src/tests/kits/app/blooper/SetCommonFilterListTest.cpp new file mode 100644 index 0000000000..931bc242fa --- /dev/null +++ b/src/tests/kits/app/blooper/SetCommonFilterListTest.cpp @@ -0,0 +1,102 @@ +//------------------------------------------------------------------------------ +// SetCommonFilterListTest.cpp +// +//------------------------------------------------------------------------------ + +// Standard Includes ----------------------------------------------------------- + +// System Includes ------------------------------------------------------------- +#include +#include +#include + +// Project Includes ------------------------------------------------------------ + +// Local Includes -------------------------------------------------------------- +#include "SetCommonFilterListTest.h" + +// Local Defines --------------------------------------------------------------- + +// Globals --------------------------------------------------------------------- + +//------------------------------------------------------------------------------ +/** + SetCommonFilterList(BList* filters) + case NULL list + */ +void TSetCommonFilterListTest::SetCommonFilterListTest1() +{ + BLooper Looper; + Looper.SetCommonFilterList(NULL); + CPPUNIT_ASSERT(Looper.CommonFilterList() == NULL); +} +//------------------------------------------------------------------------------ +/** + SetCommonFilterList(BList* filters) + case Valid list, looper not locked + */ +void TSetCommonFilterListTest::SetCommonFilterListTest2() +{ + BLooper Looper; + BList* filters = new BList; + filters->AddItem(new BMessageFilter('1234')); + Looper.Unlock(); + Looper.SetCommonFilterList(filters); + CPPUNIT_ASSERT(Looper.CommonFilterList() == NULL); +} +//------------------------------------------------------------------------------ +/** + SetCommonFilterList(BList* filters) + case Valid list, looper locked + */ +void TSetCommonFilterListTest::SetCommonFilterListTest3() +{ + BLooper Looper; + BList* filters = new BList; + filters->AddItem(new BMessageFilter('1234')); + Looper.SetCommonFilterList(filters); + CPPUNIT_ASSERT(Looper.CommonFilterList() == filters); +} +//------------------------------------------------------------------------------ +/** + SetCommonFilterList(BList* filters) + case Valid list, looper locked, owned by another looper + */ +void TSetCommonFilterListTest::SetCommonFilterListTest4() +{ + DEBUGGER_ESCAPE; + BLooper Looper1; + BLooper Looper2; + BList* filters = new BList; + filters->AddItem(new BMessageFilter('1234')); + Looper1.SetCommonFilterList(filters); + Looper2.SetCommonFilterList(filters); + CPPUNIT_ASSERT(Looper1.CommonFilterList() == filters); + CPPUNIT_ASSERT(Looper2.CommonFilterList() == filters); +} +//------------------------------------------------------------------------------ +#ifdef ADD_TEST +#undef ADD_TEST +#endif +#define ADD_TEST(__test_name__) \ + ADD_TEST4(BLooper, suite, TSetCommonFilterListTest, __test_name__) +TestSuite* TSetCommonFilterListTest::Suite() +{ + TestSuite* suite = new TestSuite("BLooper::SetCommonFilterList(BList*)"); + + ADD_TEST(SetCommonFilterListTest1); + ADD_TEST(SetCommonFilterListTest2); + ADD_TEST(SetCommonFilterListTest3); + ADD_TEST(SetCommonFilterListTest4); + + return suite; +} +//------------------------------------------------------------------------------ + +/* + * $Log $ + * + * $Id $ + * + */ + diff --git a/src/tests/kits/app/blooper/SetCommonFilterListTest.h b/src/tests/kits/app/blooper/SetCommonFilterListTest.h new file mode 100644 index 0000000000..411a20784f --- /dev/null +++ b/src/tests/kits/app/blooper/SetCommonFilterListTest.h @@ -0,0 +1,44 @@ +//------------------------------------------------------------------------------ +// SetCommonFilterTest.h +// +//------------------------------------------------------------------------------ + +#ifndef SETCOMMONFILTERTEST_H +#define SETCOMMONFILTERTEST_H + +// Standard Includes ----------------------------------------------------------- + +// System Includes ------------------------------------------------------------- + +// Project Includes ------------------------------------------------------------ + +// Local Includes -------------------------------------------------------------- +#include "../common.h" + +// Local Defines --------------------------------------------------------------- + +// Globals --------------------------------------------------------------------- + +class TSetCommonFilterListTest : public TestCase +{ + public: + TSetCommonFilterListTest() {;} + TSetCommonFilterListTest(std::string name) : TestCase(name) {;} + + void SetCommonFilterListTest1(); + void SetCommonFilterListTest2(); + void SetCommonFilterListTest3(); + void SetCommonFilterListTest4(); + + static TestSuite* Suite(); +}; + +#endif //SETCOMMONFILTERTEST_H + +/* + * $Log $ + * + * $Id $ + * + */ +