MAINT-6570: Feedback from code review.

2 files changed, 49 insertions, 51 deletions

diff --git a/indra/llcommon/llhandle.h b/indra/llcommon/llhandle.h
index 98ad7af81a..8e073aab27 100644
--- a/indra/llcommon/llhandle.h
+++ b/indra/llcommon/llhandle.h
@@ -28,6 +28,7 @@
 #define LLHANDLE_H
 
 #include "llpointer.h"
+#include "llexception.h"
 #include <stdexcept>
 #include <boost/type_traits/is_convertible.hpp>
 #include <boost/utility/enable_if.hpp>
@@ -215,64 +216,53 @@ private:
 	mutable LLRootHandle<T> mHandle;
 };
 
-/*
- * $TODO: Derive from LLException
- */
-class LLExeceptionStaleHandle : public std::runtime_error
+
+
+class LLCheckedHandleBase
 {
 public:
-    LLExeceptionStaleHandle():
-        std::runtime_error("Attempt to access stale handle.")
-   {}
+    class Stale : public LLException
+    {
+    public:
+        Stale() :
+            LLException("Attempt to access stale handle.")
+        {}
+    };
+
+protected:
+    LLCheckedHandleBase() { }
+
 };
 
 /**
  * This is a simple wrapper for Handles, allowing direct calls to the underlying 
- * pointer. The checked handle will throw a LLExeceptionStaleHandle if an attempt 
+ * pointer. The checked handle will throw a Stale if an attempt 
  * is made to access the object referenced by the handle and that object has 
  * been destroyed.
  **/
 template <typename T> 
-class LLCheckedHandle: private boost::noncopyable
+class LLCheckedHandle: public LLCheckedHandleBase
 {
 public:
+
     LLCheckedHandle(LLHandle<T> handle):
         mHandle(handle)
     { }
 
     /**
-     * Retrieve the underlying pointer by resolving the handle. If the handle
-     * returns NULL for the pointer throw an LLExeceptionStaleHandle exception.
+     * Test the underlying handle.  If it is no longer valid, throw a Stale exception.
      */
-    T* get() const
+    void check() const
     {
         T* ptr = mHandle.get();
         if (!ptr)
-            BOOST_THROW_EXCEPTION(LLExeceptionStaleHandle());
-        return ptr;
+            BOOST_THROW_EXCEPTION(Stale());
     }
 
     /**
-     * Test the handle to see if it is still valid. Returns true if it is,
-     * false if it is not. Does not trow.
+     * Cast back to an appropriate handle
      */
-    bool test() const
-    {
-        return (mHandle.get() != NULL);
-    }
-
-    /**
-     * Test the underlying handle.  If it is no longer valid, throw a LLExeceptionStaleHandle.
-     */
-    void check() const
-    {
-        get();
-    }
-
-    /**
-     * Get the contained handle. 
-     */
-    LLHandle<T> getHandle() const
+    operator LLHandle<T>() const
     {
         return mHandle;
     }
@@ -281,22 +271,26 @@ public:
      * Converts the LLCheckedHandle to a bool. Allows for if (chkdHandle) {} 
      * Does not throw.
      */
-    operator bool() const
+    explicit operator bool() const
     {
-        return test();
+        return (mHandle.get() != NULL);
     }
 
     /**
      * Attempt to call a method or access a member in the structure referenced 
      * by the handle.  If the handle no longer points to a valid structure 
-     * throw a LLExeceptionStaleHandle.
+     * throw a Stale.
      */
     T* operator ->() const
     {
-        return get();
+        T* ptr = mHandle.get();
+        if (!ptr)
+            BOOST_THROW_EXCEPTION(Stale());
+        return ptr;
     }
 
 private:
+
     LLHandle<T> mHandle;
 };
 
diff --git a/indra/newview/llcompilequeue.cpp b/indra/newview/llcompilequeue.cpp
index f16a89bd88..76e16f5a1f 100644
--- a/indra/newview/llcompilequeue.cpp
+++ b/indra/newview/llcompilequeue.cpp
@@ -342,12 +342,14 @@ void LLFloaterCompileQueue::processExperienceIdResults(LLSD result, LLUUID paren
 
 }
 
