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.

4 files changed, 138 insertions, 49 deletions

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<LLMediaDataClientObject> 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<LLVOVolume> 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<LLObjectMediaDataClient> 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<LLMediaDataClientObjectTest*>(static_cast<LLMediaDataClientObject*>(o2))->markDead();
+			dynamic_cast<LLMediaDataClientObjectTest*>(static_cast<LLMediaDataClientObject*>(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());
+		}
+		
+	}
 }