Bug 1715892 - Add thread safety notice and asserts for AppDateTimeFormat; r=platform-i18n-reviewers,dminor

Differential Revision: https://phabricator.services.mozilla.com/D131672

intl/locale/AppDateTimeFormat.cpp

@@ -11,24 +11,28 @@
 #include "mozilla/intl/LocaleService.h"
 #include "OSPreferences.h"
 #include "mozIOSPreferences.h"
+#ifdef DEBUG
+#  include "nsThreadManager.h"
+#endif
 
 namespace mozilla::intl {
 
-nsCString* AppDateTimeFormat::mLocale = nullptr;
+nsCString* AppDateTimeFormat::sLocale = nullptr;
-nsTHashMap<nsCStringHashKey, DateTimeFormat*>* AppDateTimeFormat::mFormatCache;
+nsTHashMap<nsCStringHashKey, DateTimeFormat*>* AppDateTimeFormat::sFormatCache;
 
 static const int32_t DATETIME_FORMAT_INITIAL_LEN = 127;
 
 /*static*/
 nsresult AppDateTimeFormat::Initialize() {
-  if (mLocale) {
+  MOZ_ASSERT(NS_IsMainThread());
+  if (sLocale) {
     return NS_OK;
   }
 
-  mLocale = new nsCString();
+  sLocale = new nsCString();
   AutoTArray<nsCString, 10> regionalPrefsLocales;
   LocaleService::GetInstance()->GetRegionalPrefsLocales(regionalPrefsLocales);
-  mLocale->Assign(regionalPrefsLocales[0]);
+  sLocale->Assign(regionalPrefsLocales[0]);
 
   return NS_OK;
 }
@@ -72,12 +76,12 @@ nsresult AppDateTimeFormat::Format(const DateTimeFormat::ComponentsBag& aBag,
   nsAutoString timeZoneID;
   BuildTimeZoneString(aExplodedTime->tm_params, timeZoneID);
 
-  auto genResult = DateTimePatternGenerator::TryCreate(mLocale->get());
+  auto genResult = DateTimePatternGenerator::TryCreate(sLocale->get());
   NS_ENSURE_TRUE(genResult.isOk(), NS_ERROR_FAILURE);
   auto dateTimePatternGenerator = genResult.unwrap();
 
   auto result = DateTimeFormat::TryCreateFromComponents(
-      *mLocale, aBag, dateTimePatternGenerator.get(), Some(timeZoneID));
+      *sLocale, aBag, dateTimePatternGenerator.get(), Some(timeZoneID));
   NS_ENSURE_TRUE(result.isOk(), NS_ERROR_FAILURE);
   auto dateTimeFormat = result.unwrap().release();
 
@@ -143,17 +147,17 @@ nsresult AppDateTimeFormat::Format(const DateTimeFormat::StyleBag& aStyle,
     key.AppendInt(aTimeParameters->tp_dst_offset);
   }
 
-  if (mFormatCache && mFormatCache->Count() == kMaxCachedFormats) {
+  if (sFormatCache && sFormatCache->Count() == kMaxCachedFormats) {
     // Don't allow a pathological page to extend the cache unreasonably.
     NS_WARNING("flushing DateTimeFormat cache");
     DeleteCache();
   }
-  if (!mFormatCache) {
+  if (!sFormatCache) {
-    mFormatCache =
+    sFormatCache =
         new nsTHashMap<nsCStringHashKey, DateTimeFormat*>(kMaxCachedFormats);
   }
 
-  DateTimeFormat*& dateTimeFormat = mFormatCache->LookupOrInsert(key);
+  DateTimeFormat*& dateTimeFormat = sFormatCache->LookupOrInsert(key);
 
   if (!dateTimeFormat) {
     // We didn't have a cached formatter for this key, so create one.
@@ -187,7 +191,7 @@ nsresult AppDateTimeFormat::Format(const DateTimeFormat::StyleBag& aStyle,
 
     nsAutoCString str;
     rv = OSPreferences::GetInstance()->GetDateTimePattern(
-        dateFormatStyle, timeFormatStyle, nsDependentCString(mLocale->get()),
+        dateFormatStyle, timeFormatStyle, nsDependentCString(sLocale->get()),
         str);
     NS_ENSURE_SUCCESS(rv, rv);
     nsAutoString pattern = NS_ConvertUTF8toUTF16(str);
@@ -200,7 +204,7 @@ nsresult AppDateTimeFormat::Format(const DateTimeFormat::StyleBag& aStyle,
           Some(Span<const char16_t>(timeZoneID.Data(), timeZoneID.Length()));
     }
 
-    auto result = DateTimeFormat::TryCreateFromPattern(*mLocale, pattern,
+    auto result = DateTimeFormat::TryCreateFromPattern(*sLocale, pattern,
                                                        timeZoneOverride);
     NS_ENSURE_TRUE(result.isOk(), NS_ERROR_FAILURE);
     dateTimeFormat = result.unwrap().release();
@@ -233,19 +237,21 @@ void AppDateTimeFormat::BuildTimeZoneString(
 
 /*static*/
 void AppDateTimeFormat::DeleteCache() {
-  if (mFormatCache) {
+  MOZ_ASSERT(NS_IsMainThread());
-    for (const auto& dateTimeFormat : mFormatCache->Values()) {
+  if (sFormatCache) {
+    for (const auto& dateTimeFormat : sFormatCache->Values()) {
       delete dateTimeFormat;
     }
-    delete mFormatCache;
+    delete sFormatCache;
-    mFormatCache = nullptr;
+    sFormatCache = nullptr;
   }
 }
 
 /*static*/
 void AppDateTimeFormat::Shutdown() {
+  MOZ_ASSERT(NS_IsMainThread());
   DeleteCache();
-  delete mLocale;
+  delete sLocale;
 }
 
 }  // namespace mozilla::intl

intl/locale/AppDateTimeFormat.h

@@ -20,6 +20,9 @@ namespace mozilla::intl {
  * Get a DateTimeFormat for use in Gecko. This specialized DateTimeFormat
  * respects the user's OS and app preferences, and provides caching of the
  * underlying mozilla::intl resources.
+ *
+ * This class is not thread-safe as it lazily initializes a cache without
+ * any type of multi-threaded protections.
  */
 class AppDateTimeFormat {
  public:
@@ -72,8 +75,8 @@ class AppDateTimeFormat {
   static void BuildTimeZoneString(const PRTimeParameters& aTimeParameters,
                                   nsAString& aStringOut);
 
-  static nsCString* mLocale;
+  static nsCString* sLocale;
-  static nsTHashMap<nsCStringHashKey, DateTimeFormat*>* mFormatCache;
+  static nsTHashMap<nsCStringHashKey, DateTimeFormat*>* sFormatCache;
 };
 
 }  // namespace mozilla::intl

intl/locale/tests/gtest/TestAppDateTimeFormat.cpp

@@ -21,7 +21,7 @@ TEST(AppDateTimeFormat, FormatPRExplodedTime)
   PRExplodedTime prExplodedTime;
   PR_ExplodeTime(prTime, PR_GMTParameters, &prExplodedTime);
 
-  AppDateTimeFormat::mLocale = new nsCString("en-US");
+  AppDateTimeFormat::sLocale = new nsCString("en-US");
   AppDateTimeFormat::DeleteCache();
   StyleBag style = ToStyleBag(Some(Style::Long), Some(Style::Long));
 
@@ -99,7 +99,7 @@ TEST(AppDateTimeFormat, DateFormatSelectors)
   PRExplodedTime prExplodedTime;
   PR_ExplodeTime(prTime, PR_GMTParameters, &prExplodedTime);
 
-  AppDateTimeFormat::mLocale = new nsCString("en-US");
+  AppDateTimeFormat::sLocale = new nsCString("en-US");
   AppDateTimeFormat::DeleteCache();
 
   nsAutoString formattedTime;
@@ -150,7 +150,7 @@ TEST(AppDateTimeFormat, FormatPRExplodedTimeForeign)
   PRExplodedTime prExplodedTime;
   PR_ExplodeTime(prTime, PR_GMTParameters, &prExplodedTime);
 
-  AppDateTimeFormat::mLocale = new nsCString("de-DE");
+  AppDateTimeFormat::sLocale = new nsCString("de-DE");
   AppDateTimeFormat::DeleteCache();
   StyleBag style = ToStyleBag(Some(Style::Long), Some(Style::Long));
 
@@ -230,7 +230,7 @@ TEST(AppDateTimeFormat, DateFormatSelectorsForeign)
   PRExplodedTime prExplodedTime;
   PR_ExplodeTime(prTime, PR_GMTParameters, &prExplodedTime);
 
-  AppDateTimeFormat::mLocale = new nsCString("de-DE");
+  AppDateTimeFormat::sLocale = new nsCString("de-DE");
   AppDateTimeFormat::DeleteCache();
 
   nsAutoString formattedTime;