Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(790)

Issue 29587914: Issue 5142 - Convert Element Hiding to C++

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 2 weeks ago by hub
Modified:
1 week, 1 day ago
Reviewers:
sergei, Wladimir Palant
CC:
Oleksandr
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

Issue 5142 - Convert Element Hiding to C++

Patch Set 1 #

Total comments: 9

Patch Set 2 : More test, cache the unconditional selectors. Various cleanup #

Total comments: 2

Patch Set 3 : A few more fixes. #

Patch Set 4 : Add proper delete for filters in tests. #

Patch Set 5 : Some style change following feedback from a different review. #

Patch Set 6 : Added OwnedStringMap to address ownership of keys #

Patch Set 7 : Rebased on master. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+504 lines, -400 lines) Patch
A compiled/ElemHide.h View 1 2 3 4 5 1 chunk +103 lines, -0 lines 0 comments Download
A compiled/ElemHide.cpp View 1 2 3 4 5 1 chunk +239 lines, -0 lines 0 comments Download
M compiled/Map.h View 1 2 3 4 5 6 3 chunks +20 lines, -6 lines 0 comments Download
M compiled/String.h View 1 2 3 4 5 2 chunks +6 lines, -1 line 0 comments Download
M compiled/StringMap.h View 1 2 3 4 5 6 2 chunks +33 lines, -12 lines 0 comments Download
M compiled/bindings/main.cpp View 1 2 3 4 5 2 chunks +14 lines, -0 lines 0 comments Download
M compiled/filter/ActiveFilter.h View 1 chunk +3 lines, -2 lines 0 comments Download
M lib/elemHide.js View 1 chunk +2 lines, -376 lines 0 comments Download
M meson.build View 1 chunk +1 line, -0 lines 0 comments Download
M test/elemHideEmulation.js View 1 2 3 4 1 chunk +83 lines, -3 lines 0 comments Download

Messages

Total messages: 6
hub
1 month, 2 weeks ago (2017-10-25 01:07:47 UTC) #1
hub
It is based on some unlanded patches. I have a few more changes to do ...
1 month, 2 weeks ago (2017-10-25 01:19:39 UTC) #2
hub
https://codereview.adblockplus.org/29587914/diff/29587915/compiled/ElemHide.cpp File compiled/ElemHide.cpp (right): https://codereview.adblockplus.org/29587914/diff/29587915/compiled/ElemHide.cpp#newcode172 compiled/ElemHide.cpp:172: _ElemHide_SelectorList* ElemHide::GetUnconditionalSelectors() const there is also a more efficient ...
1 month, 2 weeks ago (2017-10-25 01:39:12 UTC) #3
hub
This looks better. Still depend on unlanded patches.
1 month, 2 weeks ago (2017-10-26 19:21:10 UTC) #4
hub
https://codereview.adblockplus.org/29587914/diff/29589658/compiled/ElemHide.h File compiled/ElemHide.h (right): https://codereview.adblockplus.org/29587914/diff/29589658/compiled/ElemHide.h#newcode43 compiled/ElemHide.h:43: void push_back(ElemHideBasePtr filter) I'm doing this wrong. We are ...
1 month, 2 weeks ago (2017-10-26 20:53:31 UTC) #5
hub
1 month, 2 weeks ago (2017-10-26 20:58:27 UTC) #6
https://codereview.adblockplus.org/29587914/diff/29589658/compiled/ElemHide.h
File compiled/ElemHide.h (right):

https://codereview.adblockplus.org/29587914/diff/29589658/compiled/ElemHide.h...
compiled/ElemHide.h:43: void push_back(ElemHideBasePtr filter)
On 2017/10/26 20:53:30, hub wrote:
> I'm doing this wrong. We are supposed to return selectors. Will fix this.

but then FilterKeyAt() needs more than just the selector so this look like the
simple way to do it.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5