Issue 3823 - Split up message responder code

popup.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/>.
  */
 
 var backgroundPage = ext.backgroundPage.getWindow();
 var require = backgroundPage.require;
 
 var Filter = require("filterClasses").Filter;
 var FilterStorage = require("filterStorage").FilterStorage;
 var Prefs = require("prefs").Prefs;
 var checkWhitelisted = require("whitelisting").checkWhitelisted;
 var getDecodedHostname = require("url").getDecodedHostname;
-var getPort = require("messaging").getPort;

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

I'd really go with:

  var port = require("messaging").getPort(window);

So below you'd just access the port variable. It should only be one port, it's
not an object you create from scratch each time.

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

Well, the code below only runs once if at all. So there is no way we will create
more than one port here, but on the other hand, this way we only create the port
(and add an addional ext.onMessage listener) if necessary.

[Wladimir Palant, 2016/03/21 18:05:22]

Why not call require only in that case then?

[Sebastian Noack, 2016/03/21 19:54:13]

I lend towards having all imports at the beginning of the script. But fair
enough. Done.

 
 var page = null;
 
 function onLoad()
 {
   ext.pages.query({active: true, lastFocusedWindow: true}, function(pages)
   {
     page = pages[0];
 
     // Mark page as 'local' or 'nohtml' to hide non-relevant elements
     if (!page || (page.url.protocol != "http:" &&
                   page.url.protocol != "https:"))
       document.body.classList.add("local");
     else if (!backgroundPage.htmlPages.has(page))
     {
       document.body.classList.add("nohtml");
-      getPort(window).on("composer.ready", function(message, sender)
-      {
-        if (sender.page.id == page.id)
-          document.body.classList.remove("nohtml");
-      });
+      require("messaging").getPort(window).on(
+        "composer.ready", function(message, sender)
+        {
+          if (sender.page.id == page.id)
+            document.body.classList.remove("nohtml");
+        }
+      );
     }
 
     // Ask content script whether clickhide is active. If so, show cancel button.
     // If that isn't the case, ask background.html whether it has cached filters. If so,
     // ask the user whether she wants those filters.
     // Otherwise, we are in default state.
     if (page)
     {
       if (checkWhitelisted(page))
         document.body.classList.add("disabled");
@@ skipping 75 matching lines @@
 }
 
 function toggleCollapse(event)
 {
   var collapser = event.currentTarget;
   Prefs[collapser.dataset.option] = !Prefs[collapser.dataset.option];
   collapser.parentNode.classList.toggle("collapsed");
 }
 
 document.addEventListener("DOMContentLoaded", onLoad, false);