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

Unified Diff: lib/requestBlocker.js

Issue 29760707: Issue 6622 - Implement $rewrite filter option (Closed) Base URL: https://hg.adblockplus.org/adblockpluschrome/
Patch Set: Log all request, even failed rewrites. + comment Created May 4, 2018, 8:46 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 | « lib/devtools.js ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/requestBlocker.js
===================================================================
--- a/lib/requestBlocker.js
+++ b/lib/requestBlocker.js
@@ -65,17 +65,17 @@
for (let type in browser.webRequest.ResourceType)
yield resourceTypes.get(browser.webRequest.ResourceType[type]) || "OTHER";
// WEBRTC gets addressed through a workaround, even if the webRequest API is
// lacking support to block this kind of a request.
yield "WEBRTC";
- // POPUP, CSP and ELEMHIDE filters aren't mapped to resource types.
+ // These filter types aren't mapped to resource types.
yield "POPUP";
yield "ELEMHIDE";
yield "CSP";
}());
function getDocumentInfo(page, frame, originUrl)
{
return [
@@ -118,25 +118,25 @@
// an origin (proto + host).
else
return Promise.resolve([]);
return browser.tabs.query({url}).then(tabs => tabs.map(tab => tab.id));
}
function logRequest(tabIds, url, type, docDomain, thirdParty,
- sitekey, specificOnly, filter)
+ sitekey, specificOnly, filter, rewrittenTo)
{
if (filter)
FilterNotifier.emit("filter.hitCount", filter, 0, 0, tabIds);
devtools.logRequest(
tabIds, url, type, docDomain,
thirdParty, sitekey,
- specificOnly, filter
+ specificOnly, filter, rewrittenTo
);
}
browser.webRequest.onBeforeRequest.addListener(details =>
{
// Never block top-level documents.
if (details.type == "main_frame")
return;
@@ -189,24 +189,40 @@
return;
let type = resourceTypes.get(details.type) || "OTHER";
let [docDomain, sitekey, specificOnly] = getDocumentInfo(page, frame,
originUrl);
let [filter, urlString, thirdParty] = matchRequest(url, type, docDomain,
sitekey, specificOnly);
+ let result;
+ let rewritten;
Sebastian Noack 2018/05/05 13:46:53 Nit: Since these variables are no always assigned
hub 2018/05/07 20:13:04 In both cases they are undefined which is totally
+
+ if (filter instanceof BlockingFilter)
+ {
+ if (filter.rewrite)
+ {
+ rewritten = filter.rewriteUrl(urlString);
+ // if no rewrite happened (error, diff origin), we'll
Sebastian Noack 2018/05/05 13:46:53 Nit: Capitalize first letter of comment (if it is
hub 2018/05/07 20:13:04 Done.
+ // return undefined avoid an "infinite" loop.
Sebastian Noack 2018/05/05 13:46:53 Typo: ... in order to avoid ...
hub 2018/05/07 20:13:04 Done.
+ if (rewritten != urlString)
+ result = {redirectUrl: rewritten};
+ }
+ else
+ result = {cancel: true};
+ }
+
getRelatedTabIds(details).then(tabIds =>
{
logRequest(tabIds, urlString, type, docDomain,
- thirdParty, sitekey, specificOnly, filter);
+ thirdParty, sitekey, specificOnly, filter, rewritten);
Manish Jethani 2018/05/05 00:54:12 For what it's worth, I think we should do the logg
Sebastian Noack 2018/05/05 13:46:53 Without the logging changes, I consider the result
hub 2018/05/07 20:13:04 Without the UI patch, it will just be logged as a
});
- if (filter instanceof BlockingFilter)
- return {cancel: true};
+ return result;
}, {urls: ["<all_urls>"]}, ["blocking"]);
port.on("filters.collapse", (message, sender) =>
{
let {page, frame} = sender;
if (checkWhitelisted(page, frame))
return false;
« no previous file with comments | « lib/devtools.js ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld