DRTVWR-476: Eliminate static LLEventPump factory maps.

Having a map from std::string to a factory function returning LLEventPump* is
a cool idea, especially since you could statically populate such a map with
string literals and little lambdas.

Unfortunately, static initialization of any data is a bad idea when control
can reach consuming code before that module's static data are constructed.

Since LLEventPumps is already an LLSingleton, it's simple enough to make its
map non-static and initialize it in the constructor.

But another recent static factory-function map was introduced in
llleaplistener.cpp to support the LLLeapListener::newpump() operation. That
involves no LLSingletons.

Introduce LLEventPumps::make(name, tweak, type) to instantiate an LLEventPump
subclass of the specified type with specified (name, tweak) parameters.
Instances returned by make() are owned by LLEventPumps, as with obtain().
Introduce LLEventPumps::BadType exception for when the type string isn't
recognized.

LLEventPumps::obtain() can then simply call make() when the specified instance
name doesn't already exist. The configuration data used internally by obtain()
becomes { string instance name, string subclass name }. Although this too is
currently initialized in the LLEventPumps constructor, migrating it to a
configuration file would now be far more straightforward than before.

LLLeapListener::newpump(), too, can call LLEventPumps::make() with the
caller-specified type string. This eliminates that static factory map.
newpump() must catch BadType and report the error back to its invoker.

Given that the LLEventPump subclass instances returned by make() are owned by
LLEventPumps rather than LLLeapListener, there is no further need for the
LLLeapListener::mEventPumps ptr_map, which was used only to manage lifetime.
Also remove LLLeapListener's "killpump" operation since LLEventPumps provides
no corresponding functionality.

4 files changed, 95 insertions, 82 deletions

diff --git a/indra/llcommon/llevents.cpp b/indra/llcommon/llevents.cpp
index d31f5f2d32..281a1121bd 100644
--- a/indra/llcommon/llevents.cpp
+++ b/indra/llcommon/llevents.cpp
@@ -66,14 +66,21 @@
 /*****************************************************************************
 *   LLEventPumps
 *****************************************************************************/
-LLEventPumps::PumpFactories LLEventPumps::mFactories
-{
-    // LLEventStream is the default for obtain(), so even if somebody DOES
-    // call obtain("placeholder"), this sample entry won't break anything.
-    { "placeholder", [](const std::string& name) { return new LLEventStream(name); } }
-};
-
-LLEventPumps::LLEventPumps() {}
+LLEventPumps::LLEventPumps():
+    mFactories
+    {
+        { "LLEventStream",   [](const std::string& name, bool tweak)
+                             { return new LLEventStream(name, tweak); } },
+        { "LLEventMailDrop", [](const std::string& name, bool tweak)
+                             { return new LLEventMailDrop(name, tweak); } }
+    },
+    mTypes
+    {
+        // LLEventStream is the default for obtain(), so even if somebody DOES
+        // call obtain("placeholder"), this sample entry won't break anything.
+        { "placeholder", "LLEventStream" }
+    }
+{}
 
 LLEventPump& LLEventPumps::obtain(const std::string& name)
 {
@@ -84,14 +91,31 @@ LLEventPump& LLEventPumps::obtain(const std::string& name)
         // name.
         return *found->second;
     }
