Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: chrome/ext/background.js

Issue 5133931611422720: Simplified and removed memory leaks from code dealing with frames on Chrome (Closed)
Patch Set: Created Feb. 25, 2014, 5:52 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | popupBlocker.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 b
+ 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})
- };
- });
})();
« no previous file with comments | « no previous file | popupBlocker.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld