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

Issue 6567422169448448: Issue 119 - Switch to injecting CSS for element hiding (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 2 months ago by sergei
Modified:
11 months, 1 week ago
Visibility:
Public.

Description

# Currently there are a lot of unrelated changes, they should be in other codereviews and some of them are already in another codereview but there is no progress. - add WebBrowserEventsListener.{h,cpp} which detects when the actual frame loading/reloading happened. - add CPluginClass::m_data. It has to be a shared pointer because we capture a weak ptr on it to avoid usage of it after destroying of CPluginClass. - add CPluginFilter::m_hideFilters with getter (it should wait for the finishing of the initialization) so we can use them to inject as CSS - change CPluginFilter::LoadHideFilters to accept const reference instead of a copy - add method InjectABPCSS which injects our css - call InjectABPCSS from CPluginTabBase::OnDocumentComplete which is called when the real page loading happened - add methods to decide whether we need to traverse or to inject css (based on testing versions) - do nothing in ~CPluginClass because we should do such stuff in FinalRelease

Patch Set 1 #

Total comments: 13

Patch Set 2 : rebase and address comments #

Patch Set 3 : fix injecting of CSS when plugin is enabled/disabled and rearrange variables in InjectABPCSS #

Patch Set 4 : rebase #

Patch Set 5 : rebase #

Patch Set 6 : rebase #

Total comments: 20

Patch Set 7 : rebase #

Patch Set 8 : rebase, improve memory handling (OnQuit) and improve code organizing #

Total comments: 1

Patch Set 9 : rename OnQuit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+747 lines, -162 lines) Patch
M adblockplus.gyp View 1 2 3 4 5 6 7 3 chunks +6 lines, -0 lines 0 comments Download
M dependencies View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M src/engine/NotificationWindow.cpp View 1 2 3 4 5 6 7 1 chunk +2 lines, -2 lines 0 comments Download
M src/plugin/AdblockPlusClient.h View 1 2 3 4 5 6 7 1 chunk +0 lines, -1 line 0 comments Download
M src/plugin/AdblockPlusClient.cpp View 1 2 3 4 5 6 7 1 chunk +0 lines, -11 lines 0 comments Download
M src/plugin/AdblockPlusDomTraverser.h View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M src/plugin/AdblockPlusDomTraverser.cpp View 1 2 3 4 5 6 7 3 chunks +3 lines, -6 lines 0 comments Download
M src/plugin/PluginClass.h View 1 2 3 4 5 6 5 chunks +11 lines, -13 lines 0 comments Download
M src/plugin/PluginClass.cpp View 1 2 3 4 5 6 7 20 chunks +66 lines, -47 lines 0 comments Download
M src/plugin/PluginDomTraverserBase.h View 1 2 3 4 5 6 7 4 chunks +4 lines, -9 lines 0 comments Download
M src/plugin/PluginFilter.h View 1 2 3 4 5 6 7 2 chunks +6 lines, -6 lines 0 comments Download
M src/plugin/PluginFilter.cpp View 1 2 3 4 5 6 7 3 chunks +4 lines, -23 lines 0 comments Download
M src/plugin/PluginTabBase.h View 1 2 3 4 5 6 7 1 chunk +5 lines, -2 lines 0 comments Download
M src/plugin/PluginTabBase.cpp View 1 2 3 4 5 6 7 6 chunks +212 lines, -40 lines 0 comments Download
A src/plugin/WebBrowserEventsListener.h View 1 2 3 4 5 6 7 8 1 chunk +75 lines, -0 lines 0 comments Download
A src/plugin/WebBrowserEventsListener.cpp View 1 2 3 4 5 6 7 8 1 chunk +164 lines, -0 lines 0 comments Download
A src/shared/EventWithSetter.h View 1 2 3 4 5 6 7 1 chunk +100 lines, -0 lines 0 comments Download
A src/shared/EventWithSetter.cpp View 1 2 3 4 5 6 7 1 chunk +87 lines, -0 lines 0 comments Download

Messages

