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

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

Created:
Dec. 10, 2014, 5:12 p.m. by sergei
Modified:
March 17, 2017, 4:22 p.m.
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 ...
Dec. 10, 2014, 5:16 p.m. (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 ...
Feb. 4, 2015, 8:59 p.m. (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 ...
April 13, 2015, 8:06 a.m. (2015-04-13 08:06:58 UTC) #3
sergei
Dec. 1, 2015, 11:33 a.m. (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 ...
Dec. 1, 2015, 2:39 p.m. (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 ...
Dec. 1, 2015, 3:35 p.m. (2015-12-01 15:35:48 UTC) #6
Eric
This change, as written, will make all of our initialization and lifetime problems worse. We ...
Dec. 2, 2015, 5 p.m. (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, ...
Dec. 2, 2015, 5:02 p.m. (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 ...
Dec. 2, 2015, 6:45 p.m. (2015-12-02 18:45:19 UTC) #9
Eric
NOT LGTM Since I have upbraided Sergei for remaining silent, it behooves me to say ...
Jan. 27, 2016, 3:09 p.m. (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 ...
Feb. 25, 2016, 5:48 p.m. (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 ...
Feb. 26, 2016, 8:34 a.m. (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: > ...
Feb. 26, 2016, 4:57 p.m. (2016-02-26 16:57:57 UTC) #13
Oleksandr
Sept. 30, 2016, 3:29 p.m. (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?

Powered by Google App Engine
This is Rietveld