Issue 1801 - Use URL objects to process URLs in the background page

chrome/ext/background.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/>.
  */
 
 (function()
 {
   /* Pages */
 
   var Page = ext.Page = function(tab)
   {
     this._id = tab.id;
-    this._url = tab.url;
 
     this.browserAction = new BrowserAction(tab.id);
     this.contextMenus = new ContextMenus(this);
+
+    // Usually Page objects are created from Chrome's Tab objects, which
+    // provide the url. So we can override the fallback implemented below.
+    if (tab.url)
+      Object.defineProperty(this, "url", {value: new URL(tab.url), enumerable: true});

[Wladimir Palant, 2015/02/09 12:54:29]

Code that belongs together should really be together. Please move that logic
back into the getter - with caching if you prefer, but I don't really see
caching being necessary here.

[Sebastian Noack, 2015/02/11 10:55:51]

Whatever.

   };
   Page.prototype = {
     get url()
     {
-      // usually our Page objects are created from Chrome's Tab objects, which
-      // provide the url. So we can return the url given in the constructor.
-      if (this._url != null)
-        return this._url;
-
-      // but sometimes we only have the tab id when we create a Page object.
-      // In that case we get the url from top frame of the tab, recorded by
-      // the onBeforeRequest handler.
+      // Sometimes we only have the tab id when we create a Page object.
+      // In that case we get the url from top frame of the tab, recorded
+      // by the onBeforeRequest handler.
       var frames = framesOfTabs[this._id];
       if (frames)
       {
         var frame = frames[0];
         if (frame)
           return frame.url;
       }
     },
     sendMessage: function(message, responseCallback)
     {
@@ skipping 207 matching lines @@
   {
     tabs.forEach(function(tab)
     {
       chrome.webNavigation.getAllFrames({tabId: tab.id}, function(details)
       {
         if (details && details.length > 0)
         {
           var frames = framesOfTabs[tab.id] = {__proto__: null};
 
           for (var i = 0; i < details.length; i++)
-            frames[details[i].frameId] = {url: details[i].url, parent: null};
+            frames[details[i].frameId] = {url: new URL(details[i].url), parent: null};
 
           for (var i = 0; i < details.length; i++)
           {
             var parentFrameId = details[i].parentFrameId;
 
             if (parentFrameId != -1)
               frames[details[i].frameId].parent = frames[parentFrameId];
           }
         }
       });
@@ skipping 19 matching lines @@
         details.frameId == 0 && !(details.tabId in framesOfTabs)
       );
 
       var frames = null;
       if (!isMainFrame)
         frames = framesOfTabs[details.tabId];
       if (!frames)
         frames = framesOfTabs[details.tabId] = {__proto__: null};
 
       var frame = null;
+      var url = new URL(details.url);
       if (!isMainFrame)
       {
         // we are looking for the frame that contains the element that
         // is about to load, however if a frame is loading the surrounding
         // frame is indicated by parentFrameId instead of frameId
         var frameId;
         if (requestType == "sub_frame")
           frameId = details.parentFrameId;
         else
           frameId = details.frameId;
 
         frame = frames[frameId] || frames[Object.keys(frames)[0]];
 
         if (frame)
         {
           // Since Chrome 38 requests of type 'object' (e.g. requests
           // initiated by Flash) are mistakenly reported with the type 'other'.
           // https://code.google.com/p/chromium/issues/detail?id=410382
           if (requestType == "other" && parseInt(navigator.userAgent.match(/\bChrome\/(\d+)/)[1], 10) >= 38)
             requestType = "object";
 
           var results = ext.webRequest.onBeforeRequest._dispatch(
-            details.url,
+            url,
             requestType,
             new Page({id: details.tabId}),
             frame
           );
 
           if (results.indexOf(false) != -1)
             return {cancel: true};
         }
       }
 
       if (isMainFrame || details.type == "sub_frame")
-        frames[details.frameId] = {url: details.url, parent: frame};
+        frames[details.frameId] = {url: url, parent: frame};
     }
     catch (e)
     {
       // recent versions of Chrome cancel the request when an error occurs in
       // the onBeforeRequest listener. However in our case it is preferred, to
       // let potentially some ads through, rather than blocking legit requests.
       console.error(e);
     }
   }, {urls: ["http://*/*", "https://*/*"]}, ["blocking"]);
 
 
   /* Message passing */
 
   chrome.runtime.onMessage.addListener(function(message, rawSender, sendResponse)
   {
     var sender = {
       page: new Page(rawSender.tab),
       frame: {
-        url: rawSender.url,
+        url: new URL(rawSender.url),
         get parent()
         {
           var frames = framesOfTabs[rawSender.tab.id];
 
           if (!frames)
             return null;
 
           if ("frameId" in rawSender)
           {
             // Chrome 41+
             var frame = frames[rawSender.frameId];
             if (frame)
               return frame.parent;
           }
           else
           {
             // Chrome 28-40
             for (var frameId in frames)
             {
-              if (frames[frameId].url == rawSender.url)
+              if (frames[frameId].url.href == this.url.href)
                 return frames[frameId].parent;
             }
           }
 
           return frames[0];
         }
       }
     };
 
     return ext.onMessage._dispatch(message, sender, sendResponse).indexOf(true) != -1;
@@ skipping 39 matching lines @@
             callback(new Page(tab));
         }
         else
         {
           ext.pages.open(optionsUrl, callback);
         }
       });
     });
   };
 })();