Issue 6538, 6781 - Implement support for snippet filters

lib/snippets.js

Index: lib/snippets.js
===================================================================
new file mode 100644
--- /dev/null
+++ b/lib/snippets.js
@@ -0,0 +1,77 @@
+/*
+ * 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/>.
+ */
+
+"use strict";
+
+/**
+ * @fileOverview Snippets implementation.
+ */
+
+const {Filter} = require("./filterClasses");
+
+let filters = new Set();
+
+/**
+ * Container for snippet filters
+ * @class
+ */
+let Snippets = {
+  /**
+   * Removes all known filters
+   */
+  clear()
+  {
+    filters.clear();
+  },
+
+  /**
+   * Add a new snippet filter
+   * @param {SnippetFilter} filter
+   */
+  add(filter)
+  {
+    filters.add(filter.text);
+  },
+
+  /**
+   * Removes a snippet filter
+   * @param {SnippetFilter} filter
+   */
+  remove(filter)
+  {
+    filters.delete(filter.text);
+  },
+
+  /**
+   * Returns a list of all scripts active on a particular domain
+   * @param {string} domain
+   * @return {string[]}
+   */
+  getScriptsForDomain(domain)

[kzar, 2018/07/10 12:53:23]

This seems pretty inefficient. I would have expected this to be called quite
often, and its performance to matter therefore. But no doubt you've thought
about this, so could you explain your approach?

[Manish Jethani, 2018/07/10 18:29:21]

See my comment here:

https://codereview.adblockplus.org/29737558/#msg17

This is copied from lib/elemHideEmulation.js. Eventually we want to merge all
three modules; until then, it shouldn't matter too much (not much worse than
what we have now for element hiding emulation). In the beginning I expect not
more than two or three snippet filters with lots of domains attached to them.

[kzar, 2018/07/10 18:54:00]

Well, you're the expert.

Please could you add that snippet filters will be slow if used commonly to the
the known limitations of the issue description though?

[Manish Jethani, 2018/07/11 12:02:09]

The issue is blocked by #6665 so it won't be closed until #6665 is closed. i.e.
only the initial implementation will be slow, not the final implementation. I
expect we should be able to resolve #6665 over one or two releases.

I also don't mind that we work on #6665 first but that runs counter to our goal
of shipping a basic version of this feature as soon as we can.

[kzar, 2018/07/11 12:17:49]

Well if we plan to do a release which includes #6538 before #6665 is fixed then
#6665 should not be marked as a blocker. #6538 should be closed, added to the
milestone and then verified working before the release if it's to be included.

Please could you add a note to explain that the extension might slow down if
there are lots of snippet filters to the known limitations of the issue
description? You can mention that it will hopefully be resolved with #6665 if
you like.

[Manish Jethani, 2018/07/11 12:45:18]

OK, but we set the milestone on the Platform issue (#6539), is that right? In
that case I'll make the description of that issue clearer, including known
limitations.

Wait, I really want #6539 to remain open as well, I think I'll file a separate
Platform issue for the initial implementation and let #6539 be for the overall
work.
Yes, I'll update the issue description for #6538 as well.

[kzar, 2018/07/11 17:17:51]

That's right.
Thanks, that will help the testers and also properly set expectations.
OK, either way sounds OK to me.

+  {
+    let result = [];
+    for (let text of filters)
+    {
+      let filter = Filter.fromText(text);
+      if (filter.isActiveOnDomain(domain))
+        result.push(filter.script);
+    }
+    return result;
+  }
+};
+
+exports.Snippets = Snippets;