code review

2 files changed, 24 insertions, 47 deletions

diff --git a/platform/chromium/vapi-background.js b/platform/chromium/vapi-background.js
index f3c7ff3..1fb82e7 100644
--- a/platform/chromium/vapi-background.js
+++ b/platform/chromium/vapi-background.js
@@ -983,19 +983,8 @@ vAPI.cloud = (function() {
     var maxStorageSize = chrome.storage.sync.QUOTA_BYTES;
 
     var options = {
-        deviceName: ''
-    };
-
-    var getDeviceName = function() {
-        // Assign a permanent user-friendly id to this uBlock instance if one does
-        // not exist. This will allow to have some sort of identifier for a user
-        // to possibly identify the source of cloud data.
-        var name = window.localStorage.getItem('deviceName') || '';
-        if ( name !== '' ) {
-            return name;
-        }
-
-        return window.navigator.platform;
+        defaultDeviceName: window.navigator.platform,
+        deviceName: window.localStorage.getItem('deviceName') || ''
     };
 
     // This is used to find out a rough count of how many chunks exists:
@@ -1004,6 +993,7 @@ vAPI.cloud = (function() {
     // This allows reading a single item with only 2 sync operations -- a
     // good thing given chrome.storage.syncMAX_WRITE_OPERATIONS_PER_MINUTE
     // and chrome.storage.syncMAX_WRITE_OPERATIONS_PER_HOUR.
+
     var getCoarseChunkCount = function(dataKey, callback) {
         var bin = {};
         for ( var i = 0; i < maxChunkCountPerItem; i += 16 ) {
@@ -1016,9 +1006,6 @@ vAPI.cloud = (function() {
                 return;
             }
 
-            // Could loop backward... let's assume for now
-            // maxChunkCountPerItem could be something else than a
-            // multiple of 16.
             var chunkCount = 0;
             for ( var i = 0; i < maxChunkCountPerItem; i += 16 ) {
                 if ( bin[dataKey + i.toString()] === '' ) {
@@ -1052,7 +1039,7 @@ vAPI.cloud = (function() {
 
     var push = function(dataKey, data, callback) {
         var item = JSON.stringify({
-            'source': getDeviceName(),
+            'source': options.deviceName || options.defaultDeviceName,
             'tstamp': Date.now(),
             'data': data
         });
@@ -1129,7 +1116,6 @@ vAPI.cloud = (function() {
         if ( typeof callback !== 'function' ) {
             return;
         }
-
         callback(options);
     };
 
@@ -1140,11 +1126,10 @@ vAPI.cloud = (function() {
 
         if ( typeof details.deviceName === 'string' ) {
             window.localStorage.setItem('deviceName', details.deviceName);
+            options.deviceName = details.deviceName;
         }
 
-        if ( typeof callback === 'function' ) {
-            callback(options);
-        }
+        getOptions(callback);
     };
 
     return {
diff --git a/platform/firefox/vapi-background.js b/platform/firefox/vapi-background.js
index 7c859ce..75585d7 100644
--- a/platform/firefox/vapi-background.js
+++ b/platform/firefox/vapi-background.js
@@ -2905,35 +2905,28 @@ vAPI.cloud = (function() {
     var cloudBranchPath = extensionBranchPath + '.cloudStorage';
 
     var options = {
+        defaultDeviceName: '',
         deviceName: ''
     };
 
-    var getDeviceName = function() {
-        var name = '';
-
-        // User-supplied device name.
-        var branch = Services.prefs.getBranch(extensionBranchPath + '.');
-        try {
-            name = branch.getCharPref('deviceName');
-        } catch(ex) {
-        }
-        options.deviceName = name;
-        if ( name !== '' ) {
-            return name;
-        }
+    // User-supplied device name.
+    try {
+        options.deviceName = Services.prefs
+                                     .getBranch(extensionBranchPath + '.')
+                                     .getCharPref('deviceName');
+    } catch(ex) {
+    }
 
-        // No name: try to use device name specified by user in Preferences.
-        branch = Services.prefs.getBranch('services.sync.client.');
+    var getDefaultDeviceName = function() {
+        var name = '';
         try {
-            name = branch.getCharPref('name');
+            name = Services.prefs
+                           .getBranch('services.sync.client.')
+                           .getCharPref('name');
         } catch(ex) {
         }
-        if ( name !== '' ) {
-            return name;
-        }
 
-        // No name: use os/cpu.
-        return window.navigator.platform || window.navigator.oscpu;
+        return name || window.navigator.platform || window.navigator.oscpu;
     };
 
     var start = function(dataKeys) {
@@ -2954,7 +2947,7 @@ vAPI.cloud = (function() {
     var push = function(datakey, data, callback) {
         var branch = Services.prefs.getBranch(cloudBranchPath + '.');
         var bin = {
-            'source': getDeviceName(),
+            'source': options.deviceName || getDefaultDeviceName(),
             'tstamp': Date.now(),
             'data': data
         };
@@ -2981,7 +2974,7 @@ vAPI.cloud = (function() {
         if ( typeof callback !== 'function' ) {
             return;
         }
-
+        options.defaultDeviceName = getDefaultDeviceName();
         callback(options);
     };
 
@@ -2994,11 +2987,10 @@ vAPI.cloud = (function() {
 
         if ( typeof details.deviceName === 'string' ) {
             branch.setCharPref('deviceName', details.deviceName);
+            options.deviceName = details.deviceName;
         }
 
-        if ( typeof callback === 'function' ) {
-            callback(options);
-        }
+        getOptions(callback);
     };
 
     return {