Issue 6843 - Log snippet filter hits

lib/contentFiltering.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * 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 contentFiltering */
 
 "use strict";
 
 const {RegExpFilter} = require("../adblockpluscore/lib/filterClasses");
 const {ElemHide} = require("../adblockpluscore/lib/elemHide");
 const {ElemHideEmulation} = require("../adblockpluscore/lib/elemHideEmulation");
+const {FilterNotifier} = require("../adblockpluscore/lib/filterNotifier");
 const {Snippets, compileScript} = require("../adblockpluscore/lib/snippets");
 const {checkWhitelisted} = require("./whitelisting");
 const {extractHostFromFrame} = require("./url");
 const {port} = require("./messaging");
-const {HitLogger} = require("./hitLogger");
+const {HitLogger, logRequest} = 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;
 
@@ skipping 129 matching lines @@
   let code = executableCode.get(script);
   if (code)
     return code;
 
   code = compileScript(script, [snippetsLibrarySource]);
 
   executableCode.set(script, code);
   return code;
 }
 
-function executeScript(script, tabId, frameId)
+function executeScript(script, tabId, frameId, request)
 {
   try
   {
     let details = {
       code: getExecutableCode(script),
       matchAboutBlank: true,
       runAt: "document_start"
     };
 
     // Chrome <50 throws an exception if chrome.tabs.executeScript is called
     // with a frameId of 0.
     if (frameId != 0)
       details.frameId = frameId;
 
     browser.tabs.executeScript(tabId, details)
+    .then(() =>

[Manish Jethani, 2018/08/27 05:47:36]

Ideally this would take the form `.then(() => {}, error => {})` rather than
`.then(() => {}).catch(error => {})` since we're only specifically interested in
the errors coming from the call itself rather than any subsequent handlers.

[hub, 2018/08/27 12:56:47]

this is obsolete now.

+    {
+      let {filter} = request;

[Manish Jethani, 2018/08/27 05:47:36]

We could read out `request.url` and `request.docDomain` right here as well.

[hub, 2018/08/27 12:56:47]

this is obsolete now.

+
+      let tabIds = [tabId];
+      if (filter)
+        FilterNotifier.emit("filter.hitCount", filter, 0, 0, tabIds);
+
+      logRequest(tabIds, {
+        url: request.url, type: "SNIPPET", docDomain: request.docDomain
+      }, filter);
+    })
     .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;
 
       // Sometimes the frame in question is just not found. We don't know why
       // this is exactly, but we simply ignore the error.
       if (/^No frame with id \d+ in tab \d+\.$/.test(error.message))
         return;
 
       throw error;
     });
   }
   catch (error)
   {
     // See the comment in the catch block associated with the call to
     // tabs.insertCSS for why we catch and ignore any errors here.
   }

[Manish Jethani, 2018/08/27 05:47:36]

I think it would be cleaner to keep executeScript limited to its original
purpose, which is to call tabs.executeScript with some boilerplate. In that
case, we can return the promise returned by tabs.executeScript and let the
caller add a `then` handler to log the hit (perhaps in a separate
`logSnippetFilterHit` function). The catch block here would also then have to
return a rejected promise (`return Promise.reject(error)`).

[hub, 2018/08/27 12:56:47]

Done.

 }
 
 port.on("content.applyFilters", (message, sender) =>
 {
   let selectors = [];
   let emulatedPatterns = [];
   let trace = HitLogger.hasListener(sender.page.id);
   let inline = !userStyleSheetsSupported;
 
   let {elemhide, snippets} = message.filterTypes ||
                              {elemhide: true, snippets: true};
 
   if (!checkWhitelisted(sender.page, sender.frame, null,
                         RegExpFilter.typeMap.DOCUMENT))
   {
     let hostname = extractHostFromFrame(sender.frame);
 
     if (snippets)
     {
-      for (let script of Snippets.getScriptsForDomain(hostname))
-        executeScript(script, sender.page.id, sender.frame.id);
+      for (let filter of Snippets.getScriptsForDomain(hostname))
+        executeScript(filter.script, sender.page.id, sender.frame.id,
+          {
+            docDomain: hostname,

[Sebastian Noack, 2018/08/27 04:51:35]

Nit: If you rename the variable "hostname" to "docDomain", you can use the
object creation shorthand here, and consequently this code also wraps nicer:

  executeScript(filter.script, sender.page.id, sender.frame.id,
                {docDomain, url: sender.frame.url.href, filter});

[hub, 2018/08/27 12:56:47]

Done.

+            url: sender.frame.url.href,
+            filter
+          });
     }
 
     if (elemhide && !checkWhitelisted(sender.page, sender.frame, null,
                                       RegExpFilter.typeMap.ELEMHIDE))
     {
       let specificOnly = checkWhitelisted(sender.page, sender.frame, null,
                                           RegExpFilter.typeMap.GENERICHIDE);
       selectors = ElemHide.getSelectorsForDomain(hostname, specificOnly);
 
       for (let filter of ElemHideEmulation.getRulesForDomain(hostname))
@@ skipping 26 matching lines @@
   updateFrameStyles(sender.page.id, sender.frame.id, message.selectors,
                     message.groupName, message.appendOnly);
 });
 
 fetch(browser.extension.getURL("/snippets.js"), {cache: "no-cache"})
 .then(response => response.ok ? response.text() : "")
 .then(text =>
 {
   snippetsLibrarySource = text;
 });