-    // Here we must instantiate an LLEventPump subclass. 
-    LLEventPump* newInstance;
-    // Do we have a predefined factory for this instance name?
-    PumpFactories::const_iterator nfound = mFactories.find(name);
-    if (nfound != mFactories.end())
-        newInstance = (nfound->second)(name);
-    else
-        newInstance = new LLEventStream(name);
+
+    // Here we must instantiate an LLEventPump subclass. Is there a
+    // preregistered class name override for this specific instance name?
+    auto nfound = mTypes.find(name);
+    std::string type;
+    if (nfound != mTypes.end())
+    {
+        type = nfound->second;
+    }
+    // pass tweak=false: we already know there's no existing instance with
+    // this name
+    return make(name, false, type);
+}
+
+LLEventPump& LLEventPumps::make(const std::string& name, bool tweak,
+                                const std::string& type)
+{
+    // find the relevant factory for this (or default) type
+    auto found = mFactories.find(type.empty()? "LLEventStream" : type);
+    if (found == mFactories.end())
+    {
+        // Passing an unrecognized type name is a no-no
+        LLTHROW(BadType(type));
+    }
+    auto newInstance = (found->second)(name, tweak);
     // LLEventPump's constructor implicitly registers each new instance in
     // mPumpMap. But remember that we instantiated it (in mOurPumps) so we'll
     // delete it later.
diff --git a/indra/llcommon/llevents.h b/indra/llcommon/llevents.h
index c55351919e..e380c108f4 100644
--- a/indra/llcommon/llevents.h
+++ b/indra/llcommon/llevents.h
@@ -211,8 +211,7 @@ public:
     /// exception if you try to call when empty
     struct Empty: public LLException
     {
-        Empty(const std::string& what):
-            LLException(std::string("LLListenerOrPumpName::Empty: ") + what) {}
+        Empty(const std::string& what): LLException("LLListenerOrPumpName::Empty: " + what) {}
     };
 
 private:
@@ -247,6 +246,30 @@ public:
      */
     LLEventPump& obtain(const std::string& name);
 
+    /// exception potentially thrown by make()
+    struct BadType: public LLException
+    {
+        BadType(const std::string& what): LLException("BadType: " + what) {}
+    };
+
+    /**
+     * Create an LLEventPump with suggested name (optionally of specified
+     * LLEventPump subclass type). As with obtain(), LLEventPumps owns the new
+     * instance.
+     *
+     * As with LLEventPump's constructor, make() could throw
+     * LLEventPump::DupPumpName unless you pass tweak=true.
+     *
+     * As with a hand-constructed LLEventPump subclass, if you pass
+     * tweak=true, the tweaked name can be obtained by LLEventPump::getName().
+     *
+     * Pass empty type to get the default LLEventStream.
+     *
+     * If you pass an unrecognized type string, make() throws BadType.
+     */
+    LLEventPump& make(const std::string& name, bool tweak=false,
+                      const std::string& type=std::string());
+
     /**
      * Find the named LLEventPump instance. If it exists post the message to it.
      * If the pump does not exist, do nothing.
@@ -303,10 +326,19 @@ testable:
     // destroyed.
     typedef std::set<LLEventPump*> PumpSet;
     PumpSet mOurPumps;
-    // LLEventPump subclasses that should be instantiated for particular
-    // instance names
-    typedef std::map<std::string, std::function<LLEventPump*(const std::string&)>> PumpFactories;
-    static PumpFactories mFactories;
+    // for make(), map string type name to LLEventPump subclass factory function
+    typedef std::map<std::string, std::function<LLEventPump*(const std::string&, bool)>> PumpFactories;
+    // Data used by make().
+    // One might think mFactories and mTypes could reasonably be static. So
+    // they could -- if not for the fact that make() or obtain() might be
+    // called before this module's static variables have been initialized.
+    // This is why we use singletons in the first place.
+    PumpFactories mFactories;
+
+    // for obtain(), map desired string instance name to string type when
+    // obtain() must create the instance
+    typedef std::map<std::string, std::string> InstanceTypes;
+    InstanceTypes mTypes;
 };
 
 /*****************************************************************************
@@ -372,8 +404,7 @@ public:
      */
     struct DupPumpName: public LLException
     {
-        DupPumpName(const std::string& what):
-            LLException(std::string("DupPumpName: ") + what) {}
+        DupPumpName(const std::string& what): LLException("DupPumpName: " + what) {}
     };
 
     /**
@@ -409,9 +440,7 @@ public:
      */
     struct DupListenerName: public ListenError
     {
-        DupListenerName(const std::string& what):
-            ListenError(std::string("DupListenerName: ") + what)
-        {}
+        DupListenerName(const std::string& what): ListenError("DupListenerName: " + what) {}
     };
     /**
      * exception thrown by listen(). The order dependencies specified for your
@@ -423,7 +452,7 @@ public:
      */
     struct Cycle: public ListenError
     {
-        Cycle(const std::string& what): ListenError(std::string("Cycle: ") + what) {}
+        Cycle(const std::string& what): ListenError("Cycle: " + what) {}
     };
     /**
      * exception thrown by listen(). This one means that your new listener
@@ -444,7 +473,7 @@ public:
      */
     struct OrderChange: public ListenError
     {
-        OrderChange(const std::string& what): ListenError(std::string("OrderChange: ") + what) {}
+        OrderChange(const std::string& what): ListenError("OrderChange: " + what) {}
     };
 
     /// used by listen()
diff --git a/indra/llcommon/llleaplistener.cpp b/indra/llcommon/llleaplistener.cpp
index 0d18e5fff9..3e6ce9092c 100644
--- a/indra/llcommon/llleaplistener.cpp
+++ b/indra/llcommon/llleaplistener.cpp
@@ -62,16 +62,11 @@ LLLeapListener::LLLeapListener(const ConnectFunc& connect):
     LLSD need_name(LLSDMap("name", LLSD()));
     add("newpump",
         "Instantiate a new LLEventPump named like [\"name\"] and listen to it.\n"
-        "If [\"type\"] == \"LLEventQueue\", make LLEventQueue, else LLEventStream.\n"
+        "[\"type\"] == \"LLEventStream\", \"LLEventMailDrop\" et al.\n"
         "Events sent through new LLEventPump will be decorated with [\"pump\"]=name.\n"
         "Returns actual name in [\"name\"] (may be different if collision).",
         &LLLeapListener::newpump,
         need_name);
-    add("killpump",
-        "Delete LLEventPump [\"name\"] created by \"newpump\".\n"
-        "Returns [\"status\"] boolean indicating whether such a pump existed.",
-        &LLLeapListener::killpump,
-        need_name);
     LLSD need_source_listener(LLSDMap("source", LLSD())("listener", LLSD()));
     add("listen",
         "Listen to an existing LLEventPump named [\"source\"], with listener name\n"
@@ -121,58 +116,28 @@ LLLeapListener::~LLLeapListener()
     }
 }
 
