More cleanup around llmediadataclient.

LLMediaDataClient::Request no longer stores the LLSD payload -- it now uses the link to the LLVOVolume (which it was already keeping around) to generate the LLSD for the request at the time it's sent.

LLMediaDataClient::Request no longer stores a capability name.  Instead it uses the reference to its parent LLMediaDataClient subclass to refer to it as needed.

LLMediaDataClient::Request is now a base class, with subclasses for each specific request type -- GET, UPDATE, and NAVIGATE.

Responders are now created by a virtual method in the LLMediaDataClient::Request subclass instead of being created by the MDC.  This allows different request types to use different responder classes.

Fixed an issue with LLMediaDataClient::QueueTimer and LLMediaDataClient::RetryTimer that caused the unit test to fail (they now do all the work in their tick() functions instead of using the destructor).

2 files changed, 171 insertions, 157 deletions

diff --git a/indra/newview/llmediadataclient.cpp b/indra/newview/llmediadataclient.cpp
index 8f5290bac2..f0091d6fdc 100755
--- a/indra/newview/llmediadataclient.cpp
+++ b/indra/newview/llmediadataclient.cpp
@@ -167,14 +167,6 @@ LLMediaDataClient::request_ptr_t LLMediaDataClient::findOrRemove(request_queue_t
 	return result;
 }
 
-void LLMediaDataClient::request(const LLMediaDataClientObject::ptr_t &object, const LLSD &payload)
-{
-	if (object.isNull() || ! object->hasMedia()) return; 
-	
-	// Push the object on the queue
-	enqueue(new Request(getCapabilityName(), payload, object, this));
-}
-
 void LLMediaDataClient::enqueue(Request *request)
 {
 	if (request->isNew())
@@ -412,7 +404,7 @@ void LLMediaDataClient::serviceQueue()
 			LL_INFOS("LLMediaDataClient") << "Sending request for " << *request << LL_ENDL;
 			
 			// Call the subclass for creating the responder
-			LLHTTPClient::post(url, sd_payload, createResponder(request));
+			LLHTTPClient::post(url, sd_payload, request->createResponder());
 		}
 		else {
 			LL_INFOS("LLMediaDataClient") << "NOT Sending request for " << *request << ": empty cap url!" << LL_ENDL;
@@ -494,18 +486,24 @@ LLMediaDataClient::QueueTimer::QueueTimer(F32 time, LLMediaDataClient *mdc)
 	mMDC->setIsRunning(true);
 }
 
-LLMediaDataClient::QueueTimer::~QueueTimer()
-{
-	LL_DEBUGS("LLMediaDataClient") << "~QueueTimer" << LL_ENDL;
-	mMDC->setIsRunning(false);
-	mMDC = NULL;
-}
-
 // virtual
 BOOL LLMediaDataClient::QueueTimer::tick()
 {
-	if (mMDC.isNull()) return TRUE;
-	return mMDC->processQueueTimer();
+	BOOL result = TRUE;
+	
+	if (!mMDC.isNull())
+	{
+		result = mMDC->processQueueTimer();
+	
+		if(result)
+		{
+			// This timer won't fire again.  
+			mMDC->setIsRunning(false);
+			mMDC = NULL;
+		}
+	}
+
+	return result;
 }
 
 
@@ -541,12 +539,10 @@ BOOL LLMediaDataClient::Responder::RetryTimer::tick()
 //////////////////////////////////////////////////////////////////////////////////////
 /*static*/U32 LLMediaDataClient::Request::sNum = 0;
 
