Issue 7029 - Remove subscriptions property of Filter object

lib/subscriptionClasses.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
@@ skipping 49 matching lines @@
    */
   type: null,
 
   /**
    * Filter text contained in the filter subscription.
    * @type {Array.<string>}
    * @private
    */
   _filterText: null,
 
+  /**
+   * A searchable index of filter text in the filter subscription.

[Manish Jethani, 2019/02/26 12:29:57]

We might be able to find a better way to do this than to use a Set object, but
this is not too expensive either.

+   * @type {?Set.<string>}
+   * @private
+   */
+  _filterTextIndex: null,
+
   _title: null,
   _fixedTitle: false,
   _disabled: false,
 
   /**
    * Title of the filter subscription
    * @type {string}
    */
   get title()
   {
@@ skipping 60 matching lines @@
   /**
    * Yields the text for each filter in the subscription.
    * @yields {string}
    */
   *filterText()
   {
     yield* this._filterText;
   },
 
   /**
+   * Checks whether the subscription has the given filter text.
+   * @param {string} filterText
+   * @returns {boolean}
+   * @package
+   */
+  hasFilterText(filterText)
+  {
+    if (!this._filterTextIndex)

[Manish Jethani, 2019/02/26 12:29:57]

We build the index on first call. Unfortunately we never clear it until the list
of filters changes again. Maybe there's a better way, but again this is not too
expensive in terms of memory usage.

+      this._filterTextIndex = new Set(this._filterText);
+
+    return this._filterTextIndex.has(filterText);
+  },
+
+  /**
    * Returns the filter text at the given 0-based index.
    * @param {number} index
    * @returns {?Filter}
    */
   filterTextAt(index)
   {
     return this._filterText[index] || null;
   },
 
   /**
    * Returns the 0-based index of the given filter.
    * @param {Filter} filter
    * @param {number} [fromIndex] The index from which to start the search.
    * @return {number}
    */
-  searchFilter(filter, fromIndex = 0)
+  findFilterIndex(filter, fromIndex = 0)

[Manish Jethani, 2019/02/26 12:29:57]

While we're at it, let's rename this function to something that makes more
sense. `searchFilter` implies that we are searching a filter; actually we're
searching a subscription for the index of a filter. `findFilterIndex` seems more
appropriate.

   {
     return this._filterText.indexOf(filter.text, fromIndex);
   },
 
   /**
    * Removes all filters from the subscription.
    */
   clearFilters()
   {
     this._filterText = [];
+    this._filterTextIndex = null;
   },
 
   /**
    * Adds a filter to the subscription.
    * @param {Filter} filter
    */
   addFilter(filter)
   {
     this._filterText.push(filter.text);
+    this._filterTextIndex = null;

[hub, 2019/03/08 21:36:19]

Shouldn't it be more efficiant that we add the `filter.text` to
`this._filterTextIndex` (and remove in deleteFilterAt) instead of deleting it?
This would mean hasFilterText wouldn't need to create it.

[Manish Jethani, 2019/03/30 20:50:35]

Adding it right away would mean more memory usage and worse performance when the
extension is loaded (slower loading). Instead, we create it when it's needed
(i.e. when the subscriptions are updated).

Even when a subscription is updated, when the filters are being added/removed it
should just set it to `null` which will be faster. Later when we need to find
out in which subscriptions each filter exists, then for each subscription the
index is created only once.

   },
 
   /**
    * Adds a filter to the subscription.
    * @param {string} filterText
    */
   addFilterText(filterText)
   {
     this._filterText.push(filterText);
+    this._filterTextIndex = null;
   },
 
   /**
    * Inserts a filter into the subscription.
    * @param {Filter} filter
    * @param {number} index The index at which to insert the filter.
    */
   insertFilterAt(filter, index)
   {
     this._filterText.splice(index, 0, filter.text);
+    this._filterTextIndex = null;
   },
 
   /**
    * Deletes a filter from the subscription.
    * @param {number} index The index at which to delete the filter.
    */
   deleteFilterAt(index)
   {
     // Ignore index if out of bounds on the negative side, for consistency.
     if (index < 0)
       return;
 
     this._filterText.splice(index, 1);
+    this._filterTextIndex = null;
   },
 
   /**
    * Serializes the subscription for writing out on disk.
    * @yields {string}
    */
   *serialize()
   {
     let {url, type, _title, _fixedTitle, _disabled} = this;
 
@@ skipping 447 matching lines @@
     if (errors)
       yield "errors=" + errors;
     if (version)
       yield "version=" + version;
     if (requiredVersion)
       yield "requiredVersion=" + requiredVersion;
     if (downloadCount)
       yield "downloadCount=" + downloadCount;
   }
 });