Issue 3443 - Use process script instead of a frame script in Element Hiding Helper

chrome/content/processScript.js

 /*
  * This Source Code is subject to the terms of the Mozilla Public License
  * version 2.0 (the "License"). You can obtain a copy of the License at
  * http://mozilla.org/MPL/2.0/.
  */
 
 const Ci = Components.interfaces;
 const Cu = Components.utils;
 
 let {console} = Cu.import("resource://gre/modules/devtools/Console.jsm", {});
 let {DebuggerServer} = Cu.import("resource://gre/modules/devtools/dbg-server.jsm", {});
 let {Services} = Cu.import("resource://gre/modules/Services.jsm", {});
 
 let processID = Services.appinfo.processID;
 let maxNodeID = 0;
 let nodes = new Map();
 
 let name = "elemhidehelper";
 let actor = {
   constructorFun: Actor,
   constructorName: name,
   name: name
 };
 
 addMessageListener("ElemHideHelper:Shutdown", onShutdown);
 addMessageListener("ElemHideHelper:GetNodeInfo", onGetNodeInfo);
 addMessageListener("ElemHideHelper:Preview", onTogglePreview);
 

[saroyanm, 2015/12/21 16:48:55]

Nit: No new line should be needed here.

[Wladimir Palant, 2015/12/21 19:14:15]

I disagree - I'd rather separate functional blocks, and setting up messaging
isn't the same thing as registering the main functionality.

 DebuggerServer.addTabActor(actor, name);
 
-var onShutdown = (function()

[saroyanm, 2015/12/21 16:48:55]

I think it's something that I pointed in another review, I think we should be
fine declaring this as "let".

[Wladimir Palant, 2015/12/21 19:14:15]

True, this is no longer necessary. In fact, the complicated guard against
multiple invocation isn't really necessary either - I removed it.

[saroyanm, 2015/12/23 10:57:05]

Looks good.

+function onShutdown()
 {
-  let executed = false;
-  return function()
+  removeMessageListener("ElemHideHelper:Shutdown", onShutdown);
+  removeMessageListener("ElemHideHelper:GetNodeInfo", onGetNodeInfo);
+  removeMessageListener("ElemHideHelper:Preview", onTogglePreview);
+
+  try
   {
-    if (!executed)
-    {
-      executed = true;
-

[saroyanm, 2015/12/21 16:48:55]

Nit: No new line should be needed here.

[Wladimir Palant, 2015/12/21 19:14:15]

Same as above - I'd rather separate functional blocks.

-      removeMessageListener("ElemHideHelper:Shutdown", onShutdown);
-      removeMessageListener("ElemHideHelper:GetNodeInfo", onGetNodeInfo);
-      removeMessageListener("ElemHideHelper:Preview", onTogglePreview);
-
-      try
-      {
-        DebuggerServer.removeTabActor(actor);
-      }
-      catch (e)
-      {
-        // The call above will throw in the content process despite succeeding,
-        // see https://bugzilla.mozilla.org/show_bug.cgi?id=1189780.
-        Cu.reportError(e);
-      }
-    }
+    DebuggerServer.removeTabActor(actor);
   }
-})();
+  catch (e)
+  {
+    // The call above will throw in the content process despite succeeding,
+    // see https://bugzilla.mozilla.org/show_bug.cgi?id=1189780.
+    Cu.reportError(e);
+  }
+}
 
 function onGetNodeInfo(message)
 {
   if (Cu.isCrossProcessWrapper(message.objects.element))
     return;
 
   let nodeInfo = getNodeInfo(message.objects.element);
   nodeInfo.messageId = message.data;
   sendAsyncMessage("ElemHideHelper:GetNodeInfo:Response", nodeInfo);
 }
@@ skipping 118 matching lines @@
       // forget this node then.
       forgetNode(nodeID);
     }
   }
 }
 
 function forgetNode(nodeID)
 {
   nodes.delete(nodeID);
 }