Issue #1158 - removed unnecessary definitions and #ifdef/#else/#endif sections

Issue #1158 - removed unnecessary definitions and #ifdef/#else/#endif sections

Removed definition of PRODUCT_ADBLOCKPLUS as a command-line definition for the
compiler and assume that it's always defined instead. When this symbol was
defined, so also were SUPPORT_FILTER, SUPPORT_WHITELIST, SUPPORT_DOM_TRAVERSER,
and SUPPORT_FRAME_CACHING. Thus also assume these four symbols are always
defined.

Removed all #ifdef/#else/#endif directives associated with these symbols.
Removed the contents of the associated sections as appropriate.

Assume SUPPORT_CONFIG is never defined. For this symbol, the only actual code
removed was a single static member variable and its initializer. Note that there
is no source file PluginConfig.h, as mentioned in some of the removed sections.

[Oleksandr, 2014-08-18 20:46:04]

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
File src/plugin/Config.h (left):

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
src/plugin/Config.h:124: #define SUPPORT_DOM_TRAVERSER
I would leave the SUPPORT_DOM_TRAVERSER here

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
File src/plugin/PluginTabBase.cpp (left):

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
src/plugin/PluginTabBase.cpp:55: #ifdef SUPPORT_DOM_TRAVERSER
I would leave this ifdef and others concerning the traverser for future. It is
sometimes useful to test the plugin without ad hiding, also this is good to have
if at some point we decide to actually swtich to CSS injection instead.

[Felix Dahlke, 2014-09-30 09:33:29]

Awesome, LGTM from my side!

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
File src/plugin/PluginTabBase.cpp (left):

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
src/plugin/PluginTabBase.cpp:55: #ifdef SUPPORT_DOM_TRAVERSER
IMHO the traverser code is pretty well isolated from the rest - otherwise I
would agree with keeping the defines around to figure out where the traverser
code is.

For debugging, it should suffice to uncomment the `TraverseDocument` line below,
wouldn't it? Doesn't seem like the (then) pointless UpdateUI() call would hurt.

When it comes to switching to CSS injection - I've got a feeling we will end up
keeping the traverser around for older IE issues or maybe specific filters in
there, so it'll most likely be a runtime decision.

[Oleksandr, 2014-10-02 21:24:02]

Some more stuff to remove

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
File src/plugin/PluginClass.cpp (left):

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
src/plugin/PluginClass.cpp:49: std::map<UINT,CString>
CPluginClass::s_menuDomains;
This line can go as well

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
src/plugin/PluginClass.cpp:1381: s_menuDomains.clear();
The whole critical section lock is dummy here. s_menuDomains isn't used anywhere
now

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
src/plugin/PluginClass.cpp:1640: {
This is clearly a dummy 'if'. Can we remove it?

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
File src/plugin/PluginClass.h (left):

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
src/plugin/PluginClass.h:27: #define WM_WHITELIST_DOMAIN		       
(WM_LAUNCH_INFO + 1)
WM_WHITELIST_DOMAIN isn't used anymore

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
src/plugin/PluginClass.h:174: static std::map<UINT, CString> s_menuDomains;
Looks like we can remove s_menuDomains as well.

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
src/plugin/PluginClass.h:181: static CComAutoCriticalSection
s_criticalSectionWhiteList;
s_criticalSectionWhiteList is unused.

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
File src/plugin/PluginStdAfx.h (left):

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
src/plugin/PluginStdAfx.h:64: #define CONFIG_IN_REGISTRY
We've removed SUPPORT_CONFIG and we are not using the config file anymore, so we
can remove this whole #ifdef as well

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
File src/plugin/PluginTabBase.cpp (left):

http://codereview.adblockplus.org/5948518908821504/diff/5629499534213120/src/...
src/plugin/PluginTabBase.cpp:55: #ifdef SUPPORT_DOM_TRAVERSER
Good point. Agreed.
On 2014/09/30 09:33:29, Felix H. Dahlke wrote:
> IMHO the traverser code is pretty well isolated from the rest - otherwise I
> would agree with keeping the defines around to figure out where the traverser
> code is.
> 
> For debugging, it should suffice to uncomment the `TraverseDocument` line
below,
> wouldn't it? Doesn't seem like the (then) pointless UpdateUI() call would
hurt.
> 
> When it comes to switching to CSS injection - I've got a feeling we will end
up
> keeping the traverser around for older IE issues or maybe specific filters in
> there, so it'll most likely be a runtime decision.

[Eric, 2014-10-02 22:02:53]

I took all the suggestions and put them up in a separate review so that we can
get this one committed swiftly. See
http://codereview.adblockplus.org/5558809044975616/

Oleksandr, I think the only objection you had previously was the question of the
DOM traverser #define, and that seems resolved.

[Oleksandr, 2014-10-03 08:15:36]

Yes, LGTM for this
On 2014/10/02 22:02:53, Eric wrote:
> I took all the suggestions and put them up in a separate review so that we can
> get this one committed swiftly. See
> http://codereview.adblockplus.org/5558809044975616/
> 
> Oleksandr, I think the only objection you had previously was the question of
the
> DOM traverser #define, and that seems resolved.

[sergei, 2014-10-06 09:35:54]

LGTM