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

Unified Diff: lib/scriptInjection.js

Issue 29737561: Issue 6539, 6782 - Implement support for snippets (Closed) Base URL: https://hg.adblockplus.org/adblockpluschrome/
Patch Set: Rename ContentFiltering to Content Created July 19, 2018, 1:21 a.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
« include.preload.js ('K') | « include.preload.js ('k') | metadata.chrome » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/scriptInjection.js
===================================================================
rename from lib/cssInjection.js
rename to lib/scriptInjection.js
--- a/lib/cssInjection.js
+++ b/lib/scriptInjection.js
@@ -10,39 +10,43 @@
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.
*/
-/** @module cssInjection */
+/** @module scriptInjection */
kzar 2018/07/19 12:24:19 Be careful that Mercurial knows you've renamed lib
kzar 2018/07/19 12:24:20 Like I mentioned on one of the other reviews, I do
Manish Jethani 2018/07/19 12:55:24 Fair enough. Since we're referring to it as "cont
Manish Jethani 2018/07/19 12:55:25 Thanks, I'll try to do this correctly.
kzar 2018/07/19 13:28:55 Sounds good.
Manish Jethani 2018/07/19 14:04:35 Done.
"use strict";
const {RegExpFilter} = require("../adblockpluscore/lib/filterClasses");
const {ElemHide} = require("../adblockpluscore/lib/elemHide");
const {ElemHideEmulation} = require("../adblockpluscore/lib/elemHideEmulation");
+const {Snippets, compileScript} = require("../adblockpluscore/lib/snippets");
const {checkWhitelisted} = require("./whitelisting");
const {extractHostFromFrame} = require("./url");
const {port} = require("./messaging");
const {HitLogger} = require("./hitLogger");
const info = require("info");
// Chromium's support for tabs.removeCSS is still a work in progress and the
// API is likely to be different from Firefox's; for now we just don't use it
// at all, even if it's available.
// See https://crbug.com/608854
const styleSheetRemovalSupported = info.platform == "gecko";
const selectorGroupSize = 1024;
let userStyleSheetsSupported = true;
+let snippetsLibrarySource = "";
+let executableCode = new Map();
+
function* splitSelectors(selectors)
{
// Chromium's Blink engine supports only up to 8,192 simple selectors, and
// even fewer compound selectors, in a rule. The exact number of selectors
// that would work depends on their sizes (e.g. "#foo .bar" has a size of 2).
// Since we don't know the sizes of the selectors here, we simply split them
// into groups of 1,024, based on the reasonable assumption that the average
// selector won't have a size greater than 8. The alternative would be to
@@ -90,17 +94,17 @@
userStyleSheetsSupported = false;
// For other errors, we simply return false to indicate failure.
//
// One common error that occurs frequently is when a frame is not found
// (e.g. "Error: No frame with id 574 in tab 266"), which can happen when
// the code in the parent document has removed the frame before the
// background page has had a chance to respond to the content script's
- // "elemhide.getSelectors" message. We simply ignore such errors, because
+ // "content.applyFilters" message. We simply ignore such errors, because
// otherwise they show up in the log too often and make debugging
// difficult.
//
// Also note that the missing frame error is thrown synchronously on
// Firefox, while on Chromium it is an asychronous promise rejection. In
// the latter case, we cannot indicate failure to the caller, but we still
// explicitly ignore the error.
return false;
@@ -155,35 +159,84 @@
// style sheet now.
if (oldStyleSheet && oldStyleSheet != styleSheet)
removeStyleSheet(tabId, frameId, oldStyleSheet);
frame.injectedStyleSheets.set(groupName, styleSheet);
return true;
}
-port.on("elemhide.getSelectors", (message, sender) =>
+function getExecutableCode(script)
+{
+ let code = executableCode.get(script);
+ if (code)
+ return code;
+
+ code = compileScript(script, [snippetsLibrarySource]);
+
+ executableCode.set(script, code);
+ return code;
+}
+
+function executeScript(script, tabId, frameId)
+{
+ try
+ {
+ browser.tabs.executeScript(tabId, {
+ code: getExecutableCode(script),
+ frameId,
+ matchAboutBlank: true,
+ runAt: "document_start"
+ })
+ .catch(error =>
+ {
+ // Sometimes a frame is added and removed very quickly, in such cases we
+ // simply ignore the error.
+ if (error.message == "The frame was removed.")
+ return;
+
+ throw error;
+ });
+ }
+ catch (error)
kzar 2018/07/19 12:24:21 So we re-throw the exception, catch it again, then
Manish Jethani 2018/07/19 12:55:24 No, the error thrown from the .catch() handler is
kzar 2018/07/19 13:28:55 OK.
Manish Jethani 2018/07/19 14:04:34 It's the "The frame was removed." error.
kzar 2018/07/19 14:52:34 That's the above catch. I thought we were talking
Manish Jethani 2018/07/19 15:33:38 Ah, I see what you mean. Yeah, I probably just add
kzar 2018/07/19 15:37:45 Yea, ideally there would be an entry in the develo
Manish Jethani 2018/07/19 15:44:05 Can we simply log the error? catch (error) {
Manish Jethani 2018/07/19 15:54:08 OK, so when a snippet throws an error that error i
kzar 2018/07/19 16:07:40 Alright, fair enough.
+ {
+ }
+}
+
+port.on("content.applyFilters", (message, sender) =>
{
let selectors = [];
let emulatedPatterns = [];
let trace = HitLogger.hasListener(sender.page.id);
let inline = !userStyleSheetsSupported;
+ let {elemhide = false, snippets = false} = message.filterTypes ||
kzar 2018/07/19 12:24:19 Is this syntax supported by the browsers we suppor
Manish Jethani 2018/07/19 12:55:25 Yeah, that's just object destructuring with defaul
Manish Jethani 2018/07/19 14:04:35 Done.
+ {elemhide: true, snippets: true};
+
if (!checkWhitelisted(sender.page, sender.frame, null,
- RegExpFilter.typeMap.DOCUMENT |
- RegExpFilter.typeMap.ELEMHIDE))
+ RegExpFilter.typeMap.DOCUMENT))
{
let hostname = extractHostFromFrame(sender.frame);
- let specificOnly = checkWhitelisted(sender.page, sender.frame, null,
- RegExpFilter.typeMap.GENERICHIDE);
+
+ if (snippets)
+ {
+ for (let script of Snippets.getScriptsForDomain(hostname))
+ executeScript(script, sender.page.id, sender.frame.id);
+ }
- selectors = ElemHide.getSelectorsForDomain(hostname, specificOnly);
+ if (elemhide && !checkWhitelisted(sender.page, sender.frame, null,
+ RegExpFilter.typeMap.ELEMHIDE))
kzar 2018/07/19 12:24:20 It kind of sucks we have to call checkWhitelisted
Manish Jethani 2018/07/19 12:55:24 #@# exception will be applied within Snippets.getS
kzar 2018/07/19 13:28:57 Alright, fair enough I guess.
+ {
+ let specificOnly = checkWhitelisted(sender.page, sender.frame, null,
kzar 2018/07/19 12:24:18 If we don't check for generichide/genericblock for
Manish Jethani 2018/07/19 12:55:25 Currently there are no such checks. I'll mention t
kzar 2018/07/19 13:28:54 Yea, I definitely think so. Please could you open
Manish Jethani 2018/07/19 14:04:35 Done. https://issues.adblockplus.org/ticket/6797
kzar 2018/07/19 14:52:34 Thanks
+ RegExpFilter.typeMap.GENERICHIDE);
+ selectors = ElemHide.getSelectorsForDomain(hostname, specificOnly);
- for (let filter of ElemHideEmulation.getRulesForDomain(hostname))
- emulatedPatterns.push({selector: filter.selector, text: filter.text});
+ for (let filter of ElemHideEmulation.getRulesForDomain(hostname))
+ emulatedPatterns.push({selector: filter.selector, text: filter.text});
+ }
}
if (!inline && !updateFrameStyles(sender.page.id, sender.frame.id,
selectors, "standard"))
{
inline = true;
}
@@ -201,8 +254,15 @@
return response;
});
port.on("elemhide.injectSelectors", (message, sender) =>
{
updateFrameStyles(sender.page.id, sender.frame.id, message.selectors,
message.groupName, message.appendOnly);
});
+
+fetch(browser.extension.getURL("/snippets.js"), {cache: "no-cache"})
kzar 2018/07/19 12:24:20 What if fetch throws?
Manish Jethani 2018/07/19 12:55:24 I don't see how it could throw in practice if the
kzar 2018/07/19 13:28:55 Acknowledged.
+.then(response => response.ok ? response.text() : "")
kzar 2018/07/19 12:24:21 Nit: We normally put some indentation before the `
Manish Jethani 2018/07/19 12:55:25 I thought when we break foo.bar.lambda() then (1)
kzar 2018/07/19 13:28:56 Well, with your example it would be more like this
+.then(text =>
kzar 2018/07/19 12:24:20 Why not just do the assignment in the previous fun
Manish Jethani 2018/07/19 12:55:26 response.text() returns a promise, which then reso
kzar 2018/07/19 13:28:54 Ah right, the indirection makes sense then, fair e
+{
+ snippetsLibrarySource = text;
+});
« include.preload.js ('K') | « include.preload.js ('k') | metadata.chrome » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld