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

lib/filterComposer.js

Index: lib/filterComposer.js
===================================================================
--- a/lib/filterComposer.js
+++ b/lib/filterComposer.js
@@ -19,12 +19,27 @@
 
 "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;
 }
@@ -138,10 +153,44 @@
   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) =>