Opened 6 months ago

Closed 6 months ago

Last modified 4 weeks ago

#7285 closed change (fixed)

Abstract caching logic

Reported by: mjethani Assignee: mjethani
Priority: P4 Milestone:
Module: Core Keywords:
Cc: Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/30007559/

Description (last modified by mjethani)

Background

There was a discussion in #7254 about the strategy used for caching the results of filter matching. Even if we make no changes, it would be better to abstract the caching logic into its own module so it is less prone to logical errors. We should also do more caching wherever it makes sense (e.g. checking if a request is third-party).

What to change

Abstract the current caching logic in lib/matcher.js and lib/elemHide.js into its own module.

Hints for testers

Most of this is covered by unit tests, but you could make sure that caches are getting cleared when filters are updated.

For example, use the following document:

<!DOCTYPE html>
<html>
  <body>
    <div id="foo">foo</div>
    <img src="https://imgs.xkcd.com/comics/invisible_formatting.png">
  </body>
</html>

And add two filters:

###foo
invisible_formatting

When you load the document, the element displaying "foo" should be hidden and the image should not appear. Then remove the filters and reload the page: now "foo" should be visible and the image should appear.

Change History (6)

comment:1 Changed 6 months ago by mjethani

  • Review URL(s) modified (diff)

comment:2 Changed 6 months ago by abpbot

A commit referencing this issue has landed:
Issue 7285 - Abstract caching logic

comment:3 Changed 6 months ago by mjethani

  • Owner set to mjethani
  • Priority changed from Unknown to P4
  • Ready set

comment:4 Changed 6 months ago by mjethani

  • Description modified (diff)

comment:5 Changed 6 months ago by mjethani

  • Resolution set to fixed
  • Status changed from new to closed

comment:6 Changed 4 weeks ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. Working as described.

ABP 0.9.15.2340
Microsoft Edge 44.17763.1.0 / Windows 10 1809

ABP 3.5.2.2340
Chrome 49.0.2623.75 / Windows 10 1809
Chrome 75.0.3770.142 / Windows 10 1809
Opera 36.0.2130.65 / Windows 10 1809
Opera 62.0.3331.72 / Windows 10 1809
Firefox 51.0 / Windows 10 1809
Firefox 68.0 / Windows 10 1809
Firefox Mobile 68.0 / Android 7.2.2

Note: See TracTickets for help on using tickets.