Issue 7097 - Make element hiding filter objects ephemeral

lib/filterClasses.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 176 matching lines @@
           this._subscriptions = [...this._subscriptions][0];
       }
       else if (subscription == this._subscriptions)
       {
         this._subscriptions = null;
       }
     }
   },
 
   /**
-   * Checks whether the filter should be serialized.
-   * @return {boolean}
-   * @protected
-   */
-  shouldSerialize()
-  {
-    return false;
-  },
-
-  /**
    * Serializes the filter for writing out on disk.
    * @yields {string}
    */
   *serialize()
   {
     let {text} = this;
 
     yield "[Filter]";
     yield "text=" + text;
   },
@@ skipping 24 matching lines @@
  * Regular expression that matches an invalid Content Security Policy
  * @type {RegExp}
  */
 Filter.invalidCSPRegExp = /(;|^) ?(base-uri|referrer|report-to|report-uri|upgrade-insecure-requests)\b/i;
 
 /**
  * Creates a filter of correct type from its text representation - does the
  * basic parsing and calls the right constructor then.
  *
  * @param {string} text   as in Filter()
+ * @param {boolean} persistent Whether the filter object should be persisted in
+ *   memory.
  * @return {Filter}
  */
-Filter.fromText = function(text)
+Filter.fromText = function(text, persistent = true)

[Manish Jethani, 2018/11/18 06:19:14]

The idea here is that internally we'll always pass false, but UI for example
(for user-defined filters) will not pass anything so it'll default to true.

Rules for the ephemeral property:

 -  If persistent is true, ephemeral is false (otherwise it's the default value)
 -  The value cannot change once the filter object has been returned from here
 -  To persist state like disabled, hit count, etc., create a non-ephemeral
object first and then set the state property, otherwise it'll be lost
 -  Do not hold on to references to ephemeral objects because their state will
sooner or later be out of sync with reality (e.g. removed subscriptions will
still appear)

 {
   let filter = Filter.knownFilters.get(text);
   if (filter)
     return filter;
 
   if (text[0] == "!")
   {
     filter = new CommentFilter(text);
   }
   else
   {
     let match = text.includes("#") ? Filter.contentRegExp.exec(text) : null;
     if (match)
       filter = ContentFilter.fromText(text, match[1], match[2], match[3]);
     else
       filter = RegExpFilter.fromText(text);
   }
+
+  if (persistent)
+    filter.ephemeral = false;
 
   if (!filter.ephemeral)
     Filter.knownFilters.set(filter.text, filter);
 
   return filter;
 };
 
 /**
  * Deserializes a filter
  *
  * @param {Object}  obj map of serialized properties and their values
  * @return {Filter} filter or null if the filter couldn't be created
  */
 Filter.fromObject = function(obj)
 {
   let filter = Filter.fromText(obj.text);
   if (filter instanceof ActiveFilter)
   {
     if ("disabled" in obj)
       filter._disabled = (obj.disabled == "true");
     if ("hitCount" in obj)
       filter._hitCount = parseInt(obj.hitCount, 10) || 0;
     if ("lastHit" in obj)
       filter._lastHit = parseInt(obj.lastHit, 10) || 0;
-
-    // If the filter object is ephemeral, we normally don't keep it, but if it
-    // has any state then we must.
-    if (filter.ephemeral && this.shouldSerialize())
-      Filter.knownFilters.set(filter.text, filter);
   }
   return filter;
 };
 
 /**
  * Removes unnecessary whitespaces from filter text, will only return null if
  * the input parameter is null.
  * @param {string} text
  * @return {string}
  */
@@ skipping 142 matching lines @@
   get disabled()
   {
     return this._disabled;
   },
   set disabled(value)
   {
     if (value != this._disabled)
     {
       let oldValue = this._disabled;
       this._disabled = value;
-
-      if (value && this.ephemeral)

[Manish Jethani, 2018/11/18 03:29:12]

This could be optimized a little bit, but this property is never set in the
current extension. We are keeping it because it might get used in a future
version (see discussion in #7095).

[Manish Jethani, 2018/11/18 06:19:14]

Ignore, now removed.

-        Filter.knownFilters.set(this.text, this);
-
       filterNotifier.emit("filter.disabled", this, value, oldValue);
     }
     return this._disabled;
   },
 
   /**
    * Number of hits on the filter since the last reset
    * @type {number}
    */
   get hitCount()
   {
     return this._hitCount;
   },
   set hitCount(value)
   {
     if (value != this._hitCount)
     {
       let oldValue = this._hitCount;
       this._hitCount = value;
-
-      if (value && this.ephemeral)
-        Filter.knownFilters.set(this.text, this);
-
       filterNotifier.emit("filter.hitCount", this, value, oldValue);
     }
     return this._hitCount;
   },
 
   /**
    * Last time the filter had a hit (in milliseconds since the beginning of the
    * epoch)
    * @type {number}
    */
   get lastHit()
   {
     return this._lastHit;
   },
   set lastHit(value)
   {
     if (value != this._lastHit)
     {
       let oldValue = this._lastHit;
       this._lastHit = value;
-
-      if (value && this.ephemeral)
-        Filter.knownFilters.set(this.text, this);
-
       filterNotifier.emit("filter.lastHit", this, value, oldValue);
     }
     return this._lastHit;
   },
 
   /**
    * String that the domains property should be generated from
    * @type {?string}
    */
   domainSource: null,
@@ skipping 180 matching lines @@
    * @return {boolean}
    */
   isGeneric()
   {
     let {sitekeys, domains} = this;
 
     return !(sitekeys && sitekeys.length) && (!domains || domains.get(""));
   },
 
   /**
-   * See Filter.shouldSerialize()
-   * @inheritdoc
-   */
-  shouldSerialize()

[Manish Jethani, 2018/11/18 03:29:12]

Note that this will always return false in the current extension.

[Manish Jethani, 2018/11/18 06:19:14]

Ignore, now removed.

An object created with Filter.fromObject is never ephemeral.

-  {
-    return this._disabled || this._hitCount || this._lastHit;
-  },
-
-  /**
    * See Filter.serialize()
    * @inheritdoc
    */
   *serialize()
   {
-    if (this.shouldSerialize())
-    {
-      let {_disabled, _hitCount, _lastHit} = this;
+    let {_disabled, _hitCount, _lastHit} = this;
+
+    if (_disabled || _hitCount || _lastHit)
+    {
       yield* Filter.prototype.serialize.call(this);
       if (_disabled)
         yield "disabled=true";
       if (_hitCount)
         yield "hitCount=" + _hitCount;
       if (_lastHit)
         yield "lastHit=" + _lastHit;
     }
   }
 });
@@ skipping 687 matching lines @@
 
   /**
    * Script that should be executed
    * @type {string}
    */
   get script()
   {
     return this.body;
   }
 });