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.cpp | 89 ++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 54 deletions(-) (limited to 'indra/newview/llversioninfo.cpp') diff --git a/indra/newview/llversioninfo.cpp b/indra/newview/llversioninfo.cpp index 4e07223784..f4b1f2566d 100644 --- a/indra/newview/llversioninfo.cpp +++ b/indra/newview/llversioninfo.cpp @@ -29,6 +29,7 @@ #include #include #include "llversioninfo.h" +#include "stringize.h" #include #if ! defined(LL_VIEWER_CHANNEL) \ @@ -43,100 +44,81 @@ // Set the version numbers in indra/VIEWER_VERSION // -//static +LLVersionInfo::LLVersionInfo(): + // LL_VIEWER_CHANNEL is a macro defined on the compiler command line. The + // macro expands to the string name of the channel, but without quotes. We + // need to turn it into a quoted string. LL_TO_STRING() does that. + mWorkingChannelName(LL_TO_STRING(LL_VIEWER_CHANNEL)), + build_configuration(LLBUILD_CONFIG), // set in indra/cmake/BuildVersion.cmake + short_version(STRINGIZE(LL_VIEWER_VERSION_MAJOR << "." + << LL_VIEWER_VERSION_MINOR << "." + << LL_VIEWER_VERSION_PATCH)) +{ +} + +void LLVersionInfo::initSingleton() +{ + // We override initSingleton() not because we have dependencies on other + // LLSingletons, but because certain initializations call other member + // functions. We should refrain from calling methods until this object is + // fully constructed; such calls don't really belong in the constructor. + + // cache the version string + version = STRINGIZE(getShortVersion() << "." << getBuild()); +} + S32 LLVersionInfo::getMajor() { return LL_VIEWER_VERSION_MAJOR; } -//static S32 LLVersionInfo::getMinor() { return LL_VIEWER_VERSION_MINOR; } -//static S32 LLVersionInfo::getPatch() { return LL_VIEWER_VERSION_PATCH; } -//static S32 LLVersionInfo::getBuild() { return LL_VIEWER_VERSION_BUILD; } -//static -const std::string &LLVersionInfo::getVersion() +std::string LLVersionInfo::getVersion() { - static std::string version(""); - if (version.empty()) - { - std::ostringstream stream; - stream << LLVersionInfo::getShortVersion() << "." << LLVersionInfo::getBuild(); - // cache the version string - version = stream.str(); - } return version; } -//static -const std::string &LLVersionInfo::getShortVersion() +std::string LLVersionInfo::getShortVersion() { - static std::string short_version(""); - if(short_version.empty()) - { - // cache the version string - std::ostringstream stream; - stream << LL_VIEWER_VERSION_MAJOR << "." - << LL_VIEWER_VERSION_MINOR << "." - << LL_VIEWER_VERSION_PATCH; - short_version = stream.str(); - } return short_version; } -namespace -{ - // LL_VIEWER_CHANNEL is a macro defined on the compiler command line. The - // macro expands to the string name of the channel, but without quotes. We - // need to turn it into a quoted string. LL_TO_STRING() does that. - /// Storage of the channel name the viewer is using. - // The channel name is set by hardcoded constant, - // or by calling LLVersionInfo::resetChannel() - std::string sWorkingChannelName(LL_TO_STRING(LL_VIEWER_CHANNEL)); - - // Storage for the "version and channel" string. - // This will get reset too. - std::string sVersionChannel(""); -} - -//static -const std::string &LLVersionInfo::getChannelAndVersion() +std::string LLVersionInfo::getChannelAndVersion() { - if (sVersionChannel.empty()) + if (mVersionChannel.empty()) { // cache the version string - sVersionChannel = LLVersionInfo::getChannel() + " " + LLVersionInfo::getVersion(); + mVersionChannel = getChannel() + " " + getVersion(); } - return sVersionChannel; + return mVersionChannel; } -//static -const std::string &LLVersionInfo::getChannel() +std::string LLVersionInfo::getChannel() { - return sWorkingChannelName; + return mWorkingChannelName; } void LLVersionInfo::resetChannel(const std::string& channel) { - sWorkingChannelName = channel; - sVersionChannel.clear(); // Reset version and channel string til next use. + mWorkingChannelName = channel; + mVersionChannel.clear(); // Reset version and channel string til next use. } -//static LLVersionInfo::ViewerMaturity LLVersionInfo::getViewerMaturity() { ViewerMaturity maturity; @@ -175,8 +157,7 @@ LLVersionInfo::ViewerMaturity LLVersionInfo::getViewerMaturity() } -const std::string &LLVersionInfo::getBuildConfig() +std::string LLVersionInfo::getBuildConfig() { - static const std::string build_configuration(LLBUILD_CONFIG); // set in indra/cmake/BuildVersion.cmake return build_configuration; } -- 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.cpp | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) (limited to 'indra/newview/llversioninfo.cpp') diff --git a/indra/newview/llversioninfo.cpp b/indra/newview/llversioninfo.cpp index f4b1f2566d..4720a989b0 100644 --- a/indra/newview/llversioninfo.cpp +++ b/indra/newview/llversioninfo.cpp @@ -26,8 +26,8 @@ */ #include "llviewerprecompiledheaders.h" -#include -#include +#include "llevents.h" +#include "lleventfilter.h" #include "llversioninfo.h" #include "stringize.h" #include @@ -45,14 +45,19 @@ // LLVersionInfo::LLVersionInfo(): + short_version(STRINGIZE(LL_VIEWER_VERSION_MAJOR << "." + << LL_VIEWER_VERSION_MINOR << "." + << LL_VIEWER_VERSION_PATCH)), // LL_VIEWER_CHANNEL is a macro defined on the compiler command line. The // macro expands to the string name of the channel, but without quotes. We // need to turn it into a quoted string. LL_TO_STRING() does that. mWorkingChannelName(LL_TO_STRING(LL_VIEWER_CHANNEL)), build_configuration(LLBUILD_CONFIG), // set in indra/cmake/BuildVersion.cmake - short_version(STRINGIZE(LL_VIEWER_VERSION_MAJOR << "." - << LL_VIEWER_VERSION_MINOR << "." - << LL_VIEWER_VERSION_PATCH)) + // instantiate an LLEventMailDrop with canonical name to listen for news + // from SLVersionChecker + mPump{new LLEventMailDrop("relnotes")}, + // immediately listen on mPump, store arriving URL into mReleaseNotes + mStore{new LLStoreListener(*mPump, mReleaseNotes)} { } @@ -67,6 +72,10 @@ void LLVersionInfo::initSingleton() version = STRINGIZE(getShortVersion() << "." << getBuild()); } +LLVersionInfo::~LLVersionInfo() +{ +} + S32 LLVersionInfo::getMajor() { return LL_VIEWER_VERSION_MAJOR; @@ -161,3 +170,8 @@ std::string LLVersionInfo::getBuildConfig() { return build_configuration; } + +std::string LLVersionInfo::getReleaseNotes() +{ + return mReleaseNotes; +} -- cgit v1.2.3