Issue 3208 - Consider private browsing in contentPolicy module rather than filterStorage

lib/contentPolicy.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 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/>.
  */
 
 /**
  * @fileOverview Content policy implementation, responsible for blocking things.
  */
 
 "use strict";
 
 let {XPCOMUtils} = Cu.import("resource://gre/modules/XPCOMUtils.jsm", {});
 let {Services} = Cu.import("resource://gre/modules/Services.jsm", {});
+let {PrivateBrowsingUtils} = Cu.import("resource://gre/modules/PrivateBrowsingUtils.jsm", {});
 
 let {Utils} = require("utils");
 let {Prefs} = require("prefs");
 let {FilterStorage} = require("filterStorage");
 let {BlockingFilter, WhitelistFilter, RegExpFilter} = require("filterClasses");
 let {defaultMatcher} = require("matcher");
 let {objectMouseEventHander} = require("objectTabs");
 let {RequestNotifier} = require("requestNotifier");
 let {ElemHide} = require("elemHide");
 
@@ skipping 99 matching lines @@
     function cleanWindowLocation(wnd)
     {
       let url = getWindowLocation(wnd);
       let index = url.indexOf("#");
       if (index >= 0)
         url = url.substring(0, index);
 
       return url;
     }
 
+    function addHit(match)
+    {
+      if (!PrivateBrowsingUtils.isWindowPrivate(wnd))

[Thomas Greiner, 2015/11/02 17:29:23]

I'm wondering whether `wnd` here could potentially be of type `nsIDOMWindow` as
opposed to `nsIDOMChromeWindow` (which is what `isWindowPrivate` expects) since
that's not explicitly stated at
https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interfa....

Therefore, we may need to use `isBrowserPrivate` instead as recommended at
http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/PrivateBrowsing....

[Wladimir Palant, 2015/11/02 18:49:22]

Ouch, I overlooked the "Chrome" part, and documentation is outdated of course.
While this call works, the warning isn't intended of course.

I fixed this but the call here doesn't really matter - it is being removed a few
commits later. The call that will stay has been pushed already, I'll fix it in
https://codereview.adblockplus.org/29329554/.

+        FilterStorage.increaseHitCount(match);
+    }
+
     if (!match && Prefs.enabled)
     {
       let testWnd = wnd;
       let testSitekey = sitekey;
       let testSitekeyWnd = sitekeyWnd;
       let parentWndLocation = cleanWindowLocation(testWnd);
       while (true)
       {
         let testWndLocation = parentWndLocation;
         parentWndLocation = (testWnd == testWnd.parent ? testWndLocation : cleanWindowLocation(testWnd.parent));
         let parentDocDomain = getHostname(parentWndLocation);
 
         let typeMap = RegExpFilter.typeMap.DOCUMENT;
         if (contentType == "ELEMHIDE")
           typeMap = typeMap | RegExpFilter.typeMap.ELEMHIDE;
         let whitelistMatch = defaultMatcher.matchesAny(testWndLocation, typeMap, parentDocDomain, false, sitekey);
         if (whitelistMatch instanceof WhitelistFilter)
         {
-          FilterStorage.increaseHitCount(whitelistMatch, wnd);
+          addHit(whitelistMatch);
           RequestNotifier.addNodeData(testWnd.document, topWnd,
             (whitelistMatch.contentType & RegExpFilter.typeMap.DOCUMENT) ? "DOCUMENT" : "ELEMHIDE",
             parentDocDomain, false, testWndLocation, whitelistMatch);
           return true;
         }
 
         let genericType = (contentType == "ELEMHIDE" ? "GENERICHIDE" : "GENERICBLOCK");
         let nogenericMatch = defaultMatcher.matchesAny(testWndLocation,
             RegExpFilter.typeMap[genericType], parentDocDomain, false, testSitekey);
         if (nogenericMatch instanceof WhitelistFilter)
         {
           nogeneric = true;
 
-          FilterStorage.increaseHitCount(nogenericMatch, wnd);
+          addHit(nogenericMatch);
           RequestNotifier.addNodeData(testWnd.document, topWnd, genericType,
                                       parentDocDomain, false, testWndLocation,
                                       nogenericMatch);
         }
 
         if (testWnd.parent == testWnd)
           break;
 
         if (testWnd == testSitekeyWnd)
           [testSitekey, testSitekeyWnd] = getSitekey(testWnd.parent);
@@ skipping 13 matching lines @@
     {
       match = location;
       location = match.text.replace(/^.*?#/, '#');
 
       if (!match.isActiveOnDomain(docDomain))
         return true;
 
       let exception = ElemHide.getException(match, docDomain);
       if (exception)
       {
-        FilterStorage.increaseHitCount(exception, wnd);
+        addHit(exception);
         RequestNotifier.addNodeData(node, topWnd, contentType, docDomain, false, location, exception);
         return true;
       }
 
       if (nogeneric && match.isGeneric())
         return true;
     }
 
     let thirdParty = (contentType == "ELEMHIDE" ? false : isThirdParty(location, docDomain));
 
@@ skipping 12 matching lines @@
       if (!match && contentType == "OBJECT" && node.nodeType == Ci.nsIDOMNode.ELEMENT_NODE)
       {
         node.addEventListener("mouseover", objectMouseEventHander, true);
         node.addEventListener("mouseout", objectMouseEventHander, true);
       }
     }
 
     // Store node data
     RequestNotifier.addNodeData(node, topWnd, contentType, docDomain, thirdParty, location, match);
     if (match)
-      FilterStorage.increaseHitCount(match, wnd);
+      addHit(match);
 
     return !match || match instanceof WhitelistFilter;
   },
 
   /**
    * Checks whether the location's scheme is blockable.
    * @param location  {nsIURI}
    * @return {Boolean}
    */
   isBlockableScheme: function(location)
@@ skipping 416 matching lines @@
   if (!wnd || wnd.closed)
     return;
 
   if (entry.type == "OBJECT")
   {
     node.removeEventListener("mouseover", objectMouseEventHander, true);
     node.removeEventListener("mouseout", objectMouseEventHander, true);
   }
   Policy.processNode(wnd, node, entry.type, entry.location, true);
 }