Issue 6424 - Use internal symbol in ext namespace

ext/background.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present 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/>.
  */
 
 /* global internal, defineNamespace */

[Manish Jethani, 2018/02/28 15:23:26]

We need to do this for now. Later we can include these globals in the ESLint
config.

 
 "use strict";
 
 {
   let nonEmptyPageMaps = new Set();
 
   let PageMap = ext.PageMap = function()
   {
     defineNamespace(this, internal);

[Manish Jethani, 2018/02/28 15:23:26]

Call defineNamespace in the constructor.

 
     this[internal].map = new Map();
   };
   PageMap.prototype = {
+    [internal]: {
+      delete(pageId)
+      {
+        this[internal].map.delete(pageId);
+
+        if (this[internal].map.size == 0)
+          nonEmptyPageMaps.delete(this);
+      }
+    },
     keys()
     {
       return Array.from(this[internal].map.keys()).map(ext.getPage);
     },
     get(page)
     {
       return this[internal].map.get(page.id);
     },
     set(page, value)
     {
       this[internal].map.set(page.id, value);
       nonEmptyPageMaps.add(this);
     },
     has(page)
     {
       return this[internal].map.has(page.id);
     },
     clear()
     {
       this[internal].map.clear();
       nonEmptyPageMaps.delete(this);
     },
     delete(page)
     {
-      deletePage(this, page.id);
-    }
-  };
-
-  function deletePage(pageMap, pageId)

[Manish Jethani, 2018/02/28 15:23:26]

ext.PageMap.prototype._delete is now simply deletePage. The first argument is
the ext.PageMap instance. The rest of the code is unchanged.

-  {
-    pageMap[internal].map.delete(pageId);
-
-    if (pageMap[internal].map.size == 0)
-      nonEmptyPageMaps.delete(pageMap);
-  }
+      this[internal].delete(page.id);
+    }
+  };
 
   ext[internal].removeFromAllPageMaps = pageId =>
   {
     for (let pageMap of nonEmptyPageMaps)
-      deletePage(pageMap, pageId);
+      pageMap[internal].delete(pageId);
   };
 
   /* Pages */
 
   let Page = ext.Page = function(tab)
   {
     defineNamespace(this, internal);
 
     this.id = tab.id;
 
@@ skipping 47 matching lines @@
 
   ext.pages = {
     onLoading: new ext[internal].EventTarget(),
     onActivated: new ext[internal].EventTarget(),
     onRemoved: new ext[internal].EventTarget()
   };
 
   browser.tabs.onUpdated.addListener((tabId, changeInfo, tab) =>
   {
     if (changeInfo.status == "loading")
-      ext[internal].dispatchEvent(ext.pages.onLoading, new Page(tab));
+      ext.pages.onLoading[internal].dispatch(new Page(tab));
   });
 
   function createFrame(tabId, frameId)
   {
     let frames = framesOfTabs.get(tabId);
     if (!frames)
     {
       frames = new Map();
       framesOfTabs.set(tabId, frames);
     }
@@ skipping 18 matching lines @@
 
       browser.tabs.get(tabId, () =>
       {
         // If the tab is prerendered, browser.tabs.get() sets
         // browser.runtime.lastError and we have to dispatch the onLoading
         // event, since the onUpdated event isn't dispatched for prerendered
         // tabs. However, we have to keep relying on the onUpdated event for
         // tabs that are already visible. Otherwise browser action changes get
         // overridden when Chrome automatically resets them on navigation.
         if (browser.runtime.lastError)
-          ext[internal].dispatchEvent(ext.pages.onLoading, page);
+          ext.pages.onLoading[internal].dispatch(page);
       });
     }
 
     // Update frame URL and parent in frame structure
     let frame = createFrame(tabId, frameId);
     frame.url = new URL(url);
 
     let parentFrame = framesOfTabs.get(tabId).get(parentFrameId);
     if (parentFrame)
       frame.parent = parentFrame;
@@ skipping 89 matching lines @@
           // for Web Store URLs.
           !url.startsWith("https://chrome.google.com/webstore/")))
     {
       updatePageFrameStructure(details.frameId, details.tabId, url,
                                details.parentFrameId);
     }
   });
 
   function forgetTab(tabId)
   {
-    ext[internal].dispatchEvent(ext.pages.onRemoved, tabId);
+    ext.pages.onRemoved[internal].dispatch(tabId);
 
     ext[internal].removeFromAllPageMaps(tabId);
     framesOfTabs.delete(tabId);
   }
 
   browser.tabs.onReplaced.addListener((addedTabId, removedTabId) =>
   {
     forgetTab(removedTabId);
   });
 
   browser.tabs.onRemoved.addListener(forgetTab);
 
   browser.tabs.onActivated.addListener(details =>
   {
-    ext[internal].dispatchEvent(ext.pages.onActivated,
-                                new Page({id: details.tabId}));
+    ext.pages.onActivated[internal].dispatch(new Page({id: details.tabId}));
   });
 
 
   /* Browser actions */
 
   let BrowserAction = function(tabId)
   {
     defineNamespace(this, internal);
 
     this[internal].tabId = tabId;
     this[internal].changes = null;
   };
   BrowserAction.prototype = {
+    [internal]: {
+      applyChanges()
+      {
+        if ("iconPath" in this[internal].changes)
+        {
+          // Firefox for Android displays the browser action not as an icon but
+          // as a menu item. There is no icon, but such an option may be added
+          // in the future.
+          // https://bugzilla.mozilla.org/show_bug.cgi?id=1331746
+          if ("setIcon" in browser.browserAction)
+          {

[Manish Jethani, 2018/03/01 12:04:14]

This is mostly just an indentation change. this._applyChanges,
this._queueChanges, and this._addChange are now this[internal].applyChanges,
this[internal].queueChanges, and this[internal].addChange and that's all.

+            let path = {
+              16: this[internal].changes.iconPath.replace("$size", "16"),
+              19: this[internal].changes.iconPath.replace("$size", "19"),
+              20: this[internal].changes.iconPath.replace("$size", "20"),
+              32: this[internal].changes.iconPath.replace("$size", "32"),
+              38: this[internal].changes.iconPath.replace("$size", "38"),
+              40: this[internal].changes.iconPath.replace("$size", "40")
+            };
+            try
+            {
+              browser.browserAction.setIcon({
+                tabId: this[internal].tabId,
+                path
+              });
+            }
+            catch (e)
+            {
+              // Edge throws if passed icon sizes different than 19,20,38,40px.
+              delete path[16];
+              delete path[32];
+              browser.browserAction.setIcon({
+                tabId: this[internal].tabId,
+                path
+              });
+            }
+          }
+        }
+
+        if ("badgeText" in this[internal].changes)
+        {
+          // There is no badge on Firefox for Android; the browser action is
+          // simply a menu item.
+          if ("setBadgeText" in browser.browserAction)
+          {
+            browser.browserAction.setBadgeText({
+              tabId: this[internal].tabId,
+              text: this[internal].changes.badgeText
+            });
+          }
+        }
+
+        if ("badgeColor" in this[internal].changes)
+        {
+          // There is no badge on Firefox for Android; the browser action is
+          // simply a menu item.
+          if ("setBadgeBackgroundColor" in browser.browserAction)
+          {
+            browser.browserAction.setBadgeBackgroundColor({
+              tabId: this[internal].tabId,
+              color: this[internal].changes.badgeColor
+            });
+          }
+        }
+
+        this[internal].changes = null;
+      },
+      queueChanges()
+      {
+        browser.tabs.get(this[internal].tabId, () =>
+        {
+          // If the tab is prerendered, browser.tabs.get() sets
+          // browser.runtime.lastError and we have to delay our changes
+          // until the currently visible tab is replaced with the
+          // prerendered tab. Otherwise browser.browserAction.set* fails.
+          if (browser.runtime.lastError)
+          {
+            let onReplaced = (addedTabId, removedTabId) =>
+            {
+              if (addedTabId == this[internal].tabId)
+              {
+                browser.tabs.onReplaced.removeListener(onReplaced);
+                this[internal].applyChanges();
+              }
+            };
+            browser.tabs.onReplaced.addListener(onReplaced);
+          }
+          else
+          {
+            this[internal].applyChanges();
+          }
+        });
+      },
+      addChange(name, value)
+      {
+        if (!this[internal].changes)
+        {
+          this[internal].changes = {};
+          this[internal].queueChanges();
+        }
+
+        this[internal].changes[name] = value;
+      }
+    },
     setIcon(path)
     {
-      addBrowserActionChange(this, "iconPath", path);
+      this[internal].addChange("iconPath", path);
     },
     setBadge(badge)
     {
       if (!badge)
       {
-        addBrowserActionChange(this, "badgeText", "");
+        this[internal].addChange("badgeText", "");
       }
       else
       {
         if ("number" in badge)
-          addBrowserActionChange(this, "badgeText", badge.number.toString());
+          this[internal].addChange("badgeText", badge.number.toString());
 
         if ("color" in badge)
-          addBrowserActionChange(this, "badgeColor", badge.color);
-      }
-    }
-  };
-
-  function queueBrowserActionChanges(browserAction)