+/// This is a utility function to be bound and called from objectScriptProcessingQueueCoro.
+/// Do not call directly. It may throw a LLCheckedHandle<>::Stale exception.
 bool LLFloaterCompileQueue::processScript(LLHandle<LLFloaterCompileQueue> hfloater,
     const LLPointer<LLViewerObject> &object, LLInventoryObject* inventory, LLEventPump &pump)
 {
     LLSD result;
     LLCheckedHandle<LLFloaterCompileQueue> floater(hfloater);
-    // Calls to floater may fail. If they do they throw LLExeceptionStaleHandle.
+    // Dereferencing floater may fail. If they do they throw LLExeceptionStaleHandle.
     // which is caught in objectScriptProcessingQueueCoro
     bool monocompile = floater->mMono;
     F32 fetch_timeout = gSavedSettings.getF32("QueueInventoryFetchTimeout");
@@ -541,11 +543,13 @@ LLFloaterResetQueue::~LLFloaterResetQueue()
 { 
 }
 
-bool LLFloaterResetQueue::resetObjectScripts(LLHandle<LLFloaterScriptQueue> hfloater, 
+/// This is a utility function to be bound and called from objectScriptProcessingQueueCoro.
+/// Do not call directly. It may throw a LLCheckedHandle<>::Stale exception.
+bool LLFloaterResetQueue::resetObjectScripts(LLHandle<LLFloaterScriptQueue> hfloater,
     const LLPointer<LLViewerObject> &object, LLInventoryObject* inventory, LLEventPump &pump)
 {
     LLCheckedHandle<LLFloaterScriptQueue> floater(hfloater);
-    // Calls to floater may fail. If they do they throw LLExeceptionStaleHandle.
+    // Dereferencing floater may fail. If they do they throw LLExeceptionStaleHandle.
     // which is caught in objectScriptProcessingQueueCoro
 
     std::string buffer;
@@ -596,11 +600,13 @@ LLFloaterRunQueue::~LLFloaterRunQueue()
 { 
 }
 
-bool LLFloaterRunQueue::runObjectScripts(LLHandle<LLFloaterScriptQueue> hfloater, 
+/// This is a utility function to be bound and called from objectScriptProcessingQueueCoro.
+/// Do not call directly. It may throw a LLCheckedHandle<>::Stale exception.
+bool LLFloaterRunQueue::runObjectScripts(LLHandle<LLFloaterScriptQueue> hfloater,
     const LLPointer<LLViewerObject> &object, LLInventoryObject* inventory, LLEventPump &pump)
 {
     LLCheckedHandle<LLFloaterScriptQueue> floater(hfloater);
-    // Calls to floater may fail. If they do they throw LLExeceptionStaleHandle.
+    // Dereferencing floater may fail. If they do they throw LLExeceptionStaleHandle.
     // which is caught in objectScriptProcessingQueueCoro
 
     std::string buffer;
@@ -651,11 +657,13 @@ LLFloaterNotRunQueue::~LLFloaterNotRunQueue()
 { 
 }
 
+/// This is a utility function to be bound and called from objectScriptProcessingQueueCoro.
+/// Do not call directly. It may throw a LLCheckedHandle<>::Stale exception.
 bool LLFloaterNotRunQueue::stopObjectScripts(LLHandle<LLFloaterScriptQueue> hfloater, 
     const LLPointer<LLViewerObject> &object, LLInventoryObject* inventory, LLEventPump &pump)
 {
     LLCheckedHandle<LLFloaterScriptQueue> floater(hfloater);
-    // Calls to floater may fail. If they do they throw LLExeceptionStaleHandle.
+    // Dereferencing floater may fail. If they do they throw LLExeceptionStaleHandle.
     // which is caught in objectScriptProcessingQueueCoro
 
     std::string buffer;
@@ -708,7 +716,8 @@ void LLFloaterScriptQueue::objectScriptProcessingQueueCoro(std::string action, L
 {
     LLCoros::set_consuming(true);
     LLCheckedHandle<LLFloaterScriptQueue> floater(hfloater);    
-    // Calls to floater may fail. If they do they throw LLExeceptionStaleHandle. This is expected if the dialog closes.
+    // Dereferencing floater may fail. If they do they throw LLExeceptionStaleHandle. 
+    // This is expected if the dialog closes.
     LLEventMailDrop        maildrop(QUEUE_EVENTPUMP_NAME, true);
     F32 fetch_timeout = gSavedSettings.getF32("QueueInventoryFetchTimeout");
 
@@ -793,15 +802,10 @@ void LLFloaterScriptQueue::objectScriptProcessingQueueCoro(std::string action, L
         floater->addStringMessage("Done");
         floater->getChildView("close")->setEnabled(TRUE);
     }
-    catch (LLExeceptionStaleHandle &)
+    catch (LLCheckedHandleBase::Stale &)
     {
         // This is expected.  It means that floater has been closed before 
         // processing was completed.
-        LL_WARNS("SCRIPTQ") << "LLExeceptionStaleHandle caught! Floater has most likely been closed." << LL_ENDL;
-    }
-    catch (...)
-    {
-        // Rethrow the caught exception.
-        throw;
+        LL_DEBUGS("SCRIPTQ") << "LLExeceptionStaleHandle caught! Floater has most likely been closed." << LL_ENDL;
     }
 }