#3612 Handle missing capabilities instead of blocking further downloads

Landmarks that failed due to missing caps were blocked from being
rerequested.

3 files changed, 54 insertions, 10 deletions

diff --git a/indra/newview/lllandmarklist.cpp b/indra/newview/lllandmarklist.cpp
index d67b5885f6..b25a42a938 100644
--- a/indra/newview/lllandmarklist.cpp
+++ b/indra/newview/lllandmarklist.cpp
@@ -147,24 +147,55 @@ void LLLandmarkList::processGetAssetReply(
             else
             {
                 // failed to parse, shouldn't happen
+                LL_WARNS("Landmarks") << "Failed to parse landmark " << uuid << LL_ENDL;
                 gLandmarkList.eraseCallbacks(uuid);
             }
         }
         else
         {
             // got a good status, but no file, shouldn't happen
+            LL_WARNS("Landmarks") << "Empty buffer for landmark " << uuid << LL_ENDL;
             gLandmarkList.eraseCallbacks(uuid);
         }
+
+        // We got this asset, remove it from retry and bad lists.
+        gLandmarkList.mRetryList.erase(uuid);
+        gLandmarkList.mBadList.erase(uuid);
     }
     else
     {
-        // SJB: No use case for a notification here.
-        //
-        // Todo: potentially cap getting obsolete due to a teleport
-        // can lead to this, so this might need a timeout or smarter
-        // handling to rerequest after a time instead of just failing
-        // al future requests.
-        if( LL_ERR_ASSET_REQUEST_NOT_IN_DATABASE == status )
+        if (LL_ERR_NO_CAP == status)
+        {
+            // A problem with asset cap, always allow retrying.
+            // Todo: should this reschedule?
+            gLandmarkList.mRequestedList.erase(uuid);
+            gLandmarkList.eraseCallbacks(uuid);
+            // If there was a previous request, it likely failed due to an obsolete cap
+            // so clear the retry marker to allow multiple retries.
+            gLandmarkList.mRetryList.erase(uuid);
+            return;
+        }
+        if (gLandmarkList.mBadList.find(uuid) != gLandmarkList.mBadList.end())
+        {
+            // Already on the 'bad' list, ignore
+            gLandmarkList.mRequestedList.erase(uuid);
+            gLandmarkList.eraseCallbacks(uuid);
+            return;
+        }
+        if (LL_ERR_ASSET_REQUEST_FAILED == status
+            && gLandmarkList.mRetryList.find(uuid) == gLandmarkList.mRetryList.end())
+        {
+            // There is a number of reasons why an asset request can fail,
+            // like a cap being obsolete due to user teleporting.
+            // Let viewer rerequest at least once more.
+            // Todo: should this reshchedule?
+            gLandmarkList.mRetryList.emplace(uuid);
+            gLandmarkList.mRequestedList.erase(uuid);
+            gLandmarkList.eraseCallbacks(uuid);
+            return;
+        }
+
+        if (LL_ERR_ASSET_REQUEST_NOT_IN_DATABASE == status)
         {
             LL_WARNS("Landmarks") << "Missing Landmark " << uuid << LL_ENDL;
         }
@@ -175,6 +206,7 @@ void LLLandmarkList::processGetAssetReply(
                 << ". Extended status: " << (S64)ext_status << LL_ENDL;
         }
 
+        gLandmarkList.mRetryList.erase(uuid);
         gLandmarkList.mBadList.insert(uuid);
         gLandmarkList.mRequestedList.erase(uuid); //mBadList effectively blocks any load, so no point keeping id in requests
         gLandmarkList.eraseCallbacks(uuid);
diff --git a/indra/newview/lllandmarklist.h b/indra/newview/lllandmarklist.h
index fb8b5a1960..76b5b97211 100644
--- a/indra/newview/lllandmarklist.h
+++ b/indra/newview/lllandmarklist.h
@@ -72,6 +72,7 @@ protected:
 
     typedef std::set<LLUUID> landmark_uuid_list_t;
     landmark_uuid_list_t mBadList;
+    landmark_uuid_list_t mRetryList;
 
     typedef std::map<LLUUID,F32> landmark_requested_list_t;
     landmark_requested_list_t mRequestedList;
diff --git a/indra/newview/llviewerassetstorage.cpp b/indra/newview/llviewerassetstorage.cpp
index 141f370ecb..de6b2d9e7c 100644
--- a/indra/newview/llviewerassetstorage.cpp
+++ b/indra/newview/llviewerassetstorage.cpp
@@ -461,7 +461,7 @@ void LLViewerAssetStorage::assetRequestCoro(
     if (!gAgent.getRegion())
     {
         LL_WARNS_ONCE("ViewerAsset") << "Asset request fails: no region set" << LL_ENDL;
-        result_code = LL_ERR_ASSET_REQUEST_FAILED;
+        result_code = LL_ERR_NO_CAP;
         ext_status = LLExtStat::NONE;
         removeAndCallbackPendingDownloads(uuid, atype, uuid, atype, result_code, ext_status, 0);
         return;
@@ -475,13 +475,24 @@ void LLViewerAssetStorage::assetRequestCoro(
         gAgent.getRegion()->setCapabilitiesReceivedCallback(
             boost::bind(&LLViewerAssetStorage::capsRecvForRegion, this, _1, capsRecv.getName()));
 
-        llcoro::suspendUntilEventOn(capsRecv);
+        F32Seconds timeout_seconds(LL_ASSET_STORAGE_TIMEOUT); // from minutes to seconds, by default 5 minutes
+        LLSD result = llcoro::suspendUntilEventOnWithTimeout(capsRecv, timeout_seconds, LLSDMap("timeout", LLSD::Boolean(true)));
 
         if (LLApp::isExiting() || !gAssetStorage)
         {
             return;
         }
 
+        if (result.has("timeout"))
+        {
+            // Caps failed to arrive in 5 minutes
+            LL_WARNS_ONCE("ViewerAsset") << "Asset " << uuid << " request fails : capabilities took too long to arrive" << LL_ENDL;
+            result_code = LL_ERR_NO_CAP;
+            ext_status = LLExtStat::NONE;
+            removeAndCallbackPendingDownloads(uuid, atype, uuid, atype, result_code, ext_status, 0);
+            return;
+        }
+
         LL_WARNS_ONCE("ViewerAsset") << "capsRecv got event" << LL_ENDL;
         LL_WARNS_ONCE("ViewerAsset") << "region " << gAgent.getRegion() << " mViewerAssetUrl " << mViewerAssetUrl << LL_ENDL;
     }
@@ -492,7 +503,7 @@ void LLViewerAssetStorage::assetRequestCoro(
     if (mViewerAssetUrl.empty())
     {
         LL_WARNS_ONCE("ViewerAsset") << "asset request fails: caps received but no viewer asset cap found" << LL_ENDL;
-        result_code = LL_ERR_ASSET_REQUEST_FAILED;
+        result_code = LL_ERR_NO_CAP;
         ext_status = LLExtStat::NONE;
         removeAndCallbackPendingDownloads(uuid, atype, uuid, atype, result_code, ext_status, 0);
         return;