From 8913ed6692fddc5d72ee01ecb92a21093c5d22ad Mon Sep 17 00:00:00 2001
From: Rider Linden <rider@lindenlab.com>
Date: Thu, 3 Sep 2015 16:59:00 -0700
Subject: [PATCH] Changes from code review with Nat

---
 indra/llmessage/llcoproceduremanager.cpp      | 36 +++++++------------
 indra/llui/llurlentry.cpp                     |  4 +--
 indra/newview/llcompilequeue.cpp              |  2 +-
 indra/newview/llfloaterexperienceprofile.cpp  | 10 +++---
 indra/newview/llfloaterregioninfo.cpp         |  4 +--
 indra/newview/llfloaterreporter.cpp           |  2 +-
 indra/newview/llpanelexperiencelisteditor.cpp |  2 +-
 indra/newview/llpanelexperiencelog.cpp        |  4 +--
 indra/newview/llpanelexperiencepicker.cpp     |  2 +-
 indra/newview/llpanelgroupexperiences.cpp     |  2 +-
 indra/newview/llpreviewscript.cpp             |  8 ++---
 indra/newview/llsidepaneliteminfo.cpp         |  2 +-
 indra/newview/llstartup.cpp                   |  2 +-
 indra/newview/llviewermessage.cpp             |  2 +-
 14 files changed, 35 insertions(+), 47 deletions(-)

diff --git a/indra/llmessage/llcoproceduremanager.cpp b/indra/llmessage/llcoproceduremanager.cpp
index c9dfcae2932..68da248ee4e 100644
--- a/indra/llmessage/llcoproceduremanager.cpp
+++ b/indra/llmessage/llcoproceduremanager.cpp
@@ -140,6 +140,8 @@ LLCoprocedureManager::poolPtr_t LLCoprocedureManager::initializePool(const std::
     std::string keyName = "PoolSize" + poolName;
     int size = 0;
 
+    LL_ERRS_IF(poolName.empty(), "CoprocedureManager") << "Poolname must not be empty" << LL_ENDL;
+
     if (mPropertyQueryFn && !mPropertyQueryFn.empty())
     {
         size = mPropertyQueryFn(keyName);
@@ -159,9 +161,10 @@ LLCoprocedureManager::poolPtr_t LLCoprocedureManager::initializePool(const std::
         LL_WARNS() << "LLCoprocedureManager: No setting for \"" << keyName << "\" setting pool size to default of " << size << LL_ENDL;
     }
 
-    poolPtr_t pool = poolPtr_t(new LLCoprocedurePool(poolName, size));
+    poolPtr_t pool(new LLCoprocedurePool(poolName, size));
     mPoolMap.insert(poolMap_t::value_type(poolName, pool));
 
+    LL_ERRS_IF(!pool, "CoprocedureManager") << "Unable to create pool named \"" << poolName << "\" FATAL!" << LL_ENDL;
     return pool;
 }
 
@@ -182,12 +185,6 @@ LLUUID LLCoprocedureManager::enqueueCoprocedure(const std::string &pool, const s
         targetPool = (*it).second;
     }
 
-    if (!targetPool)
-    {
-        LL_WARNS() << "LLCoprocedureManager unable to create coprocedure pool named \"" << pool << "\"" << LL_ENDL;
-        return LLUUID::null;
-    }
-
     return targetPool->enqueueCoprocedure(name, proc);
 }
 
