Make sure ABP category cannot be removed (work-around for AVG toolbar issue)

lib/contentPolicy.js

 /*
  * This file is part of the Adblock Plus,
  * Copyright (C) 2006-2012 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
@@ skipping 364 matching lines @@
     Services.obs.addObserver(this, "content-document-global-created", true);
     Services.obs.addObserver(this, "xpcom-category-entry-removed", true);
     Services.obs.addObserver(this, "xpcom-category-cleared", true);
 
     onShutdown.add(function()
     {
       // Our category observers should be removed before changing category
       // memberships, just in case.
       Services.obs.removeObserver(this, "http-on-modify-request");
       Services.obs.removeObserver(this, "content-document-global-created");
-      Services.obs.removeObserver(this, "xpcom-category-entry-removed", true);

[Thomas Greiner, 2013/02/07 16:16:27]

removeObserver doesn't require a third parameter
https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIObserve...

-      Services.obs.removeObserver(this, "xpcom-category-cleared", true);

[Thomas Greiner, 2013/02/07 16:16:27]

removeObserver doesn't require a third parameter
https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIObserve...

+      Services.obs.removeObserver(this, "xpcom-category-entry-removed");
+      Services.obs.removeObserver(this, "xpcom-category-cleared");
 
       for each (let category in this.xpcom_categories)
         catMan.deleteCategoryEntry(category, this.contractID, false);
 
       // This needs to run asynchronously, see bug 753687
       Utils.runAsync(function()
       {
         registrar.unregisterFactory(this.classID, this);
       }.bind(this));
 
@@ skipping 102 matching lines @@
           if (wnd && wnd.opener && wnd.location.href == "about:blank")
             this.observe(wnd, "content-document-global-created", null, subject.URI);
         }
 
         break;
       }
       case "xpcom-category-entry-removed":
       case "xpcom-category-cleared":
       {
         let category = data;
-        if (!this.xpcom_categories.some(function(c) c == category))

[Thomas Greiner, 2013/02/07 16:16:27]

this.xpcom_categories.indexOf(category) == -1
should be slightly faster in this case

+        if (this.xpcom_categories.indexOf(category) < 0)
           return;
 
-        if (topic == "xpcom-category-entry-removed" && subject instanceof Ci.nsISupportsCString)
-          if (subject.data != this.contractID)

[Thomas Greiner, 2013/02/07 16:16:27]

Why did you split this up into two if-statements?

[Thomas Greiner, 2013/02/07 16:16:27]

You are already checking for the topic with the switch statement. You could get
rid of the |topic == "xpcom-category-entry-removed"| equation by moving it up
into the "xpcom-category-entry-removed" case.

[Wladimir Palant, 2013/02/08 07:52:31]

Yes, this is possible of course. However, fallthrough is something I would
rather not use - it makes the flow very non-obvious (you have to notice the
missing break statement). The code is a lot easier to read this way.

-            return;
+        if (topic == "xpcom-category-entry-removed" &&
+            subject instanceof Ci.nsISupportsCString &&
+            subject.data != this.contractID)
+        {
+          return;
+        }
 
         // Our category entry was removed, make sure to add it back
         let catMan = Utils.categoryManager;
         catMan.addCategoryEntry(category, this.contractID, this.contractID, false, true);
         break;
       }
     }
   },
 
   //
@@ skipping 210 matching lines @@
   if (!wnd || wnd.closed)
     return;
 
   if (entry.type == Policy.type.OBJECT)
   {
     node.removeEventListener("mouseover", objectMouseEventHander, true);
     node.removeEventListener("mouseout", objectMouseEventHander, true);
   }
   Policy.processNode(wnd, node, entry.type, Utils.makeURI(entry.location), true);
 }