Issue 3823 - Split up message responder code

background.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/>.
  */
 
 with(require("filterClasses"))
 {
   this.BlockingFilter = BlockingFilter;
   this.WhitelistFilter = WhitelistFilter;
   this.RegExpFilter = RegExpFilter;
 }
 with(require("whitelisting"))
 {
   this.checkWhitelisted = checkWhitelisted;
-  this.processKey = processKey;
   this.getKey = getKey;
 }
 with(require("url"))
 {
   this.stringifyURL = stringifyURL;
   this.isThirdParty = isThirdParty;
   this.extractHostFromFrame = extractHostFromFrame;
 }
 var FilterStorage = require("filterStorage").FilterStorage;
 var FilterNotifier = require("filterNotifier").FilterNotifier;
 var SpecialSubscription = require("subscriptionClasses").SpecialSubscription;
 var ElemHide = require("elemHide").ElemHide;
 var defaultMatcher = require("matcher").defaultMatcher;
 var Prefs = require("prefs").Prefs;
-var parseFilters = require("filterValidation").parseFilters;
-var composeFilters = require("filterComposer").composeFilters;
 var updateIcon = require("icon").updateIcon;
 var showNextNotificationForUrl = require("notificationHelper").showNextNotificationForUrl;
+var port = require("messaging").port;

[Wladimir Palant, 2016/03/21 15:50:12]

Arguably, this should use getPort() as well and no default port exposed - if you
expose a default port people will use it by accident and listeners will stay
around.

[Sebastian Noack, 2016/03/21 17:15:42]

background.js is matter to go away. In the mid-term all code will be
encapsulated in modules. This patch is just the first step. And I'd rather avoid
unnecessary boilerplate to get the port in each module, as well as logic to
cache the port which wouldn't be necessary otherwise.

I consider getPort() a temporarily hack, that is matter to be removed once we
use asynchronous messaging properly in the popup. And except for that edge case,
all messaging code using the port API runs in the background page (hence in the
same thread and for the same window) anyway, where we don't have to disconnect
the port.

 var devtools = require("devtools");
 
 // Special-case domains for which we cannot use style-based hiding rules.
 // See http://crbug.com/68705.
 var noStyleRulesHosts = ["mail.google.com", "mail.yahoo.com", "www.google.com"];
 
 var htmlPages = new ext.PageMap();
 
 var contextMenuItem = {
   title: ext.i18n.getMessage("block_element"),
@@ skipping 57 matching lines @@
       if (filter instanceof WhitelistFilter &&  /^@@\|\|([^\/:]+)\^\$document$/.test(filter.text))
         exceptions.push(RegExp.$1);
       else
         filters.push(filter.text);
     }
   }
 
   return {filters: filters, exceptions: exceptions};
 }
 