@@ -286,14 +283,12 @@ LLCoprocedurePool::LLCoprocedurePool(const std::string &poolName, size_t size):
 {
     for (size_t count = 0; count < mPoolSize; ++count)
     {
-        LLCoreHttpUtil::HttpCoroutineAdapter::ptr_t httpAdapter =
-            LLCoreHttpUtil::HttpCoroutineAdapter::ptr_t(
-            new LLCoreHttpUtil::HttpCoroutineAdapter( mPoolName + "Adapter", mHTTPPolicy));
+        LLCoreHttpUtil::HttpCoroutineAdapter::ptr_t httpAdapter(new LLCoreHttpUtil::HttpCoroutineAdapter( mPoolName + "Adapter", mHTTPPolicy));
 
-        std::string uploadCoro = LLCoros::instance().launch("LLCoprocedurePool("+mPoolName+")::coprocedureInvokerCoro",
+        std::string pooledCoro = LLCoros::instance().launch("LLCoprocedurePool("+mPoolName+")::coprocedureInvokerCoro",
             boost::bind(&LLCoprocedurePool::coprocedureInvokerCoro, this, httpAdapter));
 
-        mCoroMapping.insert(CoroAdapterMap_t::value_type(uploadCoro, httpAdapter));
+        mCoroMapping.insert(CoroAdapterMap_t::value_type(pooledCoro, httpAdapter));
     }
 
     LL_INFOS() << "Created coprocedure pool named \"" << mPoolName << "\" with " << size << " items." << LL_ENDL;
@@ -313,12 +308,9 @@ void LLCoprocedurePool::shutdown(bool hardShutdown)
 
     for (it = mCoroMapping.begin(); it != mCoroMapping.end(); ++it)
     {
-        if (!(*it).first.empty())
+        if (hardShutdown)
         {
-            if (hardShutdown)
-            {
-                LLCoros::instance().kill((*it).first);
-            }
+            LLCoros::instance().kill((*it).first);
         }
         if ((*it).second)
         {
@@ -366,7 +358,7 @@ bool LLCoprocedurePool::cancelCoprocedure(const LLUUID &id)
         }
     }
 
-    LL_INFOS() << "Coprocedure with Id=" << id.asString() << " was not found." << " in pool \"" << mPoolName << "\"" << LL_ENDL;
+    LL_INFOS() << "Coprocedure with Id=" << id.asString() << " was not found in pool \"" << mPoolName << "\"" << LL_ENDL;
     return false;
 }
 
@@ -385,7 +377,7 @@ void LLCoprocedurePool::coprocedureInvokerCoro(LLCoreHttpUtil::HttpCoroutineAdap
         {
             QueuedCoproc::ptr_t coproc = mPendingCoprocs.front();
             mPendingCoprocs.pop_front();
-            mActiveCoprocs.insert(ActiveCoproc_t::value_type(coproc->mId, httpAdapter));
+            ActiveCoproc_t::iterator itActive = mActiveCoprocs.insert(ActiveCoproc_t::value_type(coproc->mId, httpAdapter)).first;
 
             LL_INFOS() << "Dequeued and invoking coprocedure(" << coproc->mName << ") with id=" << coproc->mId.asString() << " in pool \"" << mPoolName << "\"" << LL_ENDL;
 
@@ -405,11 +397,7 @@ void LLCoprocedurePool::coprocedureInvokerCoro(LLCoreHttpUtil::HttpCoroutineAdap
 
             LL_INFOS() << "Finished coprocedure(" << coproc->mName << ")" << " in pool \"" << mPoolName << "\"" << LL_ENDL;
 
-            ActiveCoproc_t::iterator itActive = mActiveCoprocs.find(coproc->mId);
-            if (itActive != mActiveCoprocs.end())
-            {
-                mActiveCoprocs.erase(itActive);
-            }
+            mActiveCoprocs.erase(itActive);
         }
     }
 }
diff --git a/indra/llui/llurlentry.cpp b/indra/llui/llurlentry.cpp
index e76f2a15505..72ff89f33c9 100755
--- a/indra/llui/llurlentry.cpp
+++ b/indra/llui/llurlentry.cpp
@@ -1430,7 +1430,7 @@ std::string LLUrlEntryExperienceProfile::getLabel( const std::string &url, const
         return LLTrans::getString("ExperienceNameNull");
     }
 
-    const LLSD& experience_details = LLExperienceCache::getInstance()->get(experience_id);
+    const LLSD& experience_details = LLExperienceCache::instance().get(experience_id);
     if(!experience_details.isUndefined())
     {
 		std::string experience_name_string = experience_details[LLExperienceCache::NAME].asString();
@@ -1438,7 +1438,7 @@ std::string LLUrlEntryExperienceProfile::getLabel( const std::string &url, const
     }
 
     addObserver(experience_id_string, url, cb);
-    LLExperienceCache::getInstance()->get(experience_id, boost::bind(&LLUrlEntryExperienceProfile::onExperienceDetails, this, _1));
+    LLExperienceCache::instance().get(experience_id, boost::bind(&LLUrlEntryExperienceProfile::onExperienceDetails, this, _1));
     return LLTrans::getString("LoadingData");
 
 }
diff --git a/indra/newview/llcompilequeue.cpp b/indra/newview/llcompilequeue.cpp
index b52ccffed96..219bcf0eb03 100755
--- a/indra/newview/llcompilequeue.cpp
+++ b/indra/newview/llcompilequeue.cpp
@@ -357,7 +357,7 @@ void LLFloaterCompileQueue::handleInventory(LLViewerObject *viewer_object,
 			LLScriptQueueData* datap = new LLScriptQueueData(getKey().asUUID(),
 				viewer_object->getID(), itemp);
 
-            LLExperienceCache::getInstance()->fetchAssociatedExperience(itemp->getParentUUID(), itemp->getUUID(),
+            LLExperienceCache::instance().fetchAssociatedExperience(itemp->getParentUUID(), itemp->getUUID(),
                     boost::bind(&LLFloaterCompileQueue::requestAsset, datap, _1));
 		}
 	}
diff --git a/indra/newview/llfloaterexperienceprofile.cpp b/indra/newview/llfloaterexperienceprofile.cpp
index 8a04f9e4cc5..e850be99ab9 100644
--- a/indra/newview/llfloaterexperienceprofile.cpp
+++ b/indra/newview/llfloaterexperienceprofile.cpp
@@ -99,7 +99,7 @@ class LLExperienceHandler : public LLCommandHandler
         if(params.size() != 2 || params[1].asString() != "profile")
             return false;
 
-        LLExperienceCache::getInstance()->get(params[0].asUUID(), boost::bind(&LLExperienceHandler::experienceCallback, this, _1));
+        LLExperienceCache::instance().get(params[0].asUUID(), boost::bind(&LLExperienceHandler::experienceCallback, this, _1));
         return true;
     }
 
