Issue 3208 - Make Policy.processNode() accept plain strings instead of nsIURI

lib/contentPolicy.js

Index: lib/contentPolicy.js
===================================================================
--- a/lib/contentPolicy.js
+++ b/lib/contentPolicy.js
@@ -116,17 +116,17 @@ var Policy = exports.Policy =
     });
   },
 
   /**
    * Checks whether a node should be blocked, hides it if necessary
    * @param wnd {nsIDOMWindow}
    * @param node {nsIDOMElement}
    * @param contentType {String}
-   * @param location {nsIURI}
+   * @param location {String}
    * @param collapse {Boolean} true to force hiding of the node
    * @return {Boolean} false if the node should be blocked
    */
   processNode: function(wnd, node, contentType, location, collapse)
   {
     let topWnd = wnd.top;
     if (!topWnd || !topWnd.location || !topWnd.location.href)
       return true;
@@ -202,43 +202,41 @@ var Policy = exports.Policy =
     // Data loaded by plugins should be attached to the document
     if (contentType == "OBJECT_SUBREQUEST" && node instanceof Ci.nsIDOMElement)
       node = node.ownerDocument;
 
     // Fix type for objects misrepresented as frames or images
     if (contentType != "OBJECT" && (node instanceof Ci.nsIDOMHTMLObjectElement || node instanceof Ci.nsIDOMHTMLEmbedElement))
       contentType = "OBJECT";
 
-    let locationText = location.spec;
     if (!match && contentType == "ELEMHIDE")
     {
       match = location;
-      locationText = match.text.replace(/^.*?#/, '#');
-      location = locationText;
+      location = match.text.replace(/^.*?#/, '#');

[tschuster, 2015/10/31 11:55:23]

Wait, how does this work? 
On the right hand side match is location and location is a String. So match.text
is just undefined?

[Wladimir Palant, 2015/11/01 12:18:09]

Yes, this is quite a hack. Calls with content type ELEMHIDE come from elemHide
module rather than the content policy. Since those don't really have a location
associated with them, a Filter instance is being passed in as location
parameter. In https://issues.adblockplus.org/ticket/3251 we should make it a
little less hacky - location will be a key extracted from the elemhide channel
URI, and the code here will resolve it into a filter.

 
       if (!match.isActiveOnDomain(docDomain))
         return true;
 
       let exception = ElemHide.getException(match, docDomain);
       if (exception)
       {
         FilterStorage.increaseHitCount(exception, wnd);
-        RequestNotifier.addNodeData(node, topWnd, contentType, docDomain, false, locationText, 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));
 
     if (!match && Prefs.enabled && RegExpFilter.typeMap.hasOwnProperty(contentType))
     {
-      match = defaultMatcher.matchesAny(locationText, RegExpFilter.typeMap[contentType],
+      match = defaultMatcher.matchesAny(location, RegExpFilter.typeMap[contentType],
                                         docDomain, thirdParty, sitekey, nogeneric);
       if (match instanceof BlockingFilter && node.ownerDocument && !nonVisualTypes.has(contentType))
       {
         let prefCollapse = (match.collapse != null ? match.collapse : !Prefs.fastcollapse);
         if (collapse || prefCollapse)
           schedulePostProcess(node);
       }
 
@@ -246,17 +244,17 @@ var Policy = exports.Policy =
       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, locationText, match);
+    RequestNotifier.addNodeData(node, topWnd, contentType, docDomain, thirdParty, location, match);
     if (match)
       FilterStorage.increaseHitCount(match, wnd);
 
     return !match || match instanceof WhitelistFilter;
   },
 
   /**
    * Checks whether the location's scheme is blockable.
@@ -385,17 +383,17 @@ var PolicyImplementation =
     if (!wnd)
       return Ci.nsIContentPolicy.ACCEPT;
 
     // Ignore whitelisted schemes
     let location = Utils.unwrapURL(contentLocation);
     if (!Policy.isBlockableScheme(location))
       return Ci.nsIContentPolicy.ACCEPT;
 
-    let result = Policy.processNode(wnd, node, types.get(contentType), location, false);
+    let result = Policy.processNode(wnd, node, types.get(contentType), location.spec, false);
     return (result ? Ci.nsIContentPolicy.ACCEPT : Ci.nsIContentPolicy.REJECT_REQUEST);
   },
 
   shouldProcess: function(contentType, contentLocation, requestOrigin, insecNode, mimeType, extra)
   {
     return Ci.nsIContentPolicy.ACCEPT;
   },
 
@@ -406,34 +404,34 @@ var PolicyImplementation =
   {
     switch (topic)
     {
       case "content-document-global-created":
       {
         if (!(subject instanceof Ci.nsIDOMWindow) || !subject.opener)
           return;
 
-        let uri = additional || Utils.makeURI(subject.location.href);
+        let uri = additional || subject.location.href;
         if (!Policy.processNode(subject.opener, subject.opener.document, "POPUP", uri, false))
         {
           subject.stop();
           Utils.runAsync(() => subject.close());
         }
-        else if (uri.spec == "about:blank")
+        else if (uri == "about:blank")
         {
           // An about:blank pop-up most likely means that a load will be
           // initiated asynchronously. Wait for that.
           Utils.runAsync(function()
           {
             let channel = subject.QueryInterface(Ci.nsIInterfaceRequestor)
                                  .getInterface(Ci.nsIDocShell)
                                  .QueryInterface(Ci.nsIDocumentLoader)
                                  .documentChannel;
             if (channel)
-              this.observe(subject, topic, data, channel.URI);
+              this.observe(subject, topic, data, channel.URI.spec);
           });
         }
         break;
       }
     }
   },
 
   //
@@ -458,23 +456,23 @@ var PolicyImplementation =
 
       if (contentType == Ci.nsIContentPolicy.TYPE_DOCUMENT)
       {
         if (wnd.history.length <= 1 && wnd.opener)
         {
           // Special treatment for pop-up windows. Note that we might not have
           // seen the original channel yet because the redirect happened before
           // the async code in observe() had a chance to run.
-          this.observe(wnd, "content-document-global-created", null, oldChannel.URI);
-          this.observe(wnd, "content-document-global-created", null, newChannel.URI);
+          this.observe(wnd, "content-document-global-created", null, oldChannel.URI.spec);
+          this.observe(wnd, "content-document-global-created", null, newChannel.URI.spec);
         }
         return;
       }
 
-      if (!Policy.processNode(wnd, wnd.document, types.get(contentType), newChannel.URI, false))
+      if (!Policy.processNode(wnd, wnd.document, types.get(contentType), newChannel.URI.spec, false))
         result = Cr.NS_BINDING_ABORTED;
     }
     catch (e)
     {
       // We shouldn't throw exceptions here - this will prevent the redirect.
       Cu.reportError(e);
     }
     finally
@@ -646,32 +644,33 @@ function getWindowLocation(wnd)
 
   // Firefox branch
   return wnd.location.href;
 }
 
 /**
  * Checks whether the location's origin is different from document's origin.
  */
-function isThirdParty(/**nsIURI*/location, /**String*/ docDomain) /**Boolean*/
+function isThirdParty(/**String*/location, /**String*/ docDomain) /**Boolean*/
 {
   if (!location || !docDomain)
     return true;
 
+  let uri = Utils.makeURI(location);
   try
   {
-    return Utils.effectiveTLD.getBaseDomain(location) != Utils.effectiveTLD.getBaseDomainFromHost(docDomain);
+    return Utils.effectiveTLD.getBaseDomain(uri) != Utils.effectiveTLD.getBaseDomainFromHost(docDomain);
   }
   catch (e)
   {
     // EffectiveTLDService throws on IP addresses, just compare the host name
     let host = "";
     try
     {
-      host = location.host;
+      host = uri.host;
     } catch (e) {}
     return host != docDomain;
   }
 }
 
 /**
  * Re-checks filters on an element.
  */
@@ -681,10 +680,10 @@ function refilterNode(/**Node*/ node, /*
   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, Utils.makeURI(entry.location), true);
+  Policy.processNode(wnd, node, entry.type, entry.location, true);
 }