Issue 3208 - Unify handling of $document and $elemhide exceptions and improve performance

lib/contentPolicy.js

Index: lib/contentPolicy.js
===================================================================
--- a/lib/contentPolicy.js
+++ b/lib/contentPolicy.js
@@ -169,39 +169,54 @@ var Policy = exports.Policy =
 
     let originWindow = Utils.getOriginWindow(wnd);
     let wndLocation = originWindow.location.href;
     let docDomain = getHostname(wndLocation);
     let match = null;
     let [sitekey, sitekeyWnd] = getSitekey(wnd);
     let nogeneric = false;
 
+    function cleanWindowLocation(wnd)

[Thomas Greiner, 2015/11/10 18:22:33]

This function doesn't need to be inside `processNode` so better move it outside
to declare it only once.

[Wladimir Palant, 2015/11/10 19:13:15]

Given that this is a temporary solution I'd rather not do this - this job will
be done by getFrames() in child/utils module.

+    {
+      let url = getWindowLocation(wnd);
+      let index = url.indexOf("#");
+      if (index >= 0)
+        url = url.substring(0, index);
+
+      return url;
+    }
+
     if (!match && Prefs.enabled)
     {
       let testWnd = wnd;
       let testSitekey = sitekey;
       let testSitekeyWnd = sitekeyWnd;
-      let parentWndLocation = getWindowLocation(testWnd);
+      let parentWndLocation = cleanWindowLocation(testWnd);
       while (true)
       {
         let testWndLocation = parentWndLocation;
-        parentWndLocation = (testWnd == testWnd.parent ? testWndLocation : getWindowLocation(testWnd.parent));
-        match = Policy.isWhitelisted(testWndLocation, parentWndLocation, testSitekey);
+        parentWndLocation = (testWnd == testWnd.parent ? testWndLocation : cleanWindowLocation(testWnd.parent));
+        let parentDocDomain = getHostname(parentWndLocation);
 
-        if (match instanceof WhitelistFilter)
+        let typeMap = RegExpFilter.typeMap.DOCUMENT;
+        if (contentType == Policy.type.ELEMHIDE)
+          typeMap = typeMap | RegExpFilter.typeMap.ELEMHIDE;
+        let whitelistMatch = defaultMatcher.matchesAny(testWndLocation, typeMap, parentDocDomain, false, sitekey);

[Thomas Greiner, 2015/11/10 18:22:34]

`sitekey` is the bottom window's sitekey so shouldn't we instead pass the parent
window's sitekey since we're checking a request that originated from the parent
window?

[Wladimir Palant, 2015/11/10 19:13:15]

Yes, this should really be testSitekey - this was a bug in the $elemhide
whitelisting code I merged here. I fixed this in
https://codereview.adblockplus.org/29329571/ but I'll move that change here
tomorrow.

[Wladimir Palant, 2015/11/11 07:54:20]

Done.

+        if (whitelistMatch instanceof WhitelistFilter)
         {
-          FilterStorage.increaseHitCount(match, wnd);
-          RequestNotifier.addNodeData(testWnd.document, topWnd, Policy.type.DOCUMENT, getHostname(parentWndLocation), false, testWndLocation, match);
+          FilterStorage.increaseHitCount(whitelistMatch, wnd);
+          RequestNotifier.addNodeData(testWnd.document, topWnd,
+            (whitelistMatch.contentType & RegExpFilter.typeMap.DOCUMENT) ? Policy.type.DOCUMENT : Policy.type.ELEMHIDE,
+            parentDocDomain, false, testWndLocation, whitelistMatch);
           return true;
         }
 
         let genericType = (contentType == Policy.type.ELEMHIDE ?
                            Policy.type.GENERICHIDE :
                            Policy.type.GENERICBLOCK);
-        let parentDocDomain = getHostname(parentWndLocation);
         let nogenericMatch = defaultMatcher.matchesAny(testWndLocation,
             Policy.typeMask[genericType], parentDocDomain, false, testSitekey);
         if (nogenericMatch instanceof WhitelistFilter)
         {
           nogeneric = true;
 
           FilterStorage.increaseHitCount(nogenericMatch, wnd);
           RequestNotifier.addNodeData(testWnd.document, topWnd, genericType,
@@ -224,37 +239,16 @@ var Policy = exports.Policy =
 
     // Fix type for objects misrepresented as frames or images
     if (contentType != Policy.type.OBJECT && (node instanceof Ci.nsIDOMHTMLObjectElement || node instanceof Ci.nsIDOMHTMLEmbedElement))
       contentType = Policy.type.OBJECT;
 
     let locationText = location.spec;
     if (!match && contentType == Policy.type.ELEMHIDE)
     {
-      let testWnd = wnd;
-      let parentWndLocation = getWindowLocation(testWnd);
-      while (true)
-      {
-        let testWndLocation = parentWndLocation;
-        parentWndLocation = (testWnd == testWnd.parent ? testWndLocation : getWindowLocation(testWnd.parent));
-        let parentDocDomain = getHostname(parentWndLocation);
-        match = defaultMatcher.matchesAny(testWndLocation, RegExpFilter.typeMap.ELEMHIDE, parentDocDomain, false, sitekey);
-        if (match instanceof WhitelistFilter)
-        {
-          FilterStorage.increaseHitCount(match, wnd);
-          RequestNotifier.addNodeData(testWnd.document, topWnd, contentType, parentDocDomain, false, testWndLocation, match);
-          return true;
-        }
-
-        if (testWnd.parent == testWnd)
-          break;
-        else
-          testWnd = testWnd.parent;
-      }
-
       match = location;
       locationText = match.text.replace(/^.*?#/, '#');
       location = locationText;
 
       if (!match.isActiveOnDomain(docDomain))
         return true;
 
       let exception = ElemHide.getException(match, docDomain);