Issue 3842 - Split up the logic updating the icon and context menu

lib/filterComposer.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/>.
  */
 
 /** @module filterComposer */
 
 "use strict";
 
+let {defaultMatcher} = require("matcher");
+let {RegExpFilter} = require("filterClasses");
+let {FilterNotifier} = require("filterNotifier");
+let {Prefs} = require("prefs");
 let {extractHostFromFrame, stringifyURL, isThirdParty} = require("url");
 let {getKey, checkWhitelisted} = require("whitelisting");
-let {defaultMatcher} = require("matcher");
-let {RegExpFilter} = require("filterClasses");
 let {port} = require("messaging");
 
+let readyPages = new ext.PageMap();
+
+/**
+ * Checks whether the given page is ready to use the filter composer
+ *
+ * @param {Page} page
+ * @return {boolean}
+ */
+exports.isPageReady = function(page)
+{
+  return readyPages.has(page);
+};
+
 function isValidString(s) {
   return s && s.indexOf("\0") == -1;
 }
 
 function escapeChar(chr)
 {
   let code = chr.charCodeAt(0);
 
   // Control characters and leading digits must be escaped based on
   // their char code in CSS. Moreover, curly brackets aren't allowed
@@ skipping 93 matching lines @@
 
       // Add an element hiding filter for each generated CSS selector
       for (let selector of selectors)
         filters.push(docDomain.replace(/^www\./, "") + "##" + selector);
     }
   }
 
   return {filters: filters, selectors: selectors};
 }
 
+let contextMenuItem = {
+  title: ext.i18n.getMessage("block_element"),
+  contexts: ["image", "video", "audio"],
+  onclick: page =>
+  {
+    page.sendMessage({type: "composer.content.contextMenuClicked"});
+  }
+};
+
+function updateContextMenu(page, filter)
+{
+  page.contextMenus.remove(contextMenuItem);
+
+  if (typeof filter == "undefined")
+    filter = checkWhitelisted(page);
+  if (!filter && Prefs.shouldShowBlockElementMenu && readyPages.has(page))

[kzar, 2016/03/20 12:51:06]

(Maybe we should first check the preference before calling checkWhitelisted when
the filter parameter wasn't passed. (It's wasteful to check if the page is
whitelisted when the feature is disabled anyway.) On the other hand it would
make the logic here slightly messier and I'm not sure how much it would improve
the performance. I leave it up to you.)

[Sebastian Noack, 2016/03/20 15:12:01]

As you said, that micro optimization would make the logic more complicated here.
And that preference set to false isn't a particular common scenario.

+    page.contextMenus.create(contextMenuItem);
+}
+
+FilterNotifier.addListener((action, page, filter) =>
+{
+  if (action == "page.WhitelistingStateRevalidate")
+    updateContextMenu(page, filter);
+});
+
+Prefs.on("shouldShowBlockElementMenu", () =>
+{
+  ext.pages.query({}, pages =>
+  {
+    for (let page of pages)
+      updateContextMenu(page);
+  });
+});
+
 port.on("composer.ready", (message, sender) =>
 {
-  htmlPages.set(sender.page, null);
-  refreshIconAndContextMenu(sender.page);
+  readyPages.set(sender.page, null);
+  updateContextMenu(sender.page);
 });
 
 port.on("composer.openDialog", (message, sender) =>
 {
   return new Promise(resolve =>
   {
     ext.windows.create({
       url: ext.getURL("composer.html"),
       left: 50,
       top: 50,
@@ skipping 30 matching lines @@
     src:     message.src,
     style:   message.style,
     classes: message.classes,
     urls:    message.urls,
     type:    message.mediatype,
     baseURL: message.baseURL,
     page:    sender.page,
     frame:   sender.frame
   });
 });