Simplified and removed memory leaks from code dealing with frames on Chrome

chrome/ext/background.js

Index: chrome/ext/background.js
===================================================================
--- a/chrome/ext/background.js
+++ b/chrome/ext/background.js
@@ -191,128 +191,101 @@
   };
 
 
-  /* Frames */
+  /* Web requests */
 
   var framesOfTabs = {__proto__: null};
 
-  var Frame = ext.Frame = function(params)
+  ext.getFrame = function(tabId, frameId)
   {
-    this._frameId = params.frameId;
-    this._tabId = params.tabId;
-    this._url = params.url;
+    return framesOfTabs[tabId][frameId];

[Wladimir Palant, 2014/03/06 15:32:20]

What about tabs we don't know?

[Sebastian Noack, 2014/03/06 19:54:49]

Done.

   };
-  Frame.prototype = {
-    get url()
-    {
-      if (this._url != null)
-        return this._url;
-
-      var frames = framesOfTabs[this._tabId];
-      if (frames)
-      {
-        var frame = frames[this._frameId];
-        if (frame)
-          return frame.url;
-      }
-    },
-    get parent()
-    {
-      var frames = framesOfTabs[this._tabId];
-      if (frames)
-      {
-        var frame;
-        if (this._frameId != null)
-          frame = frames[this._frameId];
-        else
-        {
-          // the frame ID wasn't available when we created
-          // the Frame object (e.g. for the onMessage event),
-          // so we have to find the frame details by their URL.
-          for (var frameId in frames)
-          {
-            if (frames[frameId].url == this._url)
-            {
-              frame = frames[frameId];
-              break;
-            }
-          }
-        }
-
-        if (!frame || frame.parent == -1)
-          return null;
-
-        return new Frame({frameId: frame.parent, tabId: this._tabId});
-      }
-    }
-  };
-
-
-  /* Web requests */
 
   ext.webRequest = {
     onBeforeRequest: new ext._EventTarget(true),
     handlerBehaviorChanged: chrome.webRequest.handlerBehaviorChanged
   };
 
-  chrome.webRequest.onBeforeRequest.addListener(function(details)
+  chrome.tabs.query({}, function(tabs)
   {
-    // the high-level code isn't interested in requests that aren't related
-    // to a tab and since those can only be handled in Chrome, we ignore
-    // them here instead of in the browser independent high-level code.
-    if (details.tabId == -1)
-      return;
+    // unfortunatelly we can't retrieve frames already loaded.

[Sebastian Noack, 2014/03/06 19:54:49]

Cool. Done.

+    // But we can retrieve the top level url of every tab and
+    // use it as fallback when we encounter an unknown frame.
+    for (var i = 0; i < tabs.length; i++)

[Wladimir Palant, 2014/03/06 15:32:20]

Nit: Please always put brackets around multiline blocks, for better readability.

+      framesOfTabs[tabs[i].id] = {
+        __proto__: null,
 
-    var page = new Page({id: details.tabId});
-    var frames = framesOfTabs[details.tabId];
+        0: {
+          url: tabs[i].url,
+          parent: null
+        }
+      };
 
-    if (!frames)
+    chrome.webRequest.onBeforeRequest.addListener(function(details)
     {
-      frames = framesOfTabs[details.tabId] = [];
-
-      // assume that the first request belongs to the top frame. Chrome
-      // may give the top frame the type "object" instead of "main_frame".
-      // https://code.google.com/p/chromium/issues/detail?id=281711
-      if (frameId == 0)
-        details.type = "main_frame";
-    }
-
-    var frameId;
-    if (details.type == "main_frame" || details.type == "sub_frame")
-    {
-      frameId = details.parentFrameId;
-      frames[details.frameId] = {url: details.url, parent: frameId};
-
-      // the high-level code isn't interested in top frame requests and
-      // since those can only be handled in Chrome, we ignore them here
-      // instead of in the browser independent high-level code.
-      if (details.type == "main_frame")
-        return;
-    }
-    else
-      frameId = details.frameId;
-
-    if (!(frameId in frames))
-    {
-      // the high-level code relies on the frame. So ignore the request if we
-      // don't even know the top-level frame. That can happen for example when
-      // the extension was just (re)loaded.
-      if (!(0 in frames))
+      if (details.tabId == -1)
         return;
 
-      // however when the src of the frame is a javascript: or data: URL, we
-      // don't know the frame either. But since we know the top-level frame we
-      // can just pretend that we are in the top-level frame, in order to have
-      // at least most domain-based filter rules working.
-      frameId = 0;
-      if (details.type == "sub_frame")
-        frames[details.frameId].parent = frameId;
-    }
+      var isMainFrame = details.type == "main_frame" || (
+        // assume that the first request belongs to the top frame. Chrome
+        // may give the top frame the type "object" instead of "main_frame".
+        // https://code.google.com/p/chromium/issues/detail?id=281711
+        details.frameId == 0 && !(details.tabId in framesOfTabs)
+      );
 
-    var frame = new Frame({frameId: frameId, tabId: details.tabId});
+      var frames;
+      var frame;
+      if (isMainFrame)
+      {
+        frames = framesOfTabs[details.tabId] = {__proto__: null};
+        frame = null;
+      }
+      else
+      {
+        // 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 (details.type == "sub_frame")
+          frameId = details.parentFrameId;
+        else
+          frameId = details.frameId;
 
-    if (!ext.webRequest.onBeforeRequest._dispatch(details.url, details.type, page, frame))
-      return {cancel: true};
-  }, {urls: ["<all_urls>"]}, ["blocking"]);
+        frames = framesOfTabs[details.tabId];
+        frame = frames[frameId] || frames[0];
+
+        if (!ext.webRequest.onBeforeRequest._dispatch(details.url, details.type, new Page({id: details.tabId}), frame))
+          return {cancel: true};
+      }
+
+      if (isMainFrame || details.type == "sub_frame")
+        frames[details.frameId] = {url: details.url, parent: frame};
+    }, {urls: ["<all_urls>"]}, ["blocking"]);
+
+
+    /* Message passing */
+
+    ext._setupMessageListener(function(sender)
+    {
+      return {
+        page: new Page(sender.tab),
+        frame: {
+          url: sender.url,
+          get parent()
+          {
+            var frames = framesOfTabs[sender.tab.id];
+
+            for (var frameId in frames)
+            {
+              if (frames[frameId].url == sender.url)
+                return frames[frameId].parent;
+            }
+
+            return frames[0];
+          }
+        }
+      };
+    });
+  });
 
 
   /* Context menus */
@@ -366,15 +339,4 @@
       isContextMenuHidden = true;
     }
   };
-
-
-  /* Message passing */
-
-  ext._setupMessageListener(function(sender)
-  {
-    return {
-      page: new Page(sender.tab),
-      frame: new Frame({url: sender.url, tabId: sender.tab.id})
-    };
-  });
 })();