Issue #1484 - Make PassthroughAPP a pass-through again

Issue #1484 - Make PassthroughAPP a pass-through again

Create new class 'WbPassthroughSinkStartPolicy', deriving from the template
instantiation of 'CustomSinkStartPolicy' that was previously used. Shadow its
'OnStart' member function with one that replicates previous behavior.

Extract code that was previously added to 'CustomSinkStartPolicy::OnStart()'
into 'WbPassthroughSinkStartPolicy::OnStart'. Restore 'const' declaration to 'CustomSinkStartPolicy::OnStart'.

Rename 'WBPassthru' to 'WbPassthroughPolicy' to reflect the template argument
name 'Policy' in PassthroughAPP that it instantiates.

[Eric, 2015-11-30 13:59:49]

The second of two reviews to fix #1484. This one is rather more subtle than the
other.

I've tested it only insofar as to ensure that it runs without any obvious
problems. I'm not familiar with test cases for the problems that these changes
were originally made for.

[Eric, 2015-12-15 14:16:29]

Ping.

It's been more than two weeks since I posted this review and it's received no
attention. There was a dependent review, but it was closed two weeks ago. Is
there any particular reason for the delay?

[Oleksandr, 2015-12-16 13:33:31]

Test cases for these changes would be to check if you ever experience the
"Navigation cancelled" page in an iframe that we've blocked. Or the "Image not
available" icon when an image was blocked. I usually disable the traverser to
test this, since iframe or an image can be hidden after it was blocked. Overall
the change LGTM, however I have only tried it on my machine for now.

https://codereview.adblockplus.org/29331590/diff/29331602/src/plugin/PluginWb...
File src/plugin/PluginWbPassThrough.cpp (right):

https://codereview.adblockplus.org/29331590/diff/29331602/src/plugin/PluginWb...
src/plugin/PluginWbPassThrough.cpp:427: if (hr == E_ABORT &&
pSink->m_isCustomResponse)
I am actually not even sure we need this clause at all. m_isCustomResponse is
set in BeginningTransaction, which is called after OnStart. Not a good idea to
remove this in a purely refactoring commit of course, but just a note for
future.

[Eric, 2015-12-22 12:54:48]

@sergei. It's now been three weeks with no response from you at all on this. I
sent out a reminder last week.

Is there a reason you haven't responded yet?

[sergei, 2015-12-22 14:04:40]

LGTM

https://codereview.adblockplus.org/29331590/diff/29331602/src/plugin/PluginWb...
File src/plugin/PluginWbPassThrough.h (right):

https://codereview.adblockplus.org/29331590/diff/29331602/src/plugin/PluginWb...
src/plugin/PluginWbPassThrough.h:92: typedef
PassthroughAPP::CustomSinkStartPolicy<WbPassthroughProtocol, WBPassthruSink>
BaseClass;
I would use __super instead of BaseClass because it's simpler but the it's also
fine.

https://codereview.adblockplus.org/29331590/diff/29331602/src/plugin/PluginWb...
src/plugin/PluginWbPassThrough.h:95: IInternetProtocolSink *pOIProtSink,
IInternetBindInfo *pOIBindInfo,
Just noticed that we are using '*' and ' ' slightly differently, compare
`IInternetProtocolSink* pOIProtSink`
`IInternetProtocolSink *pOIProtSink` // current code.
For example the last argument is written in a different style
`IInternetProtocol* pTargetProtocol`.

[Eric, 2015-12-22 14:34:31]

https://codereview.adblockplus.org/29331590/diff/29331602/src/plugin/PluginWb...
File src/plugin/PluginWbPassThrough.h (right):

https://codereview.adblockplus.org/29331590/diff/29331602/src/plugin/PluginWb...
src/plugin/PluginWbPassThrough.h:95: IInternetProtocolSink *pOIProtSink,
IInternetBindInfo *pOIBindInfo,
On 2015/12/22 14:04:40, sergei wrote:
> Just noticed that we are using '*' and ' ' slightly differently

I changed them all. Patch set 2 is rebase only. Patch set three makes all the
'*' types consistent.

[sergei, 2015-12-22 14:39:37]

LGTM

https://codereview.adblockplus.org/29331590/diff/29331602/src/plugin/PluginWb...
File src/plugin/PluginWbPassThrough.h (right):

https://codereview.adblockplus.org/29331590/diff/29331602/src/plugin/PluginWb...
src/plugin/PluginWbPassThrough.h:95: IInternetProtocolSink *pOIProtSink,
IInternetBindInfo *pOIBindInfo,
On 2015/12/22 14:34:31, Eric wrote:
> On 2015/12/22 14:04:40, sergei wrote:
> > Just noticed that we are using '*' and ' ' slightly differently
> 
> I changed them all. Patch set 2 is rebase only. Patch set three makes all the
> '*' types consistent.

It's also the same in cpp file, but for me it's not critical, especially it
might be better to be in another no-issue commit.