-ext.onMessage.addListener(function (msg, sender, sendResponse)
+port.on("get-selectors", function(msg, sender)
 {
-  switch (msg.type)
+  var selectors = [];
+  var trace = devtools && devtools.hasPanel(sender.page);
+
+  if (!checkWhitelisted(sender.page, sender.frame,
+                        RegExpFilter.typeMap.DOCUMENT |
+                        RegExpFilter.typeMap.ELEMHIDE))
   {
-    case "composer.openDialog":
-      ext.windows.create({
-        url: ext.getURL("block.html"),
-        left: 50,
-        top: 50,
-        width: 420,
-        height: 200,
-        focused: true,
-        type: "popup"
-      },
-      function (popupPage) {
-        var popupPageId = popupPage.id;
-        function onRemoved(removedPageId)
-        {
-          if (popupPageId == removedPageId)
-          {
-            sender.page.sendMessage({
-              type: "composer.content.dialogClosed",
-              popupId: popupPageId
-            });
-            ext.pages.onRemoved.removeListener(onRemoved);
-          }
-        }
-        ext.pages.onRemoved.addListener(onRemoved);
+    var noStyleRules = false;
+    var specificOnly = checkWhitelisted(sender.page, sender.frame,
+                                        RegExpFilter.typeMap.GENERICHIDE);
+    var host = extractHostFromFrame(sender.frame);
 
-        sendResponse(popupPageId);
+    for (var i = 0; i < noStyleRulesHosts.length; i++)
+    {
+      var noStyleHost = noStyleRulesHosts[i];
+      if (host == noStyleHost || (host.length > noStyleHost.length &&
+                                  host.substr(host.length - noStyleHost.length - 1) == "." + noStyleHost))
+      {
+        noStyleRules = true;
+      }
+    }
+    selectors = ElemHide.getSelectorsForDomain(host, specificOnly);
+    if (noStyleRules)
+    {
+      selectors = selectors.filter(function(s)
+      {
+        return !/\[style[\^\$]?=/.test(s);
       });
-      return true;
-      break;
-    case "get-selectors":
-      var selectors = [];
-      var trace = devtools && devtools.hasPanel(sender.page);
+    }
+  }
 
-      if (!checkWhitelisted(sender.page, sender.frame,
-                            RegExpFilter.typeMap.DOCUMENT |
-                            RegExpFilter.typeMap.ELEMHIDE))
-      {
-        var noStyleRules = false;
-        var specificOnly = checkWhitelisted(sender.page, sender.frame,
-                                            RegExpFilter.typeMap.GENERICHIDE);
-        var host = extractHostFromFrame(sender.frame);
+  return {selectors: selectors, trace: trace};
+});
 
-        for (var i = 0; i < noStyleRulesHosts.length; i++)
-        {
-          var noStyleHost = noStyleRulesHosts[i];
-          if (host == noStyleHost || (host.length > noStyleHost.length &&
-                                      host.substr(host.length - noStyleHost.length - 1) == "." + noStyleHost))
-          {
-            noStyleRules = true;
-          }
-        }
-        selectors = ElemHide.getSelectorsForDomain(host, specificOnly);
-        if (noStyleRules)
-        {
-          selectors = selectors.filter(function(s)
-          {
-            return !/\[style[\^\$]?=/.test(s);
-          });
-        }
-      }
+port.on("should-collapse", function(msg, sender)
+{
+  if (checkWhitelisted(sender.page, sender.frame))
+    return false;
 
-      sendResponse({selectors: selectors, trace: trace});
-      break;
-    case "should-collapse":
-      if (checkWhitelisted(sender.page, sender.frame))
-      {
-        sendResponse(false);
-        break;
-      }
+  var typeMask = RegExpFilter.typeMap[msg.mediatype];
+  var documentHost = extractHostFromFrame(sender.frame);
+  var sitekey = getKey(sender.page, sender.frame);
+  var blocked = false;
 
-      var typeMask = RegExpFilter.typeMap[msg.mediatype];
-      var documentHost = extractHostFromFrame(sender.frame);
-      var sitekey = getKey(sender.page, sender.frame);
-      var blocked = false;
+  var specificOnly = checkWhitelisted(
+    sender.page, sender.frame,
+    RegExpFilter.typeMap.GENERICBLOCK
+  );
 
-      var specificOnly = checkWhitelisted(
-        sender.page, sender.frame,
-        RegExpFilter.typeMap.GENERICBLOCK
-      );
+  for (var i = 0; i < msg.urls.length; i++)
+  {
+    var url = new URL(msg.urls[i], msg.baseURL);
+    var filter = defaultMatcher.matchesAny(
+      stringifyURL(url), typeMask,
+      documentHost, isThirdParty(url, documentHost),
+      sitekey, specificOnly
+    );
 
-      for (var i = 0; i < msg.urls.length; i++)
-      {
-        var url = new URL(msg.urls[i], msg.baseURL);
-        var filter = defaultMatcher.matchesAny(
-          stringifyURL(url), typeMask,
-          documentHost, isThirdParty(url, documentHost),
-          sitekey, specificOnly
-        );
+    if (filter instanceof BlockingFilter)
+    {
+      if (filter.collapse != null)
+        return filter.collapse;
 
-        if (filter instanceof BlockingFilter)
-        {
-          if (filter.collapse != null)
-          {
-            sendResponse(filter.collapse);
-            return;
-          }
+      blocked = true;
+    }
+  }
 
-          blocked = true;
-        }
-      }
+  return blocked && Prefs.hidePlaceholders;
+});
 
-      sendResponse(blocked && Prefs.hidePlaceholders);
-      break;
-    case "get-domain-enabled-state":
-      // Returns whether this domain is in the exclusion list.
-      // The browser action popup asks us this.
-      if(sender.page)
-      {
-        sendResponse({enabled: !checkWhitelisted(sender.page)});
-        return;
-      }
-      break;
-    case "add-sitekey":
-      processKey(msg.token, sender.page, sender.frame);
-      break;
-    case "composer.ready":
-      htmlPages.set(sender.page, null);
-      refreshIconAndContextMenu(sender.page);
-      break;
-    case "composer.getFilters":
-      sendResponse(composeFilters({
-        tagName: msg.tagName,
-        id: msg.id,
-        src: msg.src,
-        style: msg.style,
-        classes: msg.classes,
-        urls: msg.urls,
-        type: msg.mediatype,
-        baseURL: msg.baseURL,
-        page: sender.page,
-        frame: sender.frame
-      }));
-      break;
-    case "trace-elemhide":
-      devtools.logHiddenElements(
-        sender.page, msg.selectors,
-        extractHostFromFrame(sender.frame)
-      );
-      break;
-    case "has-devtools-panel":
-      sendResponse(Prefs.show_devtools_panel);
-      break;
-    case "forward":
-      var targetPage;
-      if (msg.targetPageId)
-        targetPage = ext.getPage(msg.targetPageId);
-      else
-        targetPage = sender.page;
+port.on("get-domain-enabled-state", function(msg, sender)
+{
+  return {enabled: !checkWhitelisted(sender.page)};
+});
 
-      if (targetPage)
-      {
-        msg.payload.sender = sender.page.id;
-        if (msg.expectsResponse)
-        {
-          targetPage.sendMessage(msg.payload, sendResponse);
-          return true;
-        }
+port.on("forward", function(msg, sender)
+{
+  var targetPage;
+  if (msg.targetPageId)
+    targetPage = ext.getPage(msg.targetPageId);
+  else
+    targetPage = sender.page;
 
-        targetPage.sendMessage(msg.payload);
-      }
-      break;
+  if (targetPage)
+  {
+    msg.payload.sender = sender.page.id;
+    if (msg.expectsResponse)
+      return new Promise(targetPage.sendMessage.bind(targetPage, msg.payload));
+    targetPage.sendMessage(msg.payload);
   }
 });
 
 // update icon when page changes location
 ext.pages.onLoading.addListener(function(page)
 {
   page.sendMessage({type: "composer.content.finished"});
   refreshIconAndContextMenu(page);
   showNextNotificationForUrl(page.url);
 });