-namespace
-{
-
-static std::map<std::string, std::function<LLEventPump*(const std::string&)>> factories
-{
-    // tweak name for uniqueness
-    { "LLEventStream",   [](const std::string& name){ return new LLEventStream(name, true); } },
-    { "LLEventMailDrop", [](const std::string& name){ return new LLEventMailDrop(name, true); } }
-};
-
-} // anonymous namespace
-
 void LLLeapListener::newpump(const LLSD& request)
 {
     Response reply(LLSD(), request);
 
     std::string name = request["name"];
-    LLSD const & type = request["type"];
+    std::string type = request["type"];
 
-    LLEventPump * new_pump = NULL;
-    auto found = factories.find(type.asString());
-    if (found != factories.end())
+    try
     {
-        new_pump = (found->second)(name);
+        // tweak name for uniqueness
+        LLEventPump& new_pump(LLEventPumps::instance().make(name, true, type));
+        name = new_pump.getName();
+        reply["name"] = name;
+
+        // Now listen on this new pump with our plugin listener
+        std::string myname("llleap");
+        saveListener(name, myname, mConnect(new_pump, myname));
     }
-    else
+    catch (const LLEventPumps::BadType& error)
     {
-        if (! type.isUndefined())
-        {
-            reply.warn(STRINGIZE("unknown 'type' " << type << ", using LLEventStream"));
-        }
-        new_pump = new LLEventStream(name, true); // tweak name for uniqueness
+        reply.error(error.what());
     }
-
-    name = new_pump->getName();
-
-    mEventPumps.insert(name, new_pump);
-
-    // Now listen on this new pump with our plugin listener
-    std::string myname("llleap");
-    saveListener(name, myname, mConnect(*new_pump, myname));
-
-    reply["name"] = name;
-}
-
-void LLLeapListener::killpump(const LLSD& request)
-{
-    Response reply(LLSD(), request);
-
-    std::string name = request["name"];
-    // success == (nonzero number of entries were erased)
-    reply["status"] = bool(mEventPumps.erase(name));
 }
 
 void LLLeapListener::listen(const LLSD& request)
diff --git a/indra/llcommon/llleaplistener.h b/indra/llcommon/llleaplistener.h
index 2193d81b9e..0ca5893657 100644
--- a/indra/llcommon/llleaplistener.h
+++ b/indra/llcommon/llleaplistener.h
@@ -40,7 +40,6 @@ public:
 
 private:
     void newpump(const LLSD&);
-    void killpump(const LLSD&);
     void listen(const LLSD&);
     void stoplistening(const LLSD&);
     void ping(const LLSD&) const;
@@ -64,10 +63,6 @@ private:
     // and listener name.
     typedef std::map<std::pair<std::string, std::string>, LLBoundListener> ListenersMap;
     ListenersMap mListeners;
-    // Similar lifespan reasoning applies to LLEventPumps instantiated by
-    // newpump() operations.
-    typedef boost::ptr_map<std::string, LLEventPump> EventPumpsMap;
-    EventPumpsMap mEventPumps;
 };
 
 #endif /* ! defined(LL_LLLEAPLISTENER_H) */