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

chrome/ext/background.js

Index: chrome/ext/background.js
===================================================================
--- a/chrome/ext/background.js
+++ b/chrome/ext/background.js
@@ -22,22 +22,21 @@
   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)
       {
@@ -265,7 +264,7 @@
           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++)
           {
@@ -305,6 +304,7 @@
         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
@@ -327,7 +327,7 @@
             requestType = "object";
 
           var results = ext.webRequest.onBeforeRequest._dispatch(
-            details.url,
+            url,
             requestType,
             new Page({id: details.tabId}),
             frame
@@ -339,7 +339,7 @@
       }
 
       if (isMainFrame || details.type == "sub_frame")
-        frames[details.frameId] = {url: details.url, parent: frame};
+        frames[details.frameId] = {url: url, parent: frame};
     }
     catch (e)
     {
@@ -358,7 +358,7 @@
     var sender = {
       page: new Page(rawSender.tab),
       frame: {
-        url: rawSender.url,
+        url: new URL(rawSender.url),
         get parent()
         {
           var frames = framesOfTabs[rawSender.tab.id];
@@ -378,7 +378,7 @@
             // 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;
             }
           }