Issue 4871 - Start using ESLint for adblockplusui

messageResponder.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+/* globals require */
+
 "use strict";
 
+(function(global)
 {
-  if (typeof ext == "undefined")

[Sebastian Noack, 2017/02/21 12:21:21]

For consistency, please refer to: https://codereview.adblockplus.org/29370594/

That way, we also avoid defining a global variable here.

[kzar, 2017/03/01 05:36:20]

I don't think that will work since we can't redeclare ext now that we've used
`let`.

-    window.ext = require("ext_background");
+  let ext = global.ext || require("ext_background");
 
   const {port} = require("messaging");
   const {Prefs} = require("prefs");
   const {Utils} = require("utils");
   const {FilterStorage} = require("filterStorage");
   const {FilterNotifier} = require("filterNotifier");
   const {defaultMatcher} = require("matcher");
   const {ElemHideEmulation} = require("elemHideEmulation");
   const {Notification: NotificationStorage} = require("notification");
 
@@ skipping 38 matching lines @@
                              "lastDownload", "title", "url"], subscription);
     obj.isDownloading = Synchronizer.isExecuting(subscription.url);
     return obj;
   }
 
   let convertFilter = convertObject.bind(null, ["text"]);
 
   let changeListeners = new ext.PageMap();
   let listenedPreferences = Object.create(null);
   let listenedFilterChanges = Object.create(null);
-  let messageTypes = {
-    app: "app.respond",
-    filter: "filters.respond",
-    pref: "prefs.respond",
-    subscription: "subscriptions.respond"
-  };
+  let messageTypes = new Map([
+    ["app", "app.respond"],
+    ["filter", "filters.respond"],
+    ["pref", "prefs.respond"],
+    ["subscription", "subscriptions.respond"]
+  ]);
 
   function sendMessage(type, action, ...args)
   {
     let pages = changeListeners.keys();
     if (pages.length == 0)
       return;
 
-    for (let i = 0; i < args.length; i++)

[Sebastian Noack, 2017/02/21 12:21:21]

If you keep creating a new array here, you could use for..of, resulting into
simpler code. And IIRC, this would also avoid a deoptimization, triggered by
modifying rest arguments.

[kzar, 2017/03/01 05:36:20]

Done.

-    {
-      let arg = args[i];
+    let convertedArgs = [];
+    for (let arg of args)
+    {
       if (arg instanceof Subscription)
-        args[i] = convertSubscription(arg);
+        convertedArgs.push(convertSubscription(arg));
       else if (arg instanceof Filter)
-        args[i] = convertFilter(arg);
+        convertedArgs.push(convertFilter(arg));
+      else
+        convertedArgs.push(arg);
     }
 
     for (let page of pages)
     {
       let filters = changeListeners.get(page);
       let actions = filters[type];
       if (actions && actions.indexOf(action) != -1)
       {
         page.sendMessage({
-          type: messageTypes[type],
+          type: messageTypes.get(type),
           action,
-          args
+          args: convertedArgs
         });
       }
     }
   }
 
   function addFilterListeners(type, actions)
   {
     for (let action of actions)
     {
       let name;
@@ skipping 22 matching lines @@
       changeListeners.set(page, listenerFilters);
     }
     return listenerFilters;
   }
 
   port.on("app.get", (message, sender) =>
   {
     if (message.what == "issues")
     {
       let subscriptionInit = tryRequire("subscriptionInit");
-      return {
-        filterlistsReinitialized: subscriptionInit
-          ? subscriptionInit.reinitialized : false
-      };
+      let result = subscriptionInit ? subscriptionInit.reinitialized : false;
+      return {filterlistsReinitialized: result};
     }
 
     if (message.what == "doclink")
       return Utils.getDocLink(message.link);
 
     if (message.what == "localeInfo")
     {
       let bidiDir;
       if ("chromeRegistry" in Utils)
       {
@@ skipping 54 matching lines @@
     {
       let filters = [];
       const {checkWhitelisted} = require("whitelisting");
 
       if (Prefs.enabled && !checkWhitelisted(sender.page, sender.frame,
                             RegExpFilter.typeMap.DOCUMENT |
                             RegExpFilter.typeMap.ELEMHIDE))
       {
         let {hostname} = sender.frame.url;
         filters = ElemHideEmulation.getRulesForDomain(hostname);
-        filters = filters.map(filter =>
+        filters = filters.map((filter) =>
         {
           return {
             selector: filter.selector,
             text: filter.text
           };
         });
       }
       return filters;
     }
 
@@ skipping 115 matching lines @@
       FilterStorage.addSubscription(subscription);
 
       if (subscription instanceof DownloadableSubscription &&
           !subscription.lastDownload)
         Synchronizer.execute(subscription);
     }
   });
 
   port.on("subscriptions.get", (message, sender) =>
   {
-    let subscriptions = FilterStorage.subscriptions.filter(s =>
+    let subscriptions = FilterStorage.subscriptions.filter((s) =>
     {
       if (message.ignoreDisabled && s.disabled)
         return false;
       if (s instanceof DownloadableSubscription && message.downloadable)
         return true;
       if (s instanceof SpecialSubscription && message.special)
         return true;
       return false;
     });
 
@@ skipping 29 matching lines @@
       subscription.title = message.title;
       subscription.homepage = message.homepage;
       FilterStorage.addSubscription(subscription);
       if (!subscription.lastDownload)
         Synchronizer.execute(subscription);
     }
   });
 
   port.on("subscriptions.update", (message, sender) =>
   {
-    let subscriptions = message.url ? [Subscription.fromURL(message.url)]
-                        : FilterStorage.subscriptions;

[Sebastian Noack, 2017/02/21 12:21:21]

Instead of wrapping a ternary operation, the following might be more readable:

  let subscriptions = FilterStorage.subscriptions;
  if (message.url)
    subscriptions = [Subscription.fromURL(message.url)];

[kzar, 2017/03/01 05:36:20]

Done.

+    let {subscriptions} = FilterStorage;
+    if (message.url)
+      subscriptions = [Subscription.fromURL(message.url)];
+
     for (let subscription of subscriptions)
     {
       if (subscription instanceof DownloadableSubscription)
         Synchronizer.execute(subscription, true);
     }
   });
-}
+})(this);