From 5a260e0cc3beec45da1d29578855524977206022 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 30 May 2019 10:39:37 -0400 Subject: SL-11216: Convert LLVersionInfo to an LLSingleton. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This changeset is meant to exemplify how to convert a "namespace" class whose methods are static -- and whose data are module-static -- to an LLSingleton. LLVersionInfo has no initClass() or cleanupClass() methods, but the general idea is the same. * Derive the class from LLSingleton: class LLSomeSingleton: public LLSingleton { ... }; * Add LLSINGLETON(LLSomeSingleton); in the private section of the class. This usage implies a separate LLSomeSingleton::LLSomeSingleton() definition, as described in indra/llcommon/llsingleton.h. * Move module-scope data in the .cpp file to non-static class members. Change any sVariableName to mVariableName to avoid being outright misleading. * Make static class methods non-static. Remove '//static' comments from method definitions as needed. * For LLVersionInfo specifically, the 'const std::string&' return type was replaced with 'std::string'. Returning a reference to a static or a member, const or otherwise, is an anti-pattern: the interface constrains the implementation, prohibiting possibly later returning a temporary (an expression). * For LLVersionInfo specifically, 'const S32' return type was replaced with simple 'S32'. 'const' is just noise in that usage. * Simple member initialization (e.g. the original initializer expressions for static variables) can be done with member{ value } initializers (no examples here though). * Delete initClass() method. * LLSingleton's forté is of course lazy initialization. It might work to simply delete any calls to initClass(). But if there are side effects that must happen at that moment, replace LLSomeSingleton::initClass() with (void)LLSomeSingleton::instance(); * Most initClass() initialization can be done in the constructor, as would normally be the case. * Initialization that might cause a circular LLSingleton reference should be moved to initSingleton(). Override 'void initSingleton();' should be private. * For LLVersionInfo specifically, certain initialization that used to be lazily performed was made unconditional, due to its low cost. * For LLVersionInfo specifically, certain initialization involved calling methods that have become non-static. This was moved to initSingleton() because, in a constructor body, 'this' does not yet point to the enclosing class. * Delete cleanupClass() method. * There is already a generic LLSingletonBase::deleteAll() call in LLAppViewer::cleanup(). It might work to let this new LLSingleton be cleaned up with all the rest. But if there are side effects that must happen at that moment, replace LLSomeSingleton::cleanupClass() with LLSomeSingleton::deleteSingleton(). That said, much of the benefit of converting to LLSingleton is deleteAll()'s guarantee that cross-LLSingleton dependencies will be properly honored: we're trying to migrate the code base away from the present fragile manual cleanup sequence. * Most cleanupClass() cleanup can be done in the destructor, as would normally be the case. * Cleanup that might throw an exception should be moved to cleanupSingleton(). Override 'void cleanupSingleton();' should be private. * Within LLSomeSingleton methods, remove any existing LLSomeSingleton::methodName() qualification: simple methodName() is better. * In the rest of the code base, convert most LLSomeSingleton::methodName() references to LLSomeSingleton::instance().methodName(). (Prefer instance() to getInstance() because a reference does not admit the possibility of NULL.) * Of course, LLSomeSingleton::ENUM_VALUE can remain unchanged. In general, for many successive references to an LLSingleton instance, it can be useful to capture the instance() as in: auto& versionInfo{LLVersionInfo::instance()}; // ... versionInfo.getVersion() ... We did not do that here only to simplify the code review. The STRINGIZE(expression) macro encapsulates: std::ostringstream out; out << expression; return out.str(); We used that in a couple places. For LLVersionInfo specifically, lllogininstance_test.cpp used to dummy out a couple specific static methods. It's harder to dummy out LLSingleton::instance() references, so we add the real class to that test. --- indra/newview/llversioninfo.h | 47 ++++++++++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 16 deletions(-) (limited to 'indra/newview/llversioninfo.h') diff --git a/indra/newview/llversioninfo.h b/indra/newview/llversioninfo.h index b8b4341385..7857468697 100644 --- a/indra/newview/llversioninfo.h +++ b/indra/newview/llversioninfo.h @@ -30,6 +30,7 @@ #include #include "stdtypes.h" +#include "llsingleton.h" /// /// This API provides version information for the viewer. This @@ -38,42 +39,44 @@ /// viewer code that wants to query the current version should /// use this API. /// -class LLVersionInfo +class LLVersionInfo: public LLSingleton { + LLSINGLETON(LLVersionInfo); + void initSingleton(); public: - /// return the major verion number as an integer - static S32 getMajor(); + /// return the major version number as an integer + S32 getMajor(); - /// return the minor verion number as an integer - static S32 getMinor(); + /// return the minor version number as an integer + S32 getMinor(); - /// return the patch verion number as an integer - static S32 getPatch(); + /// return the patch version number as an integer + S32 getPatch(); /// return the build number as an integer - static S32 getBuild(); + S32 getBuild(); /// return the full viewer version as a string like "2.0.0.200030" - static const std::string &getVersion(); + std::string getVersion(); /// return the viewer version as a string like "2.0.0" - static const std::string &getShortVersion(); + std::string getShortVersion(); /// return the viewer version and channel as a string /// like "Second Life Release 2.0.0.200030" - static const std::string &getChannelAndVersion(); + std::string getChannelAndVersion(); /// return the channel name, e.g. "Second Life" - static const std::string &getChannel(); + std::string getChannel(); /// return the CMake build type - static const std::string &getBuildConfig(); + std::string getBuildConfig(); /// reset the channel name used by the viewer. - static void resetChannel(const std::string& channel); + void resetChannel(const std::string& channel); /// return the bit width of an address - static const S32 getAddressSize() { return ADDRESS_SIZE; } + S32 getAddressSize() { return ADDRESS_SIZE; } typedef enum { @@ -82,7 +85,19 @@ public: BETA_VIEWER, RELEASE_VIEWER } ViewerMaturity; - static ViewerMaturity getViewerMaturity(); + ViewerMaturity getViewerMaturity(); + +private: + std::string version; + std::string short_version; + /// Storage of the channel name the viewer is using. + // The channel name is set by hardcoded constant, + // or by calling resetChannel() + std::string mWorkingChannelName; + // Storage for the "version and channel" string. + // This will get reset too. + std::string mVersionChannel; + std::string build_configuration; }; #endif -- cgit v1.2.3 From 378e4fae94c9c16b0b9070d7c7d3f63cba8aee94 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 30 May 2019 16:35:45 -0400 Subject: SL-11216: Introduce LLVersionInfo::getReleaseNotes() method. The default string returned by getReleaseNotes() is empty. It must be set by posting the relevant release-notes URL string to a new LLEventMailDrop instance named "relnotes". Add unique_ptr and unique_ptr> to LLVersionInfo -- using unique_ptr to leave those classes opaque to header-file consumers. Introduce an out-of-line destructor to handle the unique_ptr idiom. Initialize the LLEventMailDrop with the desired name; initialize the LLStoreListener with that LLEventMailDrop and the data member returned by getReleaseNotes(). --- indra/newview/llversioninfo.h | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) (limited to 'indra/newview/llversioninfo.h') diff --git a/indra/newview/llversioninfo.h b/indra/newview/llversioninfo.h index 7857468697..02ff0c094a 100644 --- a/indra/newview/llversioninfo.h +++ b/indra/newview/llversioninfo.h @@ -28,9 +28,14 @@ #ifndef LL_LLVERSIONINFO_H #define LL_LLVERSIONINFO_H -#include #include "stdtypes.h" #include "llsingleton.h" +#include +#include + +class LLEventMailDrop; +template +class LLStoreListener; /// /// This API provides version information for the viewer. This @@ -44,6 +49,8 @@ class LLVersionInfo: public LLSingleton LLSINGLETON(LLVersionInfo); void initSingleton(); public: + ~LLVersionInfo(); + /// return the major version number as an integer S32 getMajor(); @@ -87,6 +94,10 @@ public: } ViewerMaturity; ViewerMaturity getViewerMaturity(); + /// get the release-notes URL, once it becomes available -- until then, + /// return empty string + std::string getReleaseNotes(); + private: std::string version; std::string short_version; @@ -98,6 +109,14 @@ private: // This will get reset too. std::string mVersionChannel; std::string build_configuration; + std::string mReleaseNotes; + // Store unique_ptrs to the next couple things so we don't have to explain + // to every consumer of this header file all the details of each. + // mPump is the LLEventMailDrop on which we listen for SLVersionChecker to + // post the release-notes URL from the Viewer Version Manager. + std::unique_ptr mPump; + // mStore is an adapter that stores the release-notes URL in mReleaseNotes. + std::unique_ptr> mStore; }; #endif -- cgit v1.2.3