Bug 1493955 - Store floating-point preferences in a locale-independent way r=njn
☠☠ backed out by 6cc26ea43938 ☠ ☠
authorGabriele Svelto <gsvelto@mozilla.com>
Thu, 27 Sep 2018 20:41:39 +0000
changeset 438600 ba1fef7b14eb4fd87ae72231196ddf29cd5fc5a6
parent 438599 b3657ab8de7bef220b9e5714fb52d0673eac3ac1
child 438601 a835077cdb631521f58bd833c8b6517cb18b1114
push id34727
push userccoroiu@mozilla.com
push dateFri, 28 Sep 2018 04:31:56 +0000
treeherdermozilla-central@0e5747bb8a98 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnjn
bugs1493955
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1493955 - Store floating-point preferences in a locale-independent way r=njn Differential Revision: https://phabricator.services.mozilla.com/D6796
modules/libpref/Preferences.cpp
modules/libpref/Preferences.h
modules/libpref/test/gtest/Basics.cpp
toolkit/components/antitracking/AntiTrackingCommon.cpp
toolkit/components/extensions/ExtensionPolicyService.cpp
toolkit/components/telemetry/tests/gtest/TestCombinedStacks.cpp
toolkit/recordreplay/ipc/ChildIPC.cpp
xpcom/threads/Scheduler.cpp
xpcom/threads/Scheduler.h
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -2389,16 +2389,17 @@ nsPrefBranch::GetFloatPrefWithDefault(co
 NS_IMETHODIMP
 nsPrefBranch::GetFloatPref(const char* aPrefName, float* aRetVal)
 {
   NS_ENSURE_ARG(aPrefName);
 
   nsAutoCString stringVal;
   nsresult rv = GetCharPref(aPrefName, stringVal);
   if (NS_SUCCEEDED(rv)) {
+    // ToFloat() does a locale-independent conversion.
     *aRetVal = stringVal.ToFloat(&rv);
   }
 
   return rv;
 }
 
 NS_IMETHODIMP
 nsPrefBranch::GetCharPrefWithDefault(const char* aPrefName,
@@ -5030,16 +5031,17 @@ Preferences::GetFloat(const char* aPrefN
                       float* aResult,
                       PrefValueKind aKind)
 {
   MOZ_ASSERT(aResult);
 
   nsAutoCString result;
   nsresult rv = Preferences::GetCString(aPrefName, result, aKind);
   if (NS_SUCCEEDED(rv)) {
+    // ToFloat() does a locale-independent conversion.
     *aResult = result.ToFloat(&rv);
   }
   return rv;
 }
 
 /* static */ nsresult
 Preferences::GetCString(const char* aPrefName,
                         nsACString& aResult,
@@ -5840,17 +5842,19 @@ SetPref_int32_t(const char* aName, int32
                /* isLocked */ false,
                /* fromInit */ true);
 }
 
 static void
 SetPref_float(const char* aName, float aDefaultValue)
 {
   PrefValue value;
-  nsPrintfCString defaultValue("%f", aDefaultValue);
+  // Convert the value in a locale-independent way.
+  nsAutoCString defaultValue;
+  defaultValue.AppendFloat(aDefaultValue);
   value.mStringVal = defaultValue.get();
   pref_SetPref(aName,
                PrefType::String,
                PrefValueKind::Default,
                value,
                /* isSticky */ false,
                /* isLocked */ false,
                /* fromInit */ true);
@@ -5968,18 +5972,17 @@ InitVarCachePref(const nsACString& aName
                     static_cast<int32_t>(aDefaultValue));
   }
   *aCache = aDefaultValue;
   if (aIsStartup) {
     Preferences::AddAtomicUintVarCache(aCache, aName, aDefaultValue, true);
   }
 }
 
-// XXX: this will eventually become used
-MOZ_MAYBE_UNUSED static void
+static void
 InitVarCachePref(const nsACString& aName,
                  float* aCache,
                  float aDefaultValue,
                  bool aIsStartup,
                  bool aSetValue)
 {
   if (aSetValue) {
     SetPref_float(PromiseFlatCString(aName).get(), aDefaultValue);
--- a/modules/libpref/Preferences.h
+++ b/modules/libpref/Preferences.h
@@ -14,17 +14,16 @@
 #include "mozilla/Atomics.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/Result.h"
 #include "mozilla/StaticPtr.h"
 #include "nsCOMPtr.h"
 #include "nsIObserver.h"
 #include "nsIPrefBranch.h"
 #include "nsIPrefService.h"
-#include "nsPrintfCString.h"
 #include "nsString.h"
 #include "nsTArray.h"
 #include "nsWeakReference.h"
 
 class nsIFile;
 
 // The callback function will get passed the pref name which triggered the call
 // and the void* data which was passed to the registered callback function.
@@ -304,17 +303,19 @@ public:
   {
     return SetInt(aPrefName, static_cast<int32_t>(aValue), aKind);
   }
 
   static nsresult SetFloat(const char* aPrefName,
                            float aValue,
                            PrefValueKind aKind = PrefValueKind::User)
   {
-    return SetCString(aPrefName, nsPrintfCString("%f", aValue), aKind);
+    nsAutoCString value;
+    value.AppendFloat(aValue);
+    return SetCString(aPrefName, value, aKind);
   }
 
   static nsresult SetCString(const char* aPrefName,
                              const char* aValue,
                              PrefValueKind aKind = PrefValueKind::User)
   {
     return Preferences::SetCString(
       aPrefName, nsDependentCString(aValue), aKind);
--- a/modules/libpref/test/gtest/Basics.cpp
+++ b/modules/libpref/test/gtest/Basics.cpp
@@ -1,14 +1,16 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#include <locale.h>
+
 #include "gtest/gtest.h"
 #include "mozilla/Preferences.h"
 
 using namespace mozilla;
 
 TEST(PrefsBasics, Errors)
 {
   Preferences::SetBool("foo.bool", true, PrefValueKind::Default);
@@ -29,8 +31,27 @@ TEST(PrefsBasics, Errors)
 
   Preferences::SetFloat("foo.float", 3.33f, PrefValueKind::Default);
   Preferences::SetFloat("foo.float", 4.44f, PrefValueKind::User);
   ASSERT_FLOAT_EQ(
     Preferences::GetFloat("foo.float", 1.0f, PrefValueKind::Default), 3.33f);
   ASSERT_FLOAT_EQ(Preferences::GetFloat("foo.float", 1.0f, PrefValueKind::User),
                   4.44f);
 }
+
+TEST(PrefsBasics, FloatConversions)
+{
+  // Set a global locale that uses the comma as the decimal separator. Since
+  // we can't tell which locales will be available on a machine the tests are
+  // executed only if the locale was set correctly.
+  const char* oldLocale = setlocale(LC_NUMERIC, "nl_NL");
+  if (oldLocale != nullptr) {
+    Preferences::SetFloat("foo.float", 3.33f, PrefValueKind::Default);
+    Preferences::SetFloat("foo.float", 4.44f, PrefValueKind::User);
+    ASSERT_FLOAT_EQ(
+      Preferences::GetFloat("foo.float", 1.0f, PrefValueKind::Default), 3.33f);
+    ASSERT_FLOAT_EQ(
+      Preferences::GetFloat("foo.float", 1.0f, PrefValueKind::User), 4.44f);
+
+    // Restore the original locale
+    setlocale(LC_NUMERIC, oldLocale);
+  }
+}
--- a/toolkit/components/antitracking/AntiTrackingCommon.cpp
+++ b/toolkit/components/antitracking/AntiTrackingCommon.cpp
@@ -25,16 +25,17 @@
 #include "nsIPermissionManager.h"
 #include "nsIPrincipal.h"
 #include "nsIScriptError.h"
 #include "nsIURI.h"
 #include "nsIURL.h"
 #include "nsIWebProgressListener.h"
 #include "nsNetUtil.h"
 #include "nsPIDOMWindow.h"
+#include "nsPrintfCString.h"
 #include "nsScriptSecurityManager.h"
 #include "nsSandboxFlags.h"
 #include "prtime.h"
 
 #define ANTITRACKING_PERM_KEY "3rdPartyStorage"
 #define USER_INTERACTION_PERM "storageAccessAPI"
 
 using namespace mozilla;
--- a/toolkit/components/extensions/ExtensionPolicyService.cpp
+++ b/toolkit/components/extensions/ExtensionPolicyService.cpp
@@ -24,16 +24,17 @@
 #include "nsIChannel.h"
 #include "nsIContentPolicy.h"
 #include "nsIDocShell.h"
 #include "nsIDocument.h"
 #include "nsGlobalWindowOuter.h"
 #include "nsILoadInfo.h"
 #include "nsIXULRuntime.h"
 #include "nsNetUtil.h"
+#include "nsPrintfCString.h"
 #include "nsPIDOMWindow.h"
 #include "nsXULAppAPI.h"
 #include "nsQueryObject.h"
 
 namespace mozilla {
 
 using namespace extensions;
 
--- a/toolkit/components/telemetry/tests/gtest/TestCombinedStacks.cpp
+++ b/toolkit/components/telemetry/tests/gtest/TestCombinedStacks.cpp
@@ -1,14 +1,16 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/
  */
 
 #include "other/CombinedStacks.h"
 #include "other/ProcessedStack.h"
+#include "nsPrintfCString.h"
+
 using namespace mozilla::Telemetry;
 using namespace TelemetryTestHelpers;
 
 TEST_F(TelemetryTestFixture, CombinedStacks) {
 
   const size_t kMaxStacksKept = 10;
   CombinedStacks stacks(kMaxStacksKept);
 
--- a/toolkit/recordreplay/ipc/ChildIPC.cpp
+++ b/toolkit/recordreplay/ipc/ChildIPC.cpp
@@ -18,16 +18,17 @@
 #include "mozilla/dom/ContentChild.h"
 #include "mozilla/layers/ImageDataSerializer.h"
 #include "mozilla/Sprintf.h"
 #include "mozilla/StackWalk.h"
 #include "mozilla/VsyncDispatcher.h"
 
 #include "InfallibleVector.h"
 #include "MemorySnapshot.h"
+#include "nsPrintfCString.h"
 #include "ParentInternal.h"
 #include "ProcessRecordReplay.h"
 #include "ProcessRedirect.h"
 #include "ProcessRewind.h"
 #include "Thread.h"
 #include "Units.h"
 
 #include <algorithm>
--- a/xpcom/threads/Scheduler.cpp
+++ b/xpcom/threads/Scheduler.cpp
@@ -11,17 +11,16 @@
 #include "LeakRefPtr.h"
 #include "MainThreadQueue.h"
 #include "mozilla/CooperativeThreadPool.h"
 #include "mozilla/dom/ScriptSettings.h"
 #include "mozilla/ipc/BackgroundChild.h"
 #include "mozilla/SchedulerGroup.h"
 #include "nsCycleCollector.h"
 #include "nsIThread.h"
-#include "nsPrintfCString.h"
 #include "nsThread.h"
 #include "nsThreadManager.h"
 #include "PrioritizedEventQueue.h"
 #include "xpcpublic.h"
 #include "xpccomponents.h"
 
 // Windows silliness. winbase.h defines an empty no-argument Yield macro.
 #undef Yield
--- a/xpcom/threads/Scheduler.h
+++ b/xpcom/threads/Scheduler.h
@@ -9,16 +9,17 @@
 
 #include "mozilla/Attributes.h"
 #include "mozilla/EventQueue.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/ThreadLocal.h"
 #include "mozilla/UniquePtr.h"
 #include "nsTArray.h"
 #include "nsILabelableRunnable.h"
+#include "nsPrintfCString.h"
 
 // Windows silliness. winbase.h defines an empty no-argument Yield macro.
 #undef Yield
 
 class nsIBlockThreadedExecutionCallback;
 class nsIIdlePeriod;
 class nsThread;