From 353ac3969efc5bda81b34f4edb3756dd6ced0412 Mon Sep 17 00:00:00 2001 From: Rick Pasetto Date: Tue, 10 Nov 2009 17:24:02 -0800 Subject: FIX DEV-41991: do not allow media settings panel to come up if media data is in flight Review #33 This change marks the current selection "not editable" if any objects in the selection are currently "in flight" (i.e. their media data has not been fetched yet, or is in the process of being fetched). This involved adding API to LLMediaDataClient to query whether an object is in the process of being fetched (i.e. in the queue). I've added a unit test for this new API. --- indra/newview/tests/llmediadataclient_test.cpp | 35 +++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) (limited to 'indra/newview/tests/llmediadataclient_test.cpp') diff --git a/indra/newview/tests/llmediadataclient_test.cpp b/indra/newview/tests/llmediadataclient_test.cpp index 445ec7aa34..3ac631d96e 100644 --- a/indra/newview/tests/llmediadataclient_test.cpp +++ b/indra/newview/tests/llmediadataclient_test.cpp @@ -497,5 +497,38 @@ namespace tut ensure("REF COUNT", o->getNumRefs(), 1); } - + template<> template<> + void mediadataclient_object_t::test<7>() + { + // Test LLMediaDataClient::isInQueue() + LOG_TEST(7); + + LLMediaDataClientObject::ptr_t o1 = new LLMediaDataClientObjectTest( + _DATA(VALID_OBJECT_ID_1,"3.0","1.0")); + LLMediaDataClientObject::ptr_t o2 = new LLMediaDataClientObjectTest( + _DATA(VALID_OBJECT_ID_2,"1.0","1.0")); + int num_refs_start = o1->getNumRefs(); + { + LLPointer mdc = new LLObjectMediaDataClient(NO_PERIOD,NO_PERIOD); + + ensure("not in queue yet 1", ! mdc->isInQueue(o1)); + ensure("not in queue yet 2", ! mdc->isInQueue(o2)); + + mdc->fetchMedia(o1); + + ensure("is in queue", mdc->isInQueue(o1)); + ensure("is not in queue", ! mdc->isInQueue(o2)); + + ::pump_timers(); + + ensure("not in queue anymore", ! mdc->isInQueue(o1)); + ensure("still is not in queue", ! mdc->isInQueue(o2)); + + ensure("queue empty", mdc->isEmpty()); + } + + // Make sure everyone's destroyed properly + ensure("REF COUNT", o1->getNumRefs(), num_refs_start); + + } } -- cgit v1.3 From ba3f7965e35e4eb0d7fcecc7267ec3af5f2d8d87 Mon Sep 17 00:00:00 2001 From: Monroe Linden Date: Thu, 19 Nov 2009 16:35:11 -0800 Subject: Made LLMediaDataClient not send requests on behalf of objects that are marked as dead. When LLMediaDataClient::QueueTimer::tick() encounters an object at the head of the queue that's dead, it will now remove that object and loop, instead of sending a request and waiting for the tick timer to fire again. Added an isDead() function to LLMediaDataClientObject, and an additional unit test that verifies the handling of dead objects. --- indra/newview/llmediadataclient.cpp | 116 +++++++++++++++---------- indra/newview/llmediadataclient.h | 2 + indra/newview/llvovolume.cpp | 3 + indra/newview/tests/llmediadataclient_test.cpp | 66 +++++++++++++- 4 files changed, 138 insertions(+), 49 deletions(-) (limited to 'indra/newview/tests/llmediadataclient_test.cpp') diff --git a/indra/newview/llmediadataclient.cpp b/indra/newview/llmediadataclient.cpp index 986c14acff..6666a03ee4 100755 --- a/indra/newview/llmediadataclient.cpp +++ b/indra/newview/llmediadataclient.cpp @@ -371,67 +371,87 @@ BOOL LLMediaDataClient::QueueTimer::tick() } LLMediaDataClient::PriorityQueue &queue = *(mMDC->pRequestQueue); - - if (queue.empty()) + + if(!queue.empty()) { - LL_DEBUGS("LLMediaDataClient") << "queue empty: " << queue << LL_ENDL; - return TRUE; + LL_INFOS("LLMediaDataClient") << "QueueTimer::tick() started, queue is: " << queue << LL_ENDL; } - LL_INFOS("LLMediaDataClient") << "QueueTimer::tick() started, queue is: " << queue << LL_ENDL; - - // Peel one off of the items from the queue, and execute request - request_ptr_t request = queue.top(); - llassert(!request.isNull()); - const LLMediaDataClientObject *object = (request.isNull()) ? NULL : request->getObject(); - bool performed_request = false; - bool error = false; - llassert(NULL != object); - if (NULL != object && object->hasMedia()) + // quick retry loop for cases where we shouldn't wait for the next timer tick + while(true) { - std::string url = request->getCapability(); - if (!url.empty()) + if (queue.empty()) { - const LLSD &sd_payload = request->getPayload(); - LL_INFOS("LLMediaDataClient") << "Sending request for " << *request << LL_ENDL; - - // Call the subclass for creating the responder - LLHTTPClient::post(url, sd_payload, mMDC->createResponder(request)); - performed_request = true; + LL_DEBUGS("LLMediaDataClient") << "queue empty: " << queue << LL_ENDL; + return TRUE; } - else { - LL_INFOS("LLMediaDataClient") << "NOT Sending request for " << *request << ": empty cap url!" << LL_ENDL; - } - } - else { - if (request.isNull()) + + // Peel one off of the items from the queue, and execute request + request_ptr_t request = queue.top(); + llassert(!request.isNull()); + const LLMediaDataClientObject *object = (request.isNull()) ? NULL : request->getObject(); + bool performed_request = false; + bool error = false; + llassert(NULL != object); + + if(object->isDead()) { - LL_WARNS("LLMediaDataClient") << "Not Sending request: NULL request!" << LL_ENDL; + // This object has been marked dead. Pop it and move on to the next item in the queue immediately. + LL_INFOS("LLMediaDataClient") << "Skipping " << *request << ": object is dead!" << LL_ENDL; + queue.pop(); + continue; // jump back to the start of the quick retry loop } - else if (NULL == object) + + if (NULL != object && object->hasMedia()) { - LL_WARNS("LLMediaDataClient") << "Not Sending request for " << *request << " NULL object!" << LL_ENDL; + std::string url = request->getCapability(); + if (!url.empty()) + { + const LLSD &sd_payload = request->getPayload(); + LL_INFOS("LLMediaDataClient") << "Sending request for " << *request << LL_ENDL; + + // Call the subclass for creating the responder + LLHTTPClient::post(url, sd_payload, mMDC->createResponder(request)); + performed_request = true; + } + else { + LL_INFOS("LLMediaDataClient") << "NOT Sending request for " << *request << ": empty cap url!" << LL_ENDL; + } } - else if (!object->hasMedia()) - { - LL_WARNS("LLMediaDataClient") << "Not Sending request for " << *request << " hasMedia() is false!" << LL_ENDL; + else { + if (request.isNull()) + { + LL_WARNS("LLMediaDataClient") << "Not Sending request: NULL request!" << LL_ENDL; + } + else if (NULL == object) + { + LL_WARNS("LLMediaDataClient") << "Not Sending request for " << *request << " NULL object!" << LL_ENDL; + } + else if (!object->hasMedia()) + { + LL_WARNS("LLMediaDataClient") << "Not Sending request for " << *request << " hasMedia() is false!" << LL_ENDL; + } + error = true; } - error = true; - } - bool exceeded_retries = request->getRetryCount() > mMDC->mMaxNumRetries; - if (performed_request || exceeded_retries || error) // Try N times before giving up - { - if (exceeded_retries) + bool exceeded_retries = request->getRetryCount() > mMDC->mMaxNumRetries; + if (performed_request || exceeded_retries || error) // Try N times before giving up { - LL_WARNS("LLMediaDataClient") << "Could not send request " << *request << " for " - << mMDC->mMaxNumRetries << " tries...popping object id " << object->getID() << LL_ENDL; - // XXX Should we bring up a warning dialog?? + if (exceeded_retries) + { + LL_WARNS("LLMediaDataClient") << "Could not send request " << *request << " for " + << mMDC->mMaxNumRetries << " tries...popping object id " << object->getID() << LL_ENDL; + // XXX Should we bring up a warning dialog?? + } + queue.pop(); } - queue.pop(); - } - else { - request->incRetryCount(); - } + else { + request->incRetryCount(); + } + + // end of quick retry loop -- any cases where we want to loop will use 'continue' to jump back to the start. + break; + } + LL_DEBUGS("LLMediaDataClient") << "QueueTimer::tick() finished, queue is now: " << (*(mMDC->pRequestQueue)) << LL_ENDL; return queue.empty(); diff --git a/indra/newview/llmediadataclient.h b/indra/newview/llmediadataclient.h index 0d1450ffbe..d5dd050111 100755 --- a/indra/newview/llmediadataclient.h +++ b/indra/newview/llmediadataclient.h @@ -62,6 +62,8 @@ public: virtual F64 getTotalMediaInterest() const = 0; // Return the given cap url virtual std::string getCapabilityUrl(const std::string &name) const = 0; + // Return whether the object has been marked dead + virtual bool isDead() const = 0; // smart pointer typedef LLPointer ptr_t; diff --git a/indra/newview/llvovolume.cpp b/indra/newview/llvovolume.cpp index 8bfbfcb9c3..5a67e64bbd 100644 --- a/indra/newview/llvovolume.cpp +++ b/indra/newview/llvovolume.cpp @@ -140,6 +140,9 @@ public: virtual std::string getCapabilityUrl(const std::string &name) const { return mObject->getRegion()->getCapability(name); } + virtual bool isDead() const + { return mObject->isDead(); } + private: LLPointer mObject; }; diff --git a/indra/newview/tests/llmediadataclient_test.cpp b/indra/newview/tests/llmediadataclient_test.cpp index 3ac631d96e..217889c390 100644 --- a/indra/newview/tests/llmediadataclient_test.cpp +++ b/indra/newview/tests/llmediadataclient_test.cpp @@ -152,12 +152,13 @@ public: std::istringstream d(data); LLSDSerialize::fromXML(mRep, d); mNumBounceBacks = 0; + mDead = false; // std::cout << ll_pretty_print_sd(mRep) << std::endl; // std::cout << "ID: " << getID() << std::endl; } LLMediaDataClientObjectTest(const LLSD &rep) - : mRep(rep), mNumBounceBacks(0) {} + : mRep(rep), mNumBounceBacks(0), mDead(false) {} ~LLMediaDataClientObjectTest() { LL_DEBUGS("LLMediaDataClient") << "~LLMediaDataClientObjectTest" << LL_ENDL; } @@ -187,12 +188,18 @@ public: virtual std::string getCapabilityUrl(const std::string &name) const { return mRep["cap_urls"][name]; } + virtual bool isDead() const + { return mDead; } + int getNumBounceBacks() const { return mNumBounceBacks; } + void markDead() + { mDead = true; } private: LLSD mRep; int mNumBounceBacks; + bool mDead; }; // This special timer delay should ensure that the timer will fire on the very @@ -531,4 +538,61 @@ namespace tut ensure("REF COUNT", o1->getNumRefs(), num_refs_start); } + + template<> template<> + void mediadataclient_object_t::test<8>() + { + // Test queue handling of objects that are marked dead. + LOG_TEST(8); + + LLMediaDataClientObject::ptr_t o1 = new LLMediaDataClientObjectTest(_DATA(VALID_OBJECT_ID_1,"1.0","1.0")); + LLMediaDataClientObject::ptr_t o2 = new LLMediaDataClientObjectTest(_DATA(VALID_OBJECT_ID_2,"2.0","1.0")); + LLMediaDataClientObject::ptr_t o3 = new LLMediaDataClientObjectTest(_DATA(VALID_OBJECT_ID_3,"3.0","1.0")); + LLMediaDataClientObject::ptr_t o4 = new LLMediaDataClientObjectTest(_DATA(VALID_OBJECT_ID_4,"4.0","1.0")); + { + LLPointer mdc = new LLObjectMediaDataClient(NO_PERIOD,NO_PERIOD); + + // queue up all 4 objects + mdc->fetchMedia(o1); + mdc->fetchMedia(o2); + mdc->fetchMedia(o3); + mdc->fetchMedia(o4); + + // and mark the second and fourth ones dead. + dynamic_cast(static_cast(o2))->markDead(); + dynamic_cast(static_cast(o4))->markDead(); + + ensure("is in queue 1", mdc->isInQueue(o1)); + ensure("is in queue 2", mdc->isInQueue(o2)); + ensure("is in queue 3", mdc->isInQueue(o3)); + ensure("is in queue 4", mdc->isInQueue(o4)); + ensure("post records", gPostRecords->size(), 0); + + ::pump_timers(); + + // The first tick should remove the first one + ensure("is not in queue 1", !mdc->isInQueue(o1)); + ensure("is in queue 2", mdc->isInQueue(o2)); + ensure("is in queue 3", mdc->isInQueue(o3)); + ensure("is in queue 4", mdc->isInQueue(o4)); + ensure("post records", gPostRecords->size(), 1); + + ::pump_timers(); + + // The second tick should skip the second and remove the third + ensure("is not in queue 2", !mdc->isInQueue(o2)); + ensure("is not in queue 3", !mdc->isInQueue(o3)); + ensure("is in queue 4", mdc->isInQueue(o4)); + ensure("post records", gPostRecords->size(), 2); + + ::pump_timers(); + + // The third tick should skip the fourth one and empty the queue. + ensure("is not in queue 4", !mdc->isInQueue(o4)); + ensure("post records", gPostRecords->size(), 2); + + ensure("queue empty", mdc->isEmpty()); + } + + } } -- cgit v1.3