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

lib/prefs.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 28 matching lines @@
   update_last_error: 0,
   update_soft_expiration: 0,
   update_hard_expiration: 0,
   currentVersion: "0.0",
   notificationdata: {},
   notificationurl: "https://notification.adblockplus.org/notification.json",
   suppress_first_run_page: false,
   disable_auto_updates: false,
   first_run_subscription_auto_select: true,
   notifications_ignoredcategories: [],
+  allowed_connection_type: null
 };
 
-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.

+// It is for non-objects, e.g. for string, number, etc.
+// Don't put here preferences used by adblockpluscore.
+let nullableValues_ExpectedTypes = {
   __proto__: null,
   allowed_connection_type: "string"
 };
 
 let preconfigurable = ["suppress_first_run_page", "disable_auto_updates",
   "first_run_subscription_auto_select", "allowed_connection_type"];
 
 let values;
 let path = _fileSystem.resolve("prefs.json");
 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;
+  let nullableValue_ExpectedType = nullableValues_ExpectedTypes[key];
+  return !nullableValue_ExpectedType ?
+    typeof value == typeof defaults[key] :
+    value === null || typeof value == nullableValue_ExpectedType;
 }
 
 function defineProperty(key)
 {
   Object.defineProperty(Prefs, key,
   {
     get: () => values[key],
     set: function(value)
     {
       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] || value == undefined)
+      if (value == defaults[key])
         delete values[key];
       else
         values[key] = value;
       save();
 
       for (let listener of listeners)
         listener(key);
     },
     enumerable: true
   });
@@ skipping 58 matching lines @@
 };
 
 // Update the default prefs with what was preconfigured
 for (let key in _preconfiguredPrefs)
   if (preconfigurable.indexOf(key) != -1)
   {
     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;
+    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);
 
 load();