[Manish Jethani, 2018/02/28 15:23:26]

Similarly the _queueChanges, _applyChanges, and _addChange methods in
BrowserAction.prototype and now simply regular functions in the outer scope
taking the actual BrowserAction instance as the first argument.

-  {
-    browser.tabs.get(browserAction[internal].tabId, () =>
-    {
-      // If the tab is prerendered, browser.tabs.get() sets
-      // browser.runtime.lastError and we have to delay our changes
-      // until the currently visible tab is replaced with the
-      // prerendered tab. Otherwise browser.browserAction.set* fails.
-      if (browser.runtime.lastError)
-      {
-        let onReplaced = (addedTabId, removedTabId) =>
-        {
-          if (addedTabId == browserAction[internal].tabId)
-          {
-            browser.tabs.onReplaced.removeListener(onReplaced);
-            applyBrowserActionChanges(browserAction);
-          }
-        };
-        browser.tabs.onReplaced.addListener(onReplaced);
-      }
-      else
-      {
-        applyBrowserActionChanges(browserAction);
-      }
-    });
-  }
-
-  function applyBrowserActionChanges(browserAction)
-  {
-    if ("iconPath" in browserAction[internal].changes)
-    {
-      // Firefox for Android displays the browser action not as an icon but
-      // as a menu item. There is no icon, but such an option may be added in
-      // the future.
-      // https://bugzilla.mozilla.org/show_bug.cgi?id=1331746
-      if ("setIcon" in browser.browserAction)
-      {
-        let path = {
-          16: browserAction[internal].changes.iconPath.replace("$size", "16"),
-          19: browserAction[internal].changes.iconPath.replace("$size", "19"),
-          20: browserAction[internal].changes.iconPath.replace("$size", "20"),
-          32: browserAction[internal].changes.iconPath.replace("$size", "32"),
-          38: browserAction[internal].changes.iconPath.replace("$size", "38"),
-          40: browserAction[internal].changes.iconPath.replace("$size", "40")
-        };
-        try
-        {
-          browser.browserAction.setIcon({
-            tabId: browserAction[internal].tabId,
-            path
-          });
-        }
-        catch (e)
-        {
-          // Edge throws if passed icon sizes different than 19,20,38,40px.
-          delete path[16];
-          delete path[32];
-          browser.browserAction.setIcon({
-            tabId: browserAction[internal].tabId,
-            path
-          });
-        }
-      }
-    }
-
-    if ("badgeText" in browserAction[internal].changes)
-    {
-      // There is no badge on Firefox for Android; the browser action is
-      // simply a menu item.
-      if ("setBadgeText" in browser.browserAction)
-      {
-        browser.browserAction.setBadgeText({
-          tabId: browserAction[internal].tabId,
-          text: browserAction[internal].changes.badgeText
-        });
-      }
-    }
-
-    if ("badgeColor" in browserAction[internal].changes)
-    {
-      // There is no badge on Firefox for Android; the browser action is
-      // simply a menu item.
-      if ("setBadgeBackgroundColor" in browser.browserAction)
-      {
-        browser.browserAction.setBadgeBackgroundColor({
-          tabId: browserAction[internal].tabId,
-          color: browserAction[internal].changes.badgeColor
-        });
-      }
-    }
-
-    browserAction[internal].changes = null;
-  }
-
-  function addBrowserActionChange(browserAction, name, value)
-  {
-    if (!browserAction[internal].changes)
-    {
-      browserAction[internal].changes = {};
-      queueBrowserActionChanges(browserAction);
-    }
-
-    browserAction[internal].changes[name] = value;
-  }
+          this[internal].addChange("badgeColor", badge.color);
+      }
+    }
+  };
 
 
   /* Context menus */
 
   let contextMenuItems = new ext.PageMap();
   let contextMenuUpdating = false;
 
   let updateContextMenu = () =>
   {
     // Firefox for Android does not support context menus.
@@ skipping 184 matching lines @@
     // Sometimes requests are not associated with a browser tab and
     // in this case we want to still be able to view the url being called.
     let frame = null;
     let page = null;
     if (details.tabId != -1)
     {
       frame = ext.getFrame(details.tabId, frameId);
       page = new Page({id: details.tabId});
     }
 
-    if (ext[internal].dispatchEvent(ext.webRequest.onBeforeRequest,
-                                    url, type, page, frame).includes(false))
+    if (ext.webRequest.onBeforeRequest[internal]
+        .dispatch(url, type, page, frame).includes(false))
     {
       return {cancel: true};
     }
   }, {urls: ["<all_urls>"]}, ["blocking"]);
 
 
   /* Message passing */
 
   browser.runtime.onMessage.addListener((message, rawSender, sendResponse) =>
   {
@@ skipping 18 matching lines @@
 
           let frame = frames.get(rawSender.frameId);
           if (frame)
             return frame.parent || null;
 
           return frames.get(0) || null;
         }
       };
     }
 
-    return ext[internal].dispatchEvent(
-      ext.onMessage,
+    return ext.onMessage[internal].dispatch(
       message, sender, sendResponse
     ).includes(true);
   });
 
 
   /* Storage */
 
   ext.storage = {
     get(keys, callback)
     {
@@ skipping 16 matching lines @@
   ext.windows = {
     create(createData, callback)
     {
       browser.windows.create(createData, createdWindow =>
       {
         afterTabLoaded(callback)(createdWindow.tabs[0]);
       });
     }
   };
 }