Issue 6843 - Log snippet filter hits

lib/contentFiltering.js

Index: lib/contentFiltering.js
===================================================================
--- a/lib/contentFiltering.js
+++ b/lib/contentFiltering.js
@@ -17,21 +17,22 @@
 
 /** @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";
 
@@ -171,32 +172,44 @@
     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
@@ -226,18 +239,23 @@
 
   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);