Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29894611: Issue 7001 - Use instance property maxCacheEntries in CombinedMatcher (Closed)

Created:
Sept. 28, 2018, 11:08 p.m. by Manish Jethani
Modified:
Sept. 29, 2018, 12:41 a.m.
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

P1 regression from https://hg.adblockplus.org/adblockpluscore/rev/84ea8a5e3951

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M lib/matcher.js View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 4
Manish Jethani
Sept. 28, 2018, 11:08 p.m. (2018-09-28 23:08:57 UTC) #1
Manish Jethani
Patch Set 1 This is a P1 regression from https://hg.adblockplus.org/adblockpluscore/rev/84ea8a5e3951 We also need to update ...
Sept. 28, 2018, 11:10 p.m. (2018-09-28 23:10:32 UTC) #2
Sebastian Noack
LGTM. Since this seems to be a regression since the last release, I suppose there ...
Sept. 28, 2018, 11:47 p.m. (2018-09-28 23:47:58 UTC) #3
Manish Jethani
Sept. 29, 2018, 12:41 a.m. (2018-09-29 00:41:37 UTC) #4
On 2018/09/28 23:47:58, Sebastian Noack wrote:
> LGTM.

Thanks.

> Since this seems to be a regression since the last release, I suppose there is
> another dependency update coming up? If so, mind also including the fix for
the
> hide-if-contains-and-matches-style snippet on Firefox? It's not clear to me if
> otherwaise this would expose a regression on Firefox to have that snippet
> included in it's previous form (which we only released to Chromium-based
browser
> so far), but either way for snippet changes we relax the code-freeze anyway.

Even if the Firefox port were not included, it would still fail gracefully on
Firefox. Nevertheless, it's a good idea to include it now.

Powered by Google App Engine
This is Rietveld