-LLMediaDataClient::Request::Request(const char *cap_name, 
-									const LLSD& sd_payload,
+LLMediaDataClient::Request::Request(Type in_type,
 									LLMediaDataClientObject *obj, 
 									LLMediaDataClient *mdc)
-: mCapName(cap_name), 
-  mPayload(sd_payload), 
+: mType(in_type),
   mObject(obj),
   mNum(++sNum), 
   mRetryCount(0),
@@ -556,43 +552,16 @@ LLMediaDataClient::Request::Request(const char *cap_name,
 {
 }
 
-LLMediaDataClient::Request::~Request()
+const char *LLMediaDataClient::Request::getCapName() const
 {
-	LL_DEBUGS("LLMediaDataClient") << "~Request" << (*this) << LL_ENDL;
-	mMDC = NULL;
-	mObject = NULL;
+	return mMDC->getCapabilityName();
 }
 
-
 std::string LLMediaDataClient::Request::getCapability() const
 {
 	return getObject()->getCapabilityUrl(getCapName());
 }
 
-// Helper function to get the "type" of request, which just pokes around to
-// discover it.
-LLMediaDataClient::Request::Type LLMediaDataClient::Request::getType() const
-{
-	if (0 == strcmp(mCapName, "ObjectMediaNavigate"))
-	{
-		return NAVIGATE;
-	}
-	else if (0 == strcmp(mCapName, "ObjectMedia"))
-	{
-		const std::string &verb = mPayload["verb"];
-		if (verb == "GET")
-		{
-			return GET;
-		}
-		else if (verb == "UPDATE")
-		{
-			return UPDATE;
-		}
-	}
-	llassert(false);
-	return GET;
-}
-
 const char *LLMediaDataClient::Request::getTypeAsString() const
 {
 	Type t = getType();
@@ -617,19 +586,17 @@ const char *LLMediaDataClient::Request::getTypeAsString() const
 
 void LLMediaDataClient::Request::reEnqueue()
 {
-	// I sure hope this doesn't deref a bad pointer:
 	mMDC->enqueue(this);
 }
 
 F32 LLMediaDataClient::Request::getRetryTimerDelay() const
 {
-	return (mMDC == NULL) ? LLMediaDataClient::UNAVAILABLE_RETRY_TIMER_DELAY :
-	mMDC->mRetryTimerDelay; 
+	return mMDC->mRetryTimerDelay; 
 }
 
 U32 LLMediaDataClient::Request::getMaxNumRetries() const
 {
-	return (mMDC == NULL) ? LLMediaDataClient::MAX_RETRIES : mMDC->mMaxNumRetries;
+	return mMDC->mMaxNumRetries;
 }
 
 void LLMediaDataClient::Request::markSent(bool flag)
@@ -675,12 +642,6 @@ LLMediaDataClient::Responder::Responder(const request_ptr_t &request)
 {
 }
 
-LLMediaDataClient::Responder::~Responder()
-{
-	LL_DEBUGS("LLMediaDataClient") << "~Responder" << *(getRequest()) << LL_ENDL;
-	mRequest = NULL;
-}
-
 /*virtual*/
 void LLMediaDataClient::Responder::error(U32 status, const std::string& reason)
 {
@@ -722,11 +683,6 @@ void LLMediaDataClient::Responder::result(const LLSD& content)
 //
 //////////////////////////////////////////////////////////////////////////////////////
 
-LLMediaDataClient::Responder *LLObjectMediaDataClient::createResponder(const request_ptr_t &request) const
-{
-	return new LLObjectMediaDataClient::Responder(request);
-}
-
 const char *LLObjectMediaDataClient::getCapabilityName() const 
 {
 	return "ObjectMedia";
@@ -734,68 +690,97 @@ const char *LLObjectMediaDataClient::getCapabilityName() const
 
 void LLObjectMediaDataClient::fetchMedia(LLMediaDataClientObject *object)
 {
-	LLSD sd_payload;
-	sd_payload["verb"] = "GET";
-	sd_payload[LLTextureEntry::OBJECT_ID_KEY] = object->getID();
-	request(object, sd_payload);
+	// Create a get request and put it in the queue.
+	enqueue(new RequestGet(object, this));
 }
 
+LLObjectMediaDataClient::RequestGet::RequestGet(LLMediaDataClientObject *obj, LLMediaDataClient *mdc):
+	LLMediaDataClient::Request(LLMediaDataClient::Request::GET, obj, mdc)
+{
+}
+
+LLSD LLObjectMediaDataClient::RequestGet::getPayload() const
+{
+	LLSD result;
+	result["verb"] = "GET";
+	result[LLTextureEntry::OBJECT_ID_KEY] = mObject->getID();
+	
+	return result;
+}
+
+LLMediaDataClient::Responder *LLObjectMediaDataClient::RequestGet::createResponder()
+{
+	return new LLObjectMediaDataClient::Responder(this);
+}
+
+
 void LLObjectMediaDataClient::updateMedia(LLMediaDataClientObject *object)
 {
-	LLSD sd_payload;
-	sd_payload["verb"] = "UPDATE";
-	sd_payload[LLTextureEntry::OBJECT_ID_KEY] = object->getID();
+	// Create an update request and put it in the queue.
+	enqueue(new RequestUpdate(object, this));
+}
+
+LLObjectMediaDataClient::RequestUpdate::RequestUpdate(LLMediaDataClientObject *obj, LLMediaDataClient *mdc):
+	LLMediaDataClient::Request(LLMediaDataClient::Request::UPDATE, obj, mdc)
+{
+}
+
+LLSD LLObjectMediaDataClient::RequestUpdate::getPayload() const
+{
+	LLSD result;
+	result["verb"] = "UPDATE";
+	result[LLTextureEntry::OBJECT_ID_KEY] = mObject->getID();
+
 	LLSD object_media_data;
 	int i = 0;
-	int end = object->getMediaDataCount();
+	int end = mObject->getMediaDataCount();
 	for ( ; i < end ; ++i) 
 	{
-		object_media_data.append(object->getMediaDataLLSD(i));
+		object_media_data.append(mObject->getMediaDataLLSD(i));
 	}
-	sd_payload[LLTextureEntry::OBJECT_MEDIA_DATA_KEY] = object_media_data;
-		
-	LL_DEBUGS("LLMediaDataClient") << "update media data: " << object->getID() << " " << ll_print_sd(sd_payload) << LL_ENDL;
 	
-	request(object, sd_payload);
+	result[LLTextureEntry::OBJECT_MEDIA_DATA_KEY] = object_media_data;
+	
+	return result;
+}
+
+LLMediaDataClient::Responder *LLObjectMediaDataClient::RequestUpdate::createResponder()
+{
+	// This just uses the base class's responder.
+	return new LLMediaDataClient::Responder(this);
 }
 
+
 /*virtual*/
 void LLObjectMediaDataClient::Responder::result(const LLSD& content)
 {
-	const LLMediaDataClient::Request::Type type = getRequest()->getType();
-	llassert(type == LLMediaDataClient::Request::GET || type == LLMediaDataClient::Request::UPDATE)
-	if (type == LLMediaDataClient::Request::GET)
+	// This responder is only used for GET requests, not UPDATE.
+
+	LL_DEBUGS("LLMediaDataClientResponse") << *(getRequest()) << " GET returned: " << ll_print_sd(content) << LL_ENDL;
+	
+	// Look for an error
+	if (content.has("error"))
 	{
-		LL_DEBUGS("LLMediaDataClientResponse") << *(getRequest()) << " GET returned: " << ll_print_sd(content) << LL_ENDL;
+		const LLSD &error = content["error"];
+		LL_WARNS("LLMediaDataClient") << *(getRequest()) << " Error getting media data for object: code=" << 
+			error["code"].asString() << ": " << error["message"].asString() << LL_ENDL;
 		
-		// Look for an error
-		if (content.has("error"))
-		{
-			const LLSD &error = content["error"];
-			LL_WARNS("LLMediaDataClient") << *(getRequest()) << " Error getting media data for object: code=" << 
-				error["code"].asString() << ": " << error["message"].asString() << LL_ENDL;
-			
-			// XXX Warn user?
-		}
-		else {
-			// Check the data
-			const LLUUID &object_id = content[LLTextureEntry::OBJECT_ID_KEY];
-			if (object_id != getRequest()->getObject()->getID()) 
-			{
-				// NOT good, wrong object id!!
-				LL_WARNS("LLMediaDataClient") << *(getRequest()) << " DROPPING response with wrong object id (" << object_id << ")" << LL_ENDL;
-				return;
-			}
-			
-			// Otherwise, update with object media data
-			getRequest()->getObject()->updateObjectMediaData(content[LLTextureEntry::OBJECT_MEDIA_DATA_KEY],
-															 content[LLTextureEntry::MEDIA_VERSION_KEY]);
-		}
+		// XXX Warn user?
 	}
-	else if (type == LLMediaDataClient::Request::UPDATE)
+	else 
 	{
-		// just do what our superclass does
-		LLMediaDataClient::Responder::result(content);
+		// Check the data
+		const LLUUID &object_id = content[LLTextureEntry::OBJECT_ID_KEY];
+		if (object_id != getRequest()->getObject()->getID()) 
+		{
+			// NOT good, wrong object id!!
+			LL_WARNS("LLMediaDataClient") << *(getRequest()) << " DROPPING response with wrong object id (" << object_id << ")" << LL_ENDL;
+			return;
+		}
+		
+		// Otherwise, update with object media data
+		getRequest()->getObject()->updateObjectMediaData(content[LLTextureEntry::OBJECT_MEDIA_DATA_KEY],
+														 content[LLTextureEntry::MEDIA_VERSION_KEY]);
 	}
 }
 
@@ -805,10 +790,6 @@ void LLObjectMediaDataClient::Responder::result(const LLSD& content)
 // Subclass of LLMediaDataClient for the ObjectMediaNavigate cap
 //
 //////////////////////////////////////////////////////////////////////////////////////
-LLMediaDataClient::Responder *LLObjectMediaNavigateClient::createResponder(const request_ptr_t &request) const
-{
-	return new LLObjectMediaNavigateClient::Responder(request);
-}
 
 const char *LLObjectMediaNavigateClient::getCapabilityName() const 
 {
@@ -817,14 +798,33 @@ const char *LLObjectMediaNavigateClient::getCapabilityName() const
 
 void LLObjectMediaNavigateClient::navigate(LLMediaDataClientObject *object, U8 texture_index, const std::string &url)
 {
-	LLSD sd_payload;
-	sd_payload[LLTextureEntry::OBJECT_ID_KEY] = object->getID();
-	sd_payload[LLMediaEntry::CURRENT_URL_KEY] = url;
-	sd_payload[LLTextureEntry::TEXTURE_INDEX_KEY] = (LLSD::Integer)texture_index;
+
+//	LL_INFOS("LLMediaDataClient") << "navigate() initiated: " << ll_print_sd(sd_payload) << LL_ENDL;
 	
-	LL_INFOS("LLMediaDataClient") << "navigate() initiated: " << ll_print_sd(sd_payload) << LL_ENDL;
+	// Create a get request and put it in the queue.
+	enqueue(new RequestNavigate(object, this, texture_index, url));
+}
+
+LLObjectMediaNavigateClient::RequestNavigate::RequestNavigate(LLMediaDataClientObject *obj, LLMediaDataClient *mdc, U8 texture_index, const std::string &url):
+	LLMediaDataClient::Request(LLMediaDataClient::Request::NAVIGATE, obj, mdc),
+	mTextureIndex(texture_index),
+	mURL(url)
+{
+}
+
+LLSD LLObjectMediaNavigateClient::RequestNavigate::getPayload() const
+{
+	LLSD result;
+	result[LLTextureEntry::OBJECT_ID_KEY] = mObject->getID();
+	result[LLMediaEntry::CURRENT_URL_KEY] = mURL;
+	result[LLTextureEntry::TEXTURE_INDEX_KEY] = (LLSD::Integer)mTextureIndex;
 	
-	request(object, sd_payload);
+	return result;
+}
+
+LLMediaDataClient::Responder *LLObjectMediaNavigateClient::RequestNavigate::createResponder()
+{
+	return new LLObjectMediaNavigateClient::Responder(this);
 }
 
 /*virtual*/
diff --git a/indra/newview/llmediadataclient.h b/indra/newview/llmediadataclient.h
index 3e86c52c63..9504a0a2a9 100755
--- a/indra/newview/llmediadataclient.h
+++ b/indra/newview/llmediadataclient.h
@@ -93,9 +93,6 @@ public:
 					  U32 max_sorted_queue_size = MAX_SORTED_QUEUE_SIZE,
 					  U32 max_round_robin_queue_size = MAX_ROUND_ROBIN_QUEUE_SIZE);
 	
-	// Make the request
-	void request(const LLMediaDataClientObject::ptr_t &object, const LLSD &payload);
-
 	F32 getRetryTimerDelay() const { return mRetryTimerDelay; }
 	
 	// Returns true iff the queue is empty
@@ -114,10 +111,17 @@ protected:
 	// Destructor
 	virtual ~LLMediaDataClient(); // use unref
     
-	// Request
+	class Responder;
+	
+	// Request (pure virtual base class for requests in the queue)
 	class Request : public LLRefCount
 	{
 	public:
+		// Subclasses must implement this to build a payload for their request type.
+		virtual LLSD getPayload() const = 0;
+		// and must create the correct type of responder.
+		virtual Responder *createResponder() = 0;
+
         enum Type {
             GET,
             UPDATE,
@@ -125,20 +129,22 @@ protected:
 			ANY
         };
         
-		Request(const char *cap_name, const LLSD& sd_payload, LLMediaDataClientObject *obj, LLMediaDataClient *mdc);
-		const char *getCapName() const { return mCapName; }
-		const LLSD &getPayload() const { return mPayload; }
+	protected:
+		// The only way to create one of these is through a subclass.
+		Request(Type in_type, LLMediaDataClientObject *obj, LLMediaDataClient *mdc);
+	public:
 		LLMediaDataClientObject *getObject() const { return mObject; }
 
         U32 getNum() const { return mNum; }
-
 		U32 getRetryCount() const { return mRetryCount; }
-		void incRetryCount() { mRetryCount++; }
+		void incRetryCount() { mRetryCount++; };
+        Type getType() const { return mType; };
+		bool isMarkedSent() const { return mMarkedSent; }
+		F64 getScore() const { return mScore; }
 		
 		// Note: may return empty string!
 		std::string getCapability() const;
-        
-        Type getType() const;
+		const char *getCapName() const;
 		const char *getTypeAsString() const;
 		
 		// Re-enqueue thyself
@@ -148,21 +154,16 @@ protected:
 		U32 getMaxNumRetries() const;
 		
 		bool isNew() const { return mObject.notNull() ? mObject->isNew() : false; }
+		bool isObjectValid() const { return mObject.notNull() ? (!mObject->isDead()) : false; }
 		void markSent(bool flag);
-		bool isMarkedSent() const { return mMarkedSent; }
 		void updateScore();
-		F64 getScore() const { return mScore; }
 		
-	public:
 		friend std::ostream& operator<<(std::ostream &s, const Request &q);
-		
-    protected:
-        virtual ~Request(); // use unref();
-        
-	private:
-		const char *mCapName;
-		LLSD mPayload;
+
+	protected:
 		LLMediaDataClientObject::ptr_t mObject;
+	private:
+		Type mType;
 		// Simple tracking
 		U32 mNum;
 		static U32 sNum;
@@ -187,9 +188,6 @@ protected:
 
 		request_ptr_t &getRequest() { return mRequest; }
 
-    protected:
-		virtual ~Responder();
-        
 	private:
 
 		class RetryTimer : public LLEventTimer
@@ -207,20 +205,17 @@ protected:
 	
 protected:
 
-	// Subclasses must override this factory method to return a new responder
-	virtual Responder *createResponder(const request_ptr_t &request) const = 0;
-	
 	// Subclasses must override to return a cap name
 	virtual const char *getCapabilityName() const = 0;
 	
 	virtual bool request_needs_purge(request_ptr_t request);
 	virtual void sortQueue();
 	virtual void serviceQueue();
+
+	virtual void enqueue(Request*);
 	
 private:
 	typedef std::list<request_ptr_t> request_queue_t;
-		
-	void enqueue(Request*);
 	
 	// Return whether the given object is/was in the queue
 	static LLMediaDataClient::request_ptr_t findOrRemove(request_queue_t &queue, const LLMediaDataClientObject::ptr_t &obj, bool remove, Request::Type type);
@@ -237,8 +232,6 @@ private:
 	public:
 		QueueTimer(F32 time, LLMediaDataClient *mdc);
 		virtual BOOL tick();
-    protected:
-		virtual ~QueueTimer();
 	private:
 		// back-pointer
 		LLPointer<LLMediaDataClient> mMDC;
@@ -280,11 +273,24 @@ public:
     
 	void fetchMedia(LLMediaDataClientObject *object); 
     void updateMedia(LLMediaDataClientObject *object);
-    
+
+	class RequestGet: public Request
+	{
+	public:
+		RequestGet(LLMediaDataClientObject *obj, LLMediaDataClient *mdc);
+		/*virtual*/ LLSD getPayload() const;
+		/*virtual*/ Responder *createResponder();
+	};
+
+	class RequestUpdate: public Request
+	{
+	public:
+		RequestUpdate(LLMediaDataClientObject *obj, LLMediaDataClient *mdc);
+		/*virtual*/ LLSD getPayload() const;
+		/*virtual*/ Responder *createResponder();
+	};
+	    
 protected:
-	// Subclasses must override this factory method to return a new responder
-	virtual Responder *createResponder(const request_ptr_t &request) const;
-	
 	// Subclasses must override to return a cap name
 	virtual const char *getCapabilityName() const;
     
@@ -315,11 +321,19 @@ public:
     virtual ~LLObjectMediaNavigateClient() {}
     
     void navigate(LLMediaDataClientObject *object, U8 texture_index, const std::string &url);
+
+	class RequestNavigate: public Request
+	{
+	public:
+		RequestNavigate(LLMediaDataClientObject *obj, LLMediaDataClient *mdc, U8 texture_index, const std::string &url);
+		/*virtual*/ LLSD getPayload() const;
+		/*virtual*/ Responder *createResponder();
+	private:
+		U8 mTextureIndex;
+		std::string mURL;
+	};
     
 protected:
-	// Subclasses must override this factory method to return a new responder
-	virtual Responder *createResponder(const request_ptr_t &request) const;
-	
 	// Subclasses must override to return a cap name
 	virtual const char *getCapabilityName() const;