Issue 5039 - add support of nullable non-object values in settings

lib/prefs.js

Index: lib/prefs.js
diff --git a/lib/prefs.js b/lib/prefs.js
index 14785c4a7f616d224030a84914e2e98ace2efad9..f60e973835cbadeaab7ea07c7051f763c2c0dcc8 100644
--- a/lib/prefs.js
+++ b/lib/prefs.js
@@ -48,6 +48,11 @@ let defaults = {
   notifications_ignoredcategories: [],
 };
 
+let optionalValues_ExpectedType = {

[Oleksandr, 2017/03/28 12:42:08]

Nit: how about optionalValues_ExpectedTypes for better distinction below.

[sergei, 2017/03/28 14:31:41]

it's a bit difficult naming case, done.

+  __proto__: null,
+  allowed_connection_type: "string"
+};
+
 let preconfigurable = ["suppress_first_run_page", "disable_auto_updates",
   "first_run_subscription_auto_select", "allowed_connection_type"];
 
@@ -57,6 +62,25 @@ let listeners = [];
 let isDirty = false;
 let isSaving = false;
 
+function isValueTypeCorrect(key, value)
+{
+  // For values of required settings the first line just works.
+  // For values of optional settings it works when the type of default
+  // value is the same as the type of value. It happens when
+  // - value is undefined and no default value
+  // - value is not undefined and there is a default value.
+  // However, for optional values types are different when
+  // - value is undefined and there is a default value
+  // - value is not undefined and there is no default value.
+  let isGoodValueType = typeof value == typeof defaults[key];
+  let optionalValue_ExpectedType = optionalValues_ExpectedType[key];
+  if (!isGoodValueType && optionalValue_ExpectedType)
+  {
+    isGoodValueType = value == undefined || typeof value == optionalValue_ExpectedType;
+  }
+  return isGoodValueType;
+}
+
 function defineProperty(key)
 {
   Object.defineProperty(Prefs, key,
@@ -64,10 +88,10 @@ function defineProperty(key)
     get: () => values[key],
     set: function(value)
     {
-      if (typeof value != typeof defaults[key])
+      if (!isValueTypeCorrect(key, value))

[Oleksandr, 2017/03/28 12:42:08]

It looks like this is done only for one property allowed_connection_type. Maybe
there's no need to go too generic and just check the specific key and typeof
value here?

         throw new Error("Attempt to change preference type");
 
-      if (value == defaults[key])
+      if (value == defaults[key] || value == undefined)
         delete values[key];
       else
         values[key] = value;
@@ -141,12 +165,26 @@ let Prefs = exports.Prefs = {
 // Update the default prefs with what was preconfigured
 for (let key in _preconfiguredPrefs)
   if (preconfigurable.indexOf(key) != -1)
-    defaults[key] = _preconfiguredPrefs[key];
+  {
+    let value = _preconfiguredPrefs[key];
+    if (!isValueTypeCorrect(key, value))
+      throw new Error("Unexpected value type in preconfigured preferences");
+
+    if (value == undefined)
+      delete defaults[key];
+    else
+      defaults[key] = value;
+  }
 
 // Define defaults
 for (let key in defaults)
   defineProperty(key);
 
+for (let key in optionalValues_ExpectedType)
+  // only those which are not defined yet
+  if (!Object.prototype.hasOwnProperty.call(defaults, key))
+    defineProperty(key);
+
 // Set values of prefs based on defaults
 values = Object.create(defaults);