Issue 3208 - Don`t keep track of content policy for redirects, Gecko will do that for us

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
@@ skipping 369 matching lines @@
    */
   init: function()
   {
     let registrar = Components.manager.QueryInterface(Ci.nsIComponentRegistrar);
     registrar.registerFactory(this.classID, this.classDescription, this.contractID, this);
 
     let catMan = Utils.categoryManager;
     for (let category of this.xpcom_categories)
       catMan.addCategoryEntry(category, this.contractID, this.contractID, false, true);
 
-    // http-on-opening-request is new in Gecko 18, http-on-modify-request can
-    // be used in earlier releases.
-    let httpTopic = "http-on-opening-request";
-    if (Services.vc.compare(Utils.platformVersion, "18.0") < 0)
-      httpTopic = "http-on-modify-request";
-
-    Services.obs.addObserver(this, httpTopic, true);
     Services.obs.addObserver(this, "content-document-global-created", true);
 
     onShutdown.add(function()
     {
-      Services.obs.removeObserver(this, httpTopic);
       Services.obs.removeObserver(this, "content-document-global-created");
 
       for (let category of this.xpcom_categories)
         catMan.deleteCategoryEntry(category, this.contractID, false);
 
       registrar.unregisterFactory(this.classID, this);
-
-      this.previousRequest = null;
     }.bind(this));
   },
 
   //
   // nsISupports interface implementation
   //
 
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIContentPolicy, Ci.nsIObserver,
     Ci.nsIChannelEventSink, Ci.nsIFactory, Ci.nsISupportsWeakReference]),
 
@@ skipping 18 matching lines @@
     // Ignore whitelisted schemes
     let location = Utils.unwrapURL(contentLocation);
     if (!Policy.isBlockableScheme(location))
       return Ci.nsIContentPolicy.ACCEPT;
 
     // Interpret unknown types as "other"
     if (!(contentType in Policy.typeDescr))
       contentType = Policy.type.OTHER;
 
     let result = Policy.processNode(wnd, node, contentType, location, false);
-    if (result)
-    {
-      // We didn't block this request so we will probably see it again in
-      // http-on-opening-request. Keep it so that we can associate it with the
-      // channel there - will be needed in case of redirect.
-      this.previousRequest = [location, contentType];
-    }
     return (result ? Ci.nsIContentPolicy.ACCEPT : Ci.nsIContentPolicy.REJECT_REQUEST);
   },
 
   shouldProcess: function(contentType, contentLocation, requestOrigin, insecNode, mimeType, extra)
   {
     return Ci.nsIContentPolicy.ACCEPT;
   },
 
   //
   // nsIObserver interface implementation
@@ skipping 22 matching lines @@
             let channel = subject.QueryInterface(Ci.nsIInterfaceRequestor)
                                  .getInterface(Ci.nsIDocShell)
                                  .QueryInterface(Ci.nsIDocumentLoader)
                                  .documentChannel;
             if (channel)
               this.observe(subject, topic, data, channel.URI);
           });
         }
         break;
       }
-      case "http-on-opening-request":
-      case "http-on-modify-request":
-      {
-        if (!(subject instanceof Ci.nsIHttpChannel))
-          return;
-
-        if (this.previousRequest && subject.URI == this.previousRequest[0] &&
-            subject instanceof Ci.nsIWritablePropertyBag)
-        {
-          // We just handled a content policy call for this request - associate
-          // the data with the channel so that we can find it in case of a redirect.
-          subject.setProperty("abpRequestType", this.previousRequest[1]);
-          this.previousRequest = null;
-        }
-        break;
-      }
     }
   },
 
   //
   // nsIChannelEventSink interface implementation
   //
 
   asyncOnChannelRedirect: function(oldChannel, newChannel, flags, callback)
   {
     let result = Cr.NS_OK;
     try
     {
-      // Try to retrieve previously stored request data from the channel
-      let contentType;
-      if (oldChannel instanceof Ci.nsIWritablePropertyBag)
-      {
-        try
-        {
-          contentType = oldChannel.getProperty("abpRequestType");
-        }
-        catch(e)
-        {
-          // No data attached, ignore this redirect
-          return;
-        }
-      }
+      // nsILoadInfo.contentPolicyType was introduced in Gecko 35, then
+      // renamed to nsILoadInfo.externalContentPolicyType in Gecko 44.
+      let loadInfo = oldChannel.loadInfo;
+      let contentType = loadInfo.externalContentPolicyType || loadInfo.contentPolicyType;

[tschuster, 2015/10/29 14:02:03]

I would prefer an explicit `"externalContentPolicyType" in loadInfo` check.
(There is no bug at the moment, because 0 = Invalid)

[Wladimir Palant, 2015/10/29 18:22:47]

Done.

+      if (!contentType)
+        return;
 
       let newLocation = null;
       try
       {
         newLocation = newChannel.URI;
       } catch(e2) {}
       if (!newLocation)
         return;
 
       let wnd = Utils.getRequestWindow(newChannel);
@@ skipping 222 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);
 }