Total messages: 14
sergei
Please check the code, actually I think we should merge it gradually thus split into ...
3 years, 2 months ago (2014-12-10 17:16:15 UTC) #1
Oleksandr
http://codereview.adblockplus.org/6567422169448448/diff/5629499534213120/src/plugin/PluginClass.cpp File src/plugin/PluginClass.cpp (right): http://codereview.adblockplus.org/6567422169448448/diff/5629499534213120/src/plugin/PluginClass.cpp#newcode498 src/plugin/PluginClass.cpp:498: { I would prefer for this to be a ...
3 years ago (2015-02-04 20:59:57 UTC) #2
sergei
http://codereview.adblockplus.org/6567422169448448/diff/5629499534213120/src/plugin/PluginClass.cpp File src/plugin/PluginClass.cpp (right): http://codereview.adblockplus.org/6567422169448448/diff/5629499534213120/src/plugin/PluginClass.cpp#newcode498 src/plugin/PluginClass.cpp:498: { On 2015/02/04 20:59:57, Oleksandr wrote: > I would ...
2 years, 10 months ago (2015-04-13 08:06:58 UTC) #3
sergei
2 years, 2 months ago (2015-12-01 11:33:05 UTC) #4
Eric
Have all the prerequisites for this change set been committed? There's a new rebased patch ...
2 years, 2 months ago (2015-12-01 14:39:46 UTC) #5
sergei
On 2015/12/01 14:39:46, Eric wrote: > Have all the prerequisites for this change set been ...
2 years, 2 months ago (2015-12-01 15:35:48 UTC) #6
Eric
This change, as written, will make all of our initialization and lifetime problems worse. We ...
2 years, 2 months ago (2015-12-02 17:00:15 UTC) #7
Eric
https://codereview.adblockplus.org/6567422169448448/diff/29331703/src/plugin/PluginClass.h File src/plugin/PluginClass.h (left): https://codereview.adblockplus.org/6567422169448448/diff/29331703/src/plugin/PluginClass.h#oldcode57 src/plugin/PluginClass.h:57: CPluginTab* m_tab; On 2015/12/02 17:00:14, Eric wrote: > ordinary, ...
2 years, 2 months ago (2015-12-02 17:02:02 UTC) #8
Eric
https://codereview.adblockplus.org/6567422169448448/diff/29331703/src/plugin/PluginClass.h File src/plugin/PluginClass.h (left): https://codereview.adblockplus.org/6567422169448448/diff/29331703/src/plugin/PluginClass.h#oldcode57 src/plugin/PluginClass.h:57: CPluginTab* m_tab; While auditing code, I found the same ...
2 years, 2 months ago (2015-12-02 18:45:19 UTC) #9
Eric
NOT LGTM Since I have upbraided Sergei for remaining silent, it behooves me to say ...
2 years ago (2016-01-27 15:09:26 UTC) #10
sergei
Pay attention that firstly a bug mentioned in the description should be fixed. https://codereview.adblockplus.org/6567422169448448/diff/29331703/adblockplus.gyp File ...
1 year, 11 months ago (2016-02-25 17:48:29 UTC) #11
sergei
https://codereview.adblockplus.org/6567422169448448/diff/29331703/src/plugin/PluginClass.h File src/plugin/PluginClass.h (right): https://codereview.adblockplus.org/6567422169448448/diff/29331703/src/plugin/PluginClass.h#newcode155 src/plugin/PluginClass.h:155: std::map<IWebBrowser2*, WebBrowserEventsListener*> connectedWebBrowsersCache; BTW, in addition we probably need ...
1 year, 11 months ago (2016-02-26 08:34:52 UTC) #12
sergei
https://codereview.adblockplus.org/6567422169448448/diff/29331703/src/plugin/PluginClass.h File src/plugin/PluginClass.h (right): https://codereview.adblockplus.org/6567422169448448/diff/29331703/src/plugin/PluginClass.h#newcode155 src/plugin/PluginClass.h:155: std::map<IWebBrowser2*, WebBrowserEventsListener*> connectedWebBrowsersCache; On 2016/02/26 08:34:51, sergei wrote: > ...
1 year, 11 months ago (2016-02-26 16:57:57 UTC) #13
Oleksandr
1 year, 4 months ago (2016-09-30 15:29:21 UTC) #14
LGTM

https://codereview.adblockplus.org/6567422169448448/diff/29340865/src/plugin/...
File src/plugin/WebBrowserEventsListener.cpp (right):

https://codereview.adblockplus.org/6567422169448448/diff/29340865/src/plugin/...
src/plugin/WebBrowserEventsListener.cpp:62: void STDMETHODCALLTYPE
WebBrowserEventsListener::OnOnQuit()
OnOnQuit sounds somewhat meta. How about just OnQuit?
Sign in to reply to this message.

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