@@ -288,8 +288,8 @@ BOOL LLFloaterExperienceProfile::postBuild()
 
     if (mExperienceId.notNull())
     {
-        LLExperienceCache::getInstance()->fetch(mExperienceId, true);
-        LLExperienceCache::getInstance()->get(mExperienceId, boost::bind(&LLFloaterExperienceProfile::experienceCallback,
+        LLExperienceCache::instance().fetch(mExperienceId, true);
+        LLExperienceCache::instance().get(mExperienceId, boost::bind(&LLFloaterExperienceProfile::experienceCallback,
             getDerivedHandle<LLFloaterExperienceProfile>(), _1)); 
         
         LLViewerRegion* region = gAgent.getRegion();
@@ -799,8 +799,8 @@ void LLFloaterExperienceProfile::onSaveComplete( const LLSD& content )
     }
  
     refreshExperience(*it);
-    LLExperienceCache::getInstance()->insert(*it);
-    LLExperienceCache::getInstance()->fetch(id, true);
+    LLExperienceCache::instance().insert(*it);
+    LLExperienceCache::instance().fetch(id, true);
 
     if(mSaveCompleteAction==VIEW)
     {
diff --git a/indra/newview/llfloaterregioninfo.cpp b/indra/newview/llfloaterregioninfo.cpp
index a166dda8ee6..94f3a45d88d 100755
--- a/indra/newview/llfloaterregioninfo.cpp
+++ b/indra/newview/llfloaterregioninfo.cpp
@@ -3699,7 +3699,7 @@ bool LLPanelRegionExperiences::refreshFromRegion(LLViewerRegion* region)
 	mTrusted->loading();
 	mTrusted->setReadonly(!allow_modify);
 
-    LLExperienceCache::getInstance()->getRegionExperiences(boost::bind(&LLPanelRegionExperiences::regionCapabilityQuery, region, _1),
+    LLExperienceCache::instance().getRegionExperiences(boost::bind(&LLPanelRegionExperiences::regionCapabilityQuery, region, _1),
         boost::bind(&LLPanelRegionExperiences::infoCallback, getDerivedHandle<LLPanelRegionExperiences>(), _1));
 
     return LLPanelRegionInfo::refreshFromRegion(region);
@@ -3727,7 +3727,7 @@ BOOL LLPanelRegionExperiences::sendUpdate()
 	content["blocked"]=addIds(mBlocked);
 	content["trusted"]=addIds(mTrusted);
 
-    LLExperienceCache::getInstance()->setRegionExperiences(boost::bind(&LLPanelRegionExperiences::regionCapabilityQuery, region, _1),
+    LLExperienceCache::instance().setRegionExperiences(boost::bind(&LLPanelRegionExperiences::regionCapabilityQuery, region, _1),
         content, boost::bind(&LLPanelRegionExperiences::infoCallback, getDerivedHandle<LLPanelRegionExperiences>(), _1));
 
 	return TRUE;
diff --git a/indra/newview/llfloaterreporter.cpp b/indra/newview/llfloaterreporter.cpp
index 714d8d0e8fb..1c2340b0efc 100755
--- a/indra/newview/llfloaterreporter.cpp
+++ b/indra/newview/llfloaterreporter.cpp
@@ -268,7 +268,7 @@ void LLFloaterReporter::getExperienceInfo(const LLUUID& experience_id)
 
 	if (LLUUID::null != mExperienceID)
 	{
-        const LLSD& experience = LLExperienceCache::getInstance()->get(mExperienceID);
+        const LLSD& experience = LLExperienceCache::instance().get(mExperienceID);
 		std::stringstream desc;
 
 		if(experience.isDefined())
diff --git a/indra/newview/llpanelexperiencelisteditor.cpp b/indra/newview/llpanelexperiencelisteditor.cpp
index 20fe0c52fa4..9d52a1906bd 100644
--- a/indra/newview/llpanelexperiencelisteditor.cpp
+++ b/indra/newview/llpanelexperiencelisteditor.cpp
@@ -183,7 +183,7 @@ void LLPanelExperienceListEditor::onItems()
 		columns[0]["value"] = getString("loading");
 		mItems->addElement(item);
 
-        LLExperienceCache::getInstance()->get(experience, boost::bind(&LLPanelExperienceListEditor::experienceDetailsCallback,
+        LLExperienceCache::instance().get(experience, boost::bind(&LLPanelExperienceListEditor::experienceDetailsCallback,
 			getDerivedHandle<LLPanelExperienceListEditor>(), _1));
 	}
 
diff --git a/indra/newview/llpanelexperiencelog.cpp b/indra/newview/llpanelexperiencelog.cpp
index 9329d900b1e..d5979b6e96f 100644
--- a/indra/newview/llpanelexperiencelog.cpp
+++ b/indra/newview/llpanelexperiencelog.cpp
@@ -140,7 +140,7 @@ void LLPanelExperienceLog::refresh()
 				}
 				const LLSD event = dayArray[i];
 				LLUUID id = event[LLExperienceCache::EXPERIENCE_ID].asUUID();
-                const LLSD& experience = LLExperienceCache::getInstance()->get(id);
+                const LLSD& experience = LLExperienceCache::instance().get(id);
 				if(experience.isUndefined()){
 					waiting = true;
 					waiting_id = id;
@@ -168,7 +168,7 @@ void LLPanelExperienceLog::refresh()
 	{
 		mEventList->deleteAllItems();
 		mEventList->setCommentText(getString("loading"));
-        LLExperienceCache::getInstance()->get(waiting_id, boost::bind(&LLPanelExperienceLog::refresh, this));
+        LLExperienceCache::instance().get(waiting_id, boost::bind(&LLPanelExperienceLog::refresh, this));
 	}
 	else
 	{
diff --git a/indra/newview/llpanelexperiencepicker.cpp b/indra/newview/llpanelexperiencepicker.cpp
index db846ffad94..d71ced443b1 100644
--- a/indra/newview/llpanelexperiencepicker.cpp
+++ b/indra/newview/llpanelexperiencepicker.cpp
@@ -130,7 +130,7 @@ void LLPanelExperiencePicker::find()
 	std::string text = getChild<LLUICtrl>(TEXT_EDIT)->getValue().asString();
 	mQueryID.generate();
 
-    LLExperienceCache::getInstance()->findExperienceByName(text, mCurrentPage,
+    LLExperienceCache::instance().findExperienceByName(text, mCurrentPage,
         boost::bind(&LLPanelExperiencePicker::findResults, getDerivedHandle<LLPanelExperiencePicker>(), mQueryID, _1));
 
     getChild<LLScrollListCtrl>(LIST_RESULTS)->deleteAllItems();
diff --git a/indra/newview/llpanelgroupexperiences.cpp b/indra/newview/llpanelgroupexperiences.cpp
index 067f9eb426f..9b4c67a120d 100644
--- a/indra/newview/llpanelgroupexperiences.cpp
+++ b/indra/newview/llpanelgroupexperiences.cpp
@@ -73,7 +73,7 @@ void LLPanelGroupExperiences::activate()
 		return;
 	}
 
-    LLExperienceCache::getInstance()->getGroupExperiences(getGroupID(),
+    LLExperienceCache::instance().getGroupExperiences(getGroupID(),
         boost::bind(&LLPanelGroupExperiences::groupExperiencesResults, getDerivedHandle<LLPanelGroupExperiences>(), _1));
 }
 
diff --git a/indra/newview/llpreviewscript.cpp b/indra/newview/llpreviewscript.cpp
index 8a493b70842..5f029ca6a22 100755
--- a/indra/newview/llpreviewscript.cpp
+++ b/indra/newview/llpreviewscript.cpp
@@ -1325,7 +1325,7 @@ void LLLiveLSLEditor::buildExperienceList()
 			position = ADD_TOP;
 		}
 		
-        const LLSD& experience = LLExperienceCache::getInstance()->get(id);
+        const LLSD& experience = LLExperienceCache::instance().get(id);
 		if(experience.isUndefined())
 		{
 			mExperiences->add(getString("loading"), id, position);
@@ -1344,7 +1344,7 @@ void LLLiveLSLEditor::buildExperienceList()
 
 	if(!foundAssociated )
 	{
-        const LLSD& experience = LLExperienceCache::getInstance()->get(associated);
+        const LLSD& experience = LLExperienceCache::instance().get(associated);
 		if(experience.isDefined())
 		{
 			std::string experience_name_string = experience[LLExperienceCache::NAME].asString();
@@ -1365,7 +1365,7 @@ void LLLiveLSLEditor::buildExperienceList()
 	if(last.notNull())
 	{
 		mExperiences->setEnabled(FALSE);
-        LLExperienceCache::getInstance()->get(last, boost::bind(&LLLiveLSLEditor::buildExperienceList, this));
+        LLExperienceCache::instance().get(last, boost::bind(&LLLiveLSLEditor::buildExperienceList, this));
 	}
 	else
 	{
@@ -2038,7 +2038,7 @@ void LLLiveLSLEditor::loadAsset()
 
 			if(item)
 			{
-                LLExperienceCache::getInstance()->fetchAssociatedExperience(item->getParentUUID(), item->getUUID(),
+                LLExperienceCache::instance().fetchAssociatedExperience(item->getParentUUID(), item->getUUID(),
                         boost::bind(&LLLiveLSLEditor::setAssociatedExperience, getDerivedHandle<LLLiveLSLEditor>(), _1));
 
 				bool isGodlike = gAgent.isGodlike();
diff --git a/indra/newview/llsidepaneliteminfo.cpp b/indra/newview/llsidepaneliteminfo.cpp
index ca17fe77b1a..12cbff888d3 100755
--- a/indra/newview/llsidepaneliteminfo.cpp
+++ b/indra/newview/llsidepaneliteminfo.cpp
@@ -328,7 +328,7 @@ void LLSidepanelItemInfo::refreshFromItem(LLViewerInventoryItem* item)
         tb->setText(getString("loading_experience"));
         tb->setVisible(TRUE);
 
-        LLExperienceCache::getInstance()->fetchAssociatedExperience(item->getParentUUID(), item->getUUID(), 
+        LLExperienceCache::instance().fetchAssociatedExperience(item->getParentUUID(), item->getUUID(), 
             boost::bind(&LLSidepanelItemInfo::setAssociatedExperience, getDerivedHandle<LLSidepanelItemInfo>(), _1));
     }
     
diff --git a/indra/newview/llstartup.cpp b/indra/newview/llstartup.cpp
index 361cc6c48b1..4246f722027 100755
--- a/indra/newview/llstartup.cpp
+++ b/indra/newview/llstartup.cpp
@@ -2822,7 +2822,7 @@ void LLStartUp::initNameCache()
 void LLStartUp::initExperiences()
 {   
     // Should trigger loading the cache.
-    LLExperienceCache::getInstance()->setCapabilityQuery(
+    LLExperienceCache::instance().setCapabilityQuery(
         boost::bind(&LLAgent::getRegionCapability, &gAgent, _1));
 
 	LLExperienceLog::instance().initialize();
diff --git a/indra/newview/llviewermessage.cpp b/indra/newview/llviewermessage.cpp
index 4e1a86bb711..ea8fc07e8a0 100755
--- a/indra/newview/llviewermessage.cpp
+++ b/indra/newview/llviewermessage.cpp
@@ -6653,7 +6653,7 @@ void process_script_question(LLMessageSystem *msg, void **user_data)
 			else if(experienceid.notNull())
 			{
 				payload["experience"]=experienceid;
-                LLExperienceCache::getInstance()->get(experienceid, boost::bind(process_script_experience_details, _1, args, payload));
+                LLExperienceCache::instance().get(experienceid, boost::bind(process_script_experience_details, _1, args, payload));
 				return;
 			}
 
-- 
GitLab