Bug 1435942 - Fix buggy getters in Preferences.h. r=glandium
authorNicholas Nethercote <nnethercote@mozilla.com>
Wed, 07 Feb 2018 09:11:11 +1100
changeset 402885 caf379dce6b1744e4d0c6cc1388979ad85c6f1a7
parent 402884 9a8f3128efea5228cc18adc890beea3f0b09564a
child 402886 d4caeb6d13373db3aa8719db966c44bec6fdb9b9
push id33405
push usershindli@mozilla.com
push dateThu, 08 Feb 2018 10:04:47 +0000
treeherdermozilla-central@0ac953fcddf1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium
bugs1435942
milestone60.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 1435942 - Fix buggy getters in Preferences.h. r=glandium They currently fail to pass on `aKind`, always getting the user value (when possible). There are three callsites that are affected: - nsSHistory::Startup, docshell/shistory/nsSHistory.cpp. - FeatureState::SetDefaultFromPref(), in gfx/config/gfxFeature.cpp. - gfxPlatform::InitOMTPConfig(), in gfx/thebes/gfxPlatform.cpp. The patch also adds a gtest that would have failed prior to the fix. MozReview-Commit-ID: L0U1XQmPUFc
modules/libpref/Preferences.cpp
modules/libpref/Preferences.h
modules/libpref/test/gtest/Basics.cpp
modules/libpref/test/gtest/moz.build
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -3918,17 +3918,17 @@ Preferences::GetString(const char* aPref
 }
 
 /* static */ nsresult
 Preferences::GetLocalizedCString(const char* aPrefName,
                                  nsACString& aResult,
                                  PrefValueKind aKind)
 {
   nsAutoString result;
-  nsresult rv = GetLocalizedString(aPrefName, result);
+  nsresult rv = GetLocalizedString(aPrefName, result, aKind);
   if (NS_SUCCEEDED(rv)) {
     CopyUTF16toUTF8(result, aResult);
   }
   return rv;
 }
 
 /* static */ nsresult
 Preferences::GetLocalizedString(const char* aPrefName,
--- a/modules/libpref/Preferences.h
+++ b/modules/libpref/Preferences.h
@@ -89,17 +89,18 @@ public:
     NS_ENSURE_TRUE(InitStaticMembers(), nullptr);
     return (aKind == PrefValueKind::Default) ? sPreferences->mDefaultRootBranch
                                              : sPreferences->mRootBranch;
   }
 
   // Gets the type of the pref.
   static int32_t GetType(const char* aPrefName);
 
-  // Fallible value getters.
+  // Fallible value getters. When `aKind` is `User` they will get the user
+  // value if possible, and fall back to the default value otherwise.
   static nsresult GetBool(const char* aPrefName,
                           bool* aResult,
                           PrefValueKind aKind = PrefValueKind::User);
   static nsresult GetInt(const char* aPrefName,
                          int32_t* aResult,
                          PrefValueKind aKind = PrefValueKind::User);
   static nsresult GetUint(const char* aPrefName,
                           uint32_t* aResult,
@@ -124,48 +125,48 @@ public:
                                      nsAString& aResult,
                                      PrefValueKind aKind = PrefValueKind::User);
   static nsresult GetComplex(const char* aPrefName,
                              const nsIID& aType,
                              void** aResult,
                              PrefValueKind aKind = PrefValueKind::User);
 
   // Infallible getters of user or default values, with fallback results on
-  // failure.
-  // Infallible value getters.
+  // failure. When `aKind` is `User` they will get the user value if possible,
+  // and fall back to the default value otherwise.
   static bool GetBool(const char* aPrefName,
                       bool aFallback = false,
                       PrefValueKind aKind = PrefValueKind::User)
   {
     bool result = aFallback;
-    GetBool(aPrefName, &result);
+    GetBool(aPrefName, &result, aKind);
     return result;
   }
   static int32_t GetInt(const char* aPrefName,
                         int32_t aFallback = 0,
                         PrefValueKind aKind = PrefValueKind::User)
   {
     int32_t result = aFallback;
-    GetInt(aPrefName, &result);
+    GetInt(aPrefName, &result, aKind);
     return result;
   }
   static uint32_t GetUint(const char* aPrefName,
                           uint32_t aFallback = 0,
                           PrefValueKind aKind = PrefValueKind::User)
   {
     uint32_t result = aFallback;
-    GetUint(aPrefName, &result);
+    GetUint(aPrefName, &result, aKind);
     return result;
   }
   static float GetFloat(const char* aPrefName,
                         float aFallback = 0.0f,
                         PrefValueKind aKind = PrefValueKind::User)
   {
     float result = aFallback;
-    GetFloat(aPrefName, &result);
+    GetFloat(aPrefName, &result, aKind);
     return result;
   }
 
   // Value setters. These fail if run outside the parent process.
 
   static nsresult SetBool(const char* aPrefName,
                           bool aValue,
                           PrefValueKind aKind = PrefValueKind::User);
new file mode 100644
--- /dev/null
+++ b/modules/libpref/test/gtest/Basics.cpp
@@ -0,0 +1,36 @@
+/* -*- 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 "gtest/gtest.h"
+#include "mozilla/Preferences.h"
+
+using namespace mozilla;
+
+TEST(PrefsBasics, Errors)
+{
+  Preferences::SetBool("foo.bool", true, PrefValueKind::Default);
+  Preferences::SetBool("foo.bool", false, PrefValueKind::User);
+  ASSERT_EQ(Preferences::GetBool("foo.bool", false, PrefValueKind::Default),
+            true);
+  ASSERT_EQ(Preferences::GetBool("foo.bool", true, PrefValueKind::User), false);
+
+  Preferences::SetInt("foo.int", -66, PrefValueKind::Default);
+  Preferences::SetInt("foo.int", -77, PrefValueKind::User);
+  ASSERT_EQ(Preferences::GetInt("foo.int", 1, PrefValueKind::Default), -66);
+  ASSERT_EQ(Preferences::GetInt("foo.int", 1, PrefValueKind::User), -77);
+
+  Preferences::SetUint("foo.uint", 88, PrefValueKind::Default);
+  Preferences::SetUint("foo.uint", 99, PrefValueKind::User);
+  ASSERT_EQ(Preferences::GetUint("foo.uint", 1, PrefValueKind::Default), 88U);
+  ASSERT_EQ(Preferences::GetUint("foo.uint", 1, PrefValueKind::User), 99U);
+
+  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);
+}
--- a/modules/libpref/test/gtest/moz.build
+++ b/modules/libpref/test/gtest/moz.build
@@ -6,16 +6,17 @@
 
 Library('libpreftests')
 
 LOCAL_INCLUDES += [
     '../..',
 ]
 
 UNIFIED_SOURCES = [
+    'Basics.cpp',
     'CallbackAndVarCacheOrder.cpp',
     'Parser.cpp',
 ]
 
 if CONFIG['CC_TYPE'] in ('clang', 'gcc'):
     CXXFLAGS += ['-Wno-error=shadow']
 
 # THE MOCK_METHOD2 macro from gtest triggers this clang warning and it's hard