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: Addressed comments Created March 6, 2014, 7:35 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,127 +191,82 @@
};
- /* 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];
};
- 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.tabs.query({}, function(tabs)
+ {
+ 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++)
+ {
+ var parentFrameId = details[i].parentFrameId;
+
+ frames[details[i].frameId] = {
+ url: details[i].url,
+ parent: parentFrameId != -1 ? frames[parentFrameId] : null
Wladimir Palant 2014/03/06 20:58:18 I don't think that parent frames are guaranteed to
Sebastian Noack 2014/03/06 21:11:55 I tested it and it lists the frames top-down. But
Wladimir Palant 2014/03/21 14:17:35 I think a second pass is exactly what we should im
Sebastian Noack 2014/03/31 15:43:44 Done.
+ };
+ }
+ }
+ });
+ });
+ });
+
chrome.webRequest.onBeforeRequest.addListener(function(details)
{
- // 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;
- var page = new Page({id: details.tabId});
- var frames = framesOfTabs[details.tabId];
-
- if (!frames)
- {
- frames = framesOfTabs[details.tabId] = [];
-
+ 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
- if (frameId == 0)
- details.type = "main_frame";
+ 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;
+ 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 (details.type == "sub_frame")
+ frameId = details.parentFrameId;
+ else
+ frameId = details.frameId;
+
+ frame = frames[frameId] || frames[0];
+
+ if (frame && !ext.webRequest.onBeforeRequest._dispatch(details.url, details.type, new Page({id: details.tabId}), frame))
+ return {cancel: true};
}
- 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))
- 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 frame = new Frame({frameId: frameId, tabId: details.tabId});
-
- if (!ext.webRequest.onBeforeRequest._dispatch(details.url, details.type, page, frame))
- return {cancel: true};
+ if (isMainFrame || details.type == "sub_frame")
+ frames[details.frameId] = {url: details.url, parent: frame};
}, {urls: ["<all_urls>"]}, ["blocking"]);
@@ -374,7 +329,24 @@
{
return {
page: new Page(sender.tab),
- frame: new Frame({url: sender.url, tabId: sender.tab.id})
+ frame: {
+ url: sender.url,
+ get parent()
+ {
+ var frames = framesOfTabs[sender.tab.id];
+
+ if (!frames)
+ return null;
+
+ for (var frameId in frames)
+ {
+ if (frames[frameId].url == sender.url)
+ return frames[frameId].parent;
+ }
+
+ return frames[0];
+ }
+ }
};
});
})();
« no previous file with comments | « no previous file | popupBlocker.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld