Refactoring of the request completion thread and removal of 206/content-range hack in xport.

Retry/response handling is decided in policy so moved that there.  Removed special case
206-without-content-range response in transport.  Have this sitation recognizable in the
API and let callers deal with it as needed.

6 files changed, 72 insertions, 63 deletions

diff --git a/indra/llcorehttp/_httplibcurl.cpp b/indra/llcorehttp/_httplibcurl.cpp
index 332b6f3856..e134a28401 100644
--- a/indra/llcorehttp/_httplibcurl.cpp
+++ b/indra/llcorehttp/_httplibcurl.cpp
@@ -113,8 +113,11 @@ HttpService::ELoopSpeed HttpLibcurl::processTransport()
 				CURL * handle(msg->easy_handle);
 				CURLcode result(msg->data.result);
 
-				HttpService::ELoopSpeed	speed(completeRequest(mMultiHandles[policy_class], handle, result));
-				ret = (std::min)(ret, speed);
+				if (completeRequest(mMultiHandles[policy_class], handle, result))
+				{
+					// Request is still active, don't get too sleepy
+					ret = (std::min)(ret, HttpService::NORMAL);
+				}
 				handle = NULL;			// No longer valid on return
 			}
 			else if (CURLMSG_NONE == msg->msg)
@@ -161,12 +164,8 @@ void HttpLibcurl::addOp(HttpOpRequest * op)
 }
 
 
-HttpService::ELoopSpeed HttpLibcurl::completeRequest(CURLM * multi_handle, CURL * handle, CURLcode status)
+bool HttpLibcurl::completeRequest(CURLM * multi_handle, CURL * handle, CURLcode status)
 {
-	static const HttpStatus cant_connect(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_CONNECT);
-	static const HttpStatus cant_res_proxy(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_RESOLVE_PROXY);
-	static const HttpStatus cant_res_host(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_RESOLVE_HOST);
-
 	HttpOpRequest * op(NULL);
 	curl_easy_getinfo(handle, CURLINFO_PRIVATE, &op);
 	// *FIXME:  check the pointer
@@ -209,43 +208,11 @@ HttpService::ELoopSpeed HttpLibcurl::completeRequest(CURLM * multi_handle, CURL
 	curl_multi_remove_handle(multi_handle, handle);
 	curl_easy_cleanup(handle);
 	op->mCurlHandle = NULL;
-	
-	// Retry or finalize
-	if (! op->mStatus)
-	{
-		// If this failed, we might want to retry.  Have to inspect
-		// the status a little more deeply for those reasons worth retrying...
-		if (op->mPolicyRetries < op->mPolicyRetryLimit &&
-			((op->mStatus.isHttpStatus() && op->mStatus.mType >= 499 && op->mStatus.mType <= 599) ||
-			 cant_connect == op->mStatus ||
-			 cant_res_proxy == op->mStatus ||
-			 cant_res_host == op->mStatus))
-		{
-			// Okay, worth a retry.  We include 499 in this test as
-			// it's the old 'who knows?' error from many grid services...
-			HttpPolicy & policy(mService->getPolicy());
-		
-			policy.retryOp(op);
-			return HttpService::NORMAL;			// Having pushed to retry, keep things running
-		}
-	}
 
-	// This op is done, finalize it delivering it to the reply queue...
-	if (! op->mStatus)
-	{
-		LL_WARNS("CoreHttp") << "URL op failed after " << op->mPolicyRetries
-							 << " retries.  Reason:  " << op->mStatus.toString()
-							 << LL_ENDL;
-	}
-	else if (op->mPolicyRetries)
-	{
-		LL_WARNS("CoreHttp") << "URL op succeeded after " << op->mPolicyRetries << " retries."
-							 << LL_ENDL;
-	}
+	HttpPolicy & policy(mService->getPolicy());
+	bool still_active(policy.stageAfterCompletion(op));
 
-	op->stageFromActive(mService);
-	op->release();
-	return HttpService::REQUEST_SLEEP;
+	return still_active;
 }
 
 
diff --git a/indra/llcorehttp/_httplibcurl.h b/indra/llcorehttp/_httplibcurl.h
index fe628b9ab0..16b68bde43 100644
--- a/indra/llcorehttp/_httplibcurl.h
+++ b/indra/llcorehttp/_httplibcurl.h
@@ -83,9 +83,7 @@ public:
 protected:
 	/// Invoked when libcurl has indicated a request has been processed
 	/// to completion and we need to move the request to a new state.
-	HttpService::ELoopSpeed completeRequest(CURLM * multi_handle,
-											CURL * handle,
-											CURLcode status);
+	bool completeRequest(CURLM * multi_handle, CURL * handle, CURLcode status);
 	
 protected:
 	typedef std::set<HttpOpRequest *> active_set_t;
diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp
index 895629c514..ea0b99303e 100644
--- a/indra/llcorehttp/_httpoprequest.cpp
+++ b/indra/llcorehttp/_httpoprequest.cpp
@@ -144,6 +144,8 @@ HttpOpRequest::~HttpOpRequest()
 		mCurlHeaders = NULL;
 	}
 
+	mReplyOffset = 0;
+	mReplyLength = 0;
 	if (mReplyBody)
 	{
 		mReplyBody->release();
@@ -211,26 +213,11 @@ void HttpOpRequest::visitNotifier(HttpRequest * request)
 		response->setStatus(mStatus);
 		response->setBody(mReplyBody);
 		response->setHeaders(mReplyHeaders);
-		unsigned int offset(0), length(0);
 		if (mReplyOffset || mReplyLength)
 		{
 			// Got an explicit offset/length in response
-			offset = mReplyOffset;
-			length = mReplyLength;
-		}
-		else if (mReplyBody && partial_content == mStatus)
-		{
-			// Legacy grid services did not provide a 'Content-Range'
-			// header in responses to full- or partly-satisfyiable
-			// 'Range' requests.  For these, we have to hope that
-			// the data starts where requested and the length is simply
-			// whatever we received.  A bit of sanity could be provided
-			// by overlapping ranged requests and verifying that the
-			// overlap matches.
-			offset = mReqOffset;
-			length = mReplyBody->size();
+			response->setRange(mReplyOffset, mReplyLength);
 		}
-		response->setRange(offset, length);
 
 		mLibraryHandler->onCompleted(static_cast<HttpHandle>(this), response);
 
diff --git a/indra/llcorehttp/_httppolicy.cpp b/indra/llcorehttp/_httppolicy.cpp
index 1f4cd34a4b..72bb6f14e4 100644
--- a/indra/llcorehttp/_httppolicy.cpp
+++ b/indra/llcorehttp/_httppolicy.cpp
@@ -185,5 +185,47 @@ bool HttpPolicy::changePriority(HttpHandle handle, HttpRequest::priority_t prior
 	return false;
 }
 
+bool HttpPolicy::stageAfterCompletion(HttpOpRequest * op)
+{
+	static const HttpStatus cant_connect(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_CONNECT);
+	static const HttpStatus cant_res_proxy(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_RESOLVE_PROXY);
+	static const HttpStatus cant_res_host(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_RESOLVE_HOST);
+
+	// Retry or finalize
+	if (! op->mStatus)
+	{
+		// If this failed, we might want to retry.  Have to inspect
+		// the status a little more deeply for those reasons worth retrying...
+		if (op->mPolicyRetries < op->mPolicyRetryLimit &&
+			((op->mStatus.isHttpStatus() && op->mStatus.mType >= 499 && op->mStatus.mType <= 599) ||
+			 cant_connect == op->mStatus ||
+			 cant_res_proxy == op->mStatus ||
+			 cant_res_host == op->mStatus))
+		{
+			// Okay, worth a retry.  We include 499 in this test as
+			// it's the old 'who knows?' error from many grid services...
+			retryOp(op);
+			return true;				// still active/ready
+		}
+	}
+
+	// This op is done, finalize it delivering it to the reply queue...
+	if (! op->mStatus)
+	{
+		LL_WARNS("CoreHttp") << "URL op failed after " << op->mPolicyRetries
+							 << " retries.  Reason:  " << op->mStatus.toString()
+							 << LL_ENDL;
+	}
+	else if (op->mPolicyRetries)
+	{
+		LL_WARNS("CoreHttp") << "URL op succeeded after " << op->mPolicyRetries << " retries."
+							 << LL_ENDL;
+	}
+
+	op->stageFromActive(mService);
+	op->release();
+	return false;						// not active
+}
+
 
 }  // end namespace LLCore
diff --git a/indra/llcorehttp/_httppolicy.h b/indra/llcorehttp/_httppolicy.h
index 6f18264f3d..14f6a9a676 100644
--- a/indra/llcorehttp/_httppolicy.h
+++ b/indra/llcorehttp/_httppolicy.h
@@ -79,6 +79,16 @@ public:
 	// Shadows HttpService's method
 	bool changePriority(HttpHandle handle, HttpRequest::priority_t priority);
 
+	/// When transport is finished with an op and takes it off the
+	/// active queue, it is delivered here for dispatch.  Policy
+	/// may send it back to the ready/retry queues if it needs another
+	/// go or we may finalize it and send it on to the reply queue.
+	///
+	/// @return			Returns true of the request is still active
+	///					or ready after staging, false if has been
+	///					sent on to the reply queue.
+	bool stageAfterCompletion(HttpOpRequest * op);
+	
 	// Get pointer to global policy options.  Caller is expected
 	// to do context checks like no setting once running.
 	HttpPolicyGlobal &	getGlobalOptions()
diff --git a/indra/newview/lltexturefetch.cpp b/indra/newview/lltexturefetch.cpp
index 4a46ea0e97..571f9ab3b5 100644
--- a/indra/newview/lltexturefetch.cpp
+++ b/indra/newview/lltexturefetch.cpp
@@ -1692,8 +1692,8 @@ void LLTextureFetchWorker::onCompleted(LLCore::HttpHandle handle, LLCore::HttpRe
 			 << " status: " << status.toHex()
 			 << " '" << status.toString() << "'"
 			 << llendl;
-	unsigned int offset(0), length(0);
-	response->getRange(&offset, &length);
+//	unsigned int offset(0), length(0);
+//	response->getRange(&offset, &length);
 // 	llwarns << "HTTP COMPLETE: " << mID << " handle: " << handle
 // 			<< " status: " << status.toULong() << " '" << status.toString() << "'"
 // 			<< " req offset: " << mRequestedOffset << " req length: " << mRequestedSize
@@ -1710,6 +1710,11 @@ void LLTextureFetchWorker::onCompleted(LLCore::HttpHandle handle, LLCore::HttpRe
 	}
 	else
 	{
+		// A warning about partial (HTTP 206) data.  Some grid services
+		// do *not* return a 'Content-Range' header in the response to
+		// Range requests with a 206 status.  We're forced to assume
+		// we get what we asked for in these cases until we can fix
+		// the services.
 		static const LLCore::HttpStatus par_status(HTTP_PARTIAL_CONTENT);
 
 		partial = (par_status == status);