Fix: split up usercss logic from saveStyle

background/background.js

@@ -1,6 +1,6 @@
 /* global dbExec, getStyles, saveStyle */
 /* global handleCssTransitionBug */
-/* global filterUsercss, saveUsercss */
+/* global usercssHelper */
 'use strict';
 
 // eslint-disable-next-line no-var
@@ -304,11 +304,11 @@ function onRuntimeMessage(request, sender, sendResponse) {
       return KEEP_CHANNEL_OPEN;
 
     case 'saveUsercss':
-      saveUsercss(request).then(sendResponse);
+      usercssHelper.save(request, true).then(sendResponse);
       return KEEP_CHANNEL_OPEN;
 
-    case 'filterUsercss':
-      filterUsercss(request).then(sendResponse);
+    case 'buildUsercss':
+      usercssHelper.build(request, true).then(sendResponse);
       return KEEP_CHANNEL_OPEN;
 
     case 'healthCheck':

background/storage.js

@@ -1,5 +1,5 @@
 /* global LZString */
-/* global usercss, openEditor */
+/* global openEditor */
 
 'use strict';
 
@@ -369,75 +369,8 @@ function filterStylesInternal({
 }
 
 
-// Parse the source and find the duplication
-// {id: int, style: object, sourceCode: string, checkDup: boolean}
-function filterUsercss(req) {
-  let style;
-  let pendingBuild;
-  return buildMeta()
-    .then(buildSection)
-    .then(decide)
-    .catch(err => ({status: 'error', error: err.message || String(err)}));
-
-  function buildMeta() {
-    return new Promise(resolve => {
-      if (req.sourceCode) {
-        style = usercss.buildMeta(req.sourceCode);
-      } else {
-        style = req.style;
-      }
-      if (!style.id && req.id) {
-        style.id = req.id;
-      }
-      resolve();
-    });
-  }
-
-  function buildSection() {
-    if (!style.sections || !style.sections.length) {
-      pendingBuild = usercss.buildCode(style);
-    } else {
-      pendingBuild = Promise.resolve(style);
-    }
-  }
-
-  function decide() {
-    // decide result
-    if (!style.id && req.checkDup) {
-      return Promise.all([pendingBuild, findDupUsercss(style)])
-        .then(([, dup]) => ({status: 'success', style, dup}));
-    }
-    return pendingBuild.then(() => ({status: 'success', style}));
-  }
-}
-
-function saveUsercss(style) {
-  // This function use `saveStyle`, however the response is different.
-  return buildMeta()
-    .then(saveStyle)
-    .then(result => ({
-      status: 'success',
-      style: result
-    }))
-    .catch(err => ({
-      status: 'error',
-      error: String(err)
-    }));
-
-  function buildMeta() {
-    return new Promise(resolve => {
-      if (!style.usercssData) {
-        resolve(Object.assign(usercss.buildMeta(style.sourceCode), style));
-        return;
-      }
-      resolve(style);
-    });
-  }
-}
-
-
 function saveStyle(style) {
-  let id = Number(style.id) || null;
+  const id = Number(style.id) || null;
   const reason = style.reason;
   const notify = style.notify !== false;
   delete style.method;
@@ -449,19 +382,17 @@ function saveStyle(style) {
   let existed;
   let codeIsUpdated;
 
-  const maybeProcess = style.usercssData ? processUsercss() : Promise.resolve();
-
-  return maybeProcess
-    .then(maybeUpdate)
+  return maybeCalcDigest()
     .then(maybeImportFix)
     .then(decide);
 
-  function maybeUpdate() {
+  function maybeCalcDigest() {
     if (reason === 'update' || reason === 'update-digest') {
       return calcStyleDigest(style).then(digest => {
         style.originalDigest = digest;
       });
     }
+    return Promise.resolve();
   }
 
   function maybeImportFix() {
@@ -474,23 +405,6 @@ function saveStyle(style) {
     }
   }
 
-  function processUsercss() {
-    return findDupUsercss(style)
-      .then(dup => {
-        if (!dup) {
-          return;
-        }
-        if (!id) {
-          id = dup.id;
-        }
-        if (reason !== 'config') {
-          // preserve style.vars during update
-          usercss.assignVars(style, dup);
-        }
-      })
-      .then(() => usercss.buildCode(style));
-  }
-
   function decide() {
     if (id !== null) {
       // Update or create
@@ -563,21 +477,6 @@ function deleteStyle({id, notify = true}) {
   });
 }
 
-function findDupUsercss(style) {
-  if (style.id) {
-    return getStyles({id: style.id}).then(s => s[0]);
-  }
-  return getStyles().then(styles =>
-    styles.find(target => {
-      if (!target.usercssData) {
-        return false;
-      }
-      return target.usercssData.name === style.usercssData.name &&
-        target.usercssData.namespace === style.usercssData.namespace;
-    })
-  );
-}
-
 
 function getApplicableSections({
   style,

background/update.js

@@ -1,6 +1,6 @@
 /* global getStyles, saveStyle, styleSectionsEqual, chromeLocal */
 /* global calcStyleDigest */
-/* global usercss semverCompare */
+/* global usercss semverCompare usercssHelper */
 'use strict';
 
 // eslint-disable-next-line no-var
@@ -68,6 +68,7 @@ var updater = {
     return (ignoreDigest ? Promise.resolve() : calcStyleDigest(style))
       .then(checkIfEdited)
       .then(maybeUpdate)
+      .then(maybeValidate)
       .then(maybeSave)
       .then(saved => {
         observer(updater.UPDATED, saved);
@@ -114,28 +115,35 @@ var updater = {
         if (semverCompare(version, newVersion) > 0) {
           return Promise.reject(updater.ERROR_VERSION);
         }
-        return json;
+        return usercss.buildCode(json);
       });
     }
 
+    function maybeValidate(json) {
+      if (json.usercssData) {
+        // usercss is already validated while building
+        return json;
+      }
+      if (!styleJSONseemsValid(json)) {
+        return Promise.reject(updater.ERROR_JSON);
+      }
+      if (styleSectionsEqual(json, style)) {
+        // JSONs may have different order of items even if sections are effectively equal
+        // so we'll update the digest anyway
+        saveStyle(Object.assign(json, {reason: 'update-digest'}));
+        return Promise.reject(updater.SAME_CODE);
+      } else if (!style.originalDigest && !ignoreDigest) {
+        return Promise.reject(updater.MAYBE_EDITED);
+      }
+      return json;
+    }
+
     function maybeSave(json) {
+      const doSave = json.usercssData ? usercssHelper.save : saveStyle;
       json.id = style.id;
       // no need to compare section code for usercss, they are built dynamically
-      if (!json.usercssData) {
-        if (!styleJSONseemsValid(json)) {
-          return Promise.reject(updater.ERROR_JSON);
-        }
-        if (styleSectionsEqual(json, style)) {
-          // JSONs may have different order of items even if sections are effectively equal
-          // so we'll update the digest anyway
-          saveStyle(Object.assign(json, {reason: 'update-digest'}));
-          return Promise.reject(updater.SAME_CODE);
-        } else if (!style.originalDigest && !ignoreDigest) {
-          return Promise.reject(updater.MAYBE_EDITED);
-        }
-      }
       return !save ? json :
-        saveStyle(Object.assign(json, {
+        doSave(Object.assign(json, {
           name: null, // keep local name customizations
           reason: 'update',
         }));

background/usercss-helper.js

@@ -0,0 +1,100 @@
+/* global usercss saveStyle getStyles */
+
+'use strict';
+
+// eslint-disable-next-line no-var
+var usercssHelper = (function () {
+  function buildMeta(style) {
+    if (style.usercssData) {
+      return Promise.resolve(style);
+    }
+    try {
+      const {sourceCode} = style;
+      // allow sourceCode to be normalized
+      delete style.sourceCode;
+      return Promise.resolve(Object.assign(usercss.buildMeta(sourceCode), style));
+    } catch (e) {
+      return Promise.reject(e);
+    }
+  }
+
+  function buildCode(style) {
+    return usercss.buildCode(style);
+  }
+
+  function wrapReject(pending) {
+    return pending.then(result => ({status: 'success', result}))
+      .catch(err => {
+        console.error(err);
+        return {status: 'error', result: err.message || String(err)};
+      });
+  }
+
+  // Parse the source and find the duplication
+  // style: {sourceCode: string, checkDup: boolean}
+  function build(request, noReject) {
+    const pending = buildMeta(request)
+      .then(style => Promise.all([buildCode(style), checkDup(style)]))
+      .then(([style, dup]) => ({style, dup}));
+
+    if (noReject) {
+      return wrapReject(pending);
+    }
+    return pending;
+
+    function checkDup(style) {
+      const {checkDup} = style;
+      delete style.checkDup;
+      if (checkDup) {
+        return findDup(style);
+      }
+    }
+  }
+
+  function save(style, noReject) {
+    const pending = buildMeta(style)
+      .then(assignVars)
+      .then(buildCode)
+      .then(saveStyle);
+
+    if (noReject) {
+      return wrapReject(pending);
+    }
+
+    return pending;
+
+    function assignVars(style) {
+      if (style.reason === 'config' && style.id) {
+        return style;
+      }
+      return findDup(style)
+        .then(dup => {
+          if (dup) {
+            style.id = dup.id;
+            if (style.reason !== 'config') {
+              // preserve style.vars during update
+              usercss.assignVars(style, dup);
+            }
+          }
+          return style;
+        });
+    }
+  }
+
+  function findDup(style) {
+    if (style.id) {
+      return getStyles({id: style.id}).then(s => s[0]);
+    }
+    return getStyles().then(styles =>
+      styles.find(target => {
+        if (!target.usercssData) {
+          return false;
+        }
+        return target.usercssData.name === style.usercssData.name &&
+          target.usercssData.namespace === style.usercssData.namespace;
+      })
+    );
+  }
+
+  return {build, save, findDup};
+})();

content/install-user-css.js

@@ -26,13 +26,10 @@ function install(style) {
 
 function runtimeSend(request) {
   return new Promise((resolve, reject) => {
-    chrome.runtime.sendMessage(request, result => {
-      if (result.status === 'error') {
-        reject(result.error);
-      } else {
-        resolve(result);
-      }
-    });
+    chrome.runtime.sendMessage(
+      request,
+      ({status, result}) => (status === 'error' ? reject : resolve)(result)
+    );
   });
 }
 
@@ -156,7 +153,7 @@ function initLiveReload(sourceLoader) {
       }
     });
   });
-  window.addEventListener('installed', ({detail: {style}}) => {
+  window.addEventListener('installed', ({detail: style}) => {
     installed = style;
     if ($('.live-reload-checkbox').checked) {
       watcher.start();
@@ -270,7 +267,7 @@ function initUsercssInstall() {
   sourceLoader.load()
     .then(() =>
       runtimeSend({
-        method: 'filterUsercss',
+        method: 'buildUsercss',
         sourceCode: sourceLoader.source(),
         checkDup: true
       })

edit/source-editor.js

@@ -444,23 +444,14 @@ function createSourceEditor(style) {
     if (!dirty.isDirty()) {
       return;
     }
-    const req = {
-      method: 'saveUsercss',
-      reason: 'editSave',
-      id: style.id,
-      enabled: style.enabled,
-      sourceCode: style.sourceCode
-    };
-    return onBackgroundReady().then(() => BG.saveUsercss(req))
-      .then(result => {
-        if (result.status === 'error') {
-          throw new Error(result.error);
-        }
-        return result;
-      })
-      .then(({style}) => {
-        replaceStyle(style);
-      })
+    return onBackgroundReady()
+      .then(() => BG.usercssHelper.save({
+        reason: 'editSave',
+        id: style.id,
+        enabled: style.enabled,
+        sourceCode: style.sourceCode
+      }))
+      .then(replaceStyle)
       .catch(err => {
         console.error(err);
         alert(err);

manage/manage.js

@@ -302,7 +302,8 @@ Object.assign(handleEvent, {
       for (const key of keys) {
         style.usercssData.vars[key].value = vars[key].value;
       }
-      saveStyleSafe(style);
+      onBackgroundReady()
+        .then(() => BG.usercssHelper.save(style));
     });
   },
 

manifest.json

@@ -24,6 +24,7 @@
       "vendor-overwrites/lz-string/LZString-2xspeedup.js",
       "js/usercss.js",
       "background/storage.js",
+      "background/usercss-helper.js",
       "js/prefs.js",
       "js/script-loader.js",
       "background/background.js",