[superseded] Issue 1265 - Improve detection of mime type of the expected response

This code review is superseded by http://codereview.adblockplus.org/6299667012780032/ .
It's not removed so far to don't loose comments.

Sometimes the mime type is not available in `WBPassthruSink::OnStart` and it's not possible to derive any information from the url. For such cases one can obtain `Accept` header in `BeginningTransaction` and make a decision here, if it's blocked we should return E_ABORT. It's possible via querying for `IWinInetHttpInfo` from the target protocol and calling `QueryInfo(HTTP_QUERY_RAW_HEADERS_CRLF | HTTP_QUERY_FLAG_REQUEST_HEADERS...)`. The latter call fails when it's done from our `OnStart` method, it seems that original target `protocol::Start` prepares the request and headers and then calls `BeginningTransaction`.
To do it we need to store the url of the issuer of the request (`WBPassthruSink::m_boundDomain`) because it's needed by `CPluginFilter::ShouldBlock` in `BeginningTransaction`. As well as make `WBPassthruSink::m_contentType` which is used in `BeginningTransaction`, when the value is `CFilter::EContentType::contentTypeAny` it means that we could not derive the "type of request" in `OnStart` and we should try it here.

Clean some code and a make couple of fixes:
- If cannot derive mime type from URL then use `CFilter::EContentType::contentTypeAny` without subtracting `CFilter::EContentType::contentTypeSubdocument`. It's indeed in many cases `subdocument`.
- add `CPluginDebug::DebugResultIgnoring(type, src, domain);` into `CPluginFilter::ShouldBlock`. There was already a call `CPluginDebug::DebugResultBlocking`, so this call is for consistency. On another hand that pair of calls is removed from `WBPassthruSink`, because it only duplicates blocked entries in the log.

[Oleksandr, 2014-09-11 08:58:54]

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

http://codereview.adblockplus.org/6390087684194304/diff/5629499534213120/src/...
src/plugin/PluginWbPassThrough.cpp:225: else type = "OTHER";
We will still need a way to convert contentType to text representation for
debugging purposes.

http://codereview.adblockplus.org/6390087684194304/diff/5629499534213120/src/...
File src/plugin/PluginWbPassThrough.cpp (right):

http://codereview.adblockplus.org/6390087684194304/diff/5629499534213120/src/...
src/plugin/PluginWbPassThrough.cpp:190: && CFilter::EContentType::contentTypeAny
!= m_contentType
This line is not clear to me. We can in theory block requests that have
contentTypeAny as well.

http://codereview.adblockplus.org/6390087684194304/diff/5629499534213120/src/...
src/plugin/PluginWbPassThrough.cpp:194: DEBUG_BLOCKER("Blocker::Blocking
Http-request:" + src);
DEBUG_BLOCKER is broken here. Either use CPluginDebug::DebugResultBlocking
directly (prefered), or fix the DEBUG_BLOCKER. The debug build should write the
output to debug_result.txt

http://codereview.adblockplus.org/6390087684194304/diff/5629499534213120/src/...
src/plugin/PluginWbPassThrough.cpp:198: DEBUG_BLOCKER("Blocker::Ignoring
Http-request:" + src)
CPluginDebug::DebugResultIgnoring here accordingly

http://codereview.adblockplus.org/6390087684194304/diff/5629499534213120/src/...
src/plugin/PluginWbPassThrough.cpp:313: auto acceptHeader = [&]() -> std::string
I'd just like to say this looks too cool :D

http://codereview.adblockplus.org/6390087684194304/diff/5629499534213120/src/...
src/plugin/PluginWbPassThrough.cpp:352:
//client->GetHostFromUrl(m_boundDomain.operator LPCWSTR()).c_str(), /*debug flag
but must be set*/true);
Leftover code here?

http://codereview.adblockplus.org/6390087684194304/diff/5629499534213120/src/...
src/plugin/PluginWbPassThrough.cpp:355: {
Log blocking here as well

[sergei, 2014-09-11 09:53:25]

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

http://codereview.adblockplus.org/6390087684194304/diff/5629499534213120/src/...
src/plugin/PluginWbPassThrough.cpp:225: else type = "OTHER";
On 2014/09/11 08:58:54, Oleksandr wrote:
> We will still need a way to convert contentType to text representation for
> debugging purposes.
It's done in `CPluginFilter`

http://codereview.adblockplus.org/6390087684194304/diff/5629499534213120/src/...
File src/plugin/PluginWbPassThrough.cpp (right):

http://codereview.adblockplus.org/6390087684194304/diff/5629499534213120/src/...
src/plugin/PluginWbPassThrough.cpp:190: && CFilter::EContentType::contentTypeAny
!= m_contentType
On 2014/09/11 08:58:54, Oleksandr wrote:
> This line is not clear to me. We can in theory block requests that have
> contentTypeAny as well.

If the type is `contentTypeAny` then the decision is made in
`BeginningTransaction`, it will be blocked there.

http://codereview.adblockplus.org/6390087684194304/diff/5629499534213120/src/...
src/plugin/PluginWbPassThrough.cpp:194: DEBUG_BLOCKER("Blocker::Blocking
Http-request:" + src);
On 2014/09/11 08:58:54, Oleksandr wrote:
> DEBUG_BLOCKER is broken here. Either use CPluginDebug::DebugResultBlocking
> directly (prefered), or fix the DEBUG_BLOCKER. The debug build should write
the
> output to debug_result.txt

`client->ShouldBlock` already puts into the log. `DEBUG_BLOCKER` is removed.

http://codereview.adblockplus.org/6390087684194304/diff/5629499534213120/src/...
src/plugin/PluginWbPassThrough.cpp:198: DEBUG_BLOCKER("Blocker::Ignoring
Http-request:" + src)
On 2014/09/11 08:58:54, Oleksandr wrote:
> CPluginDebug::DebugResultIgnoring here accordingly

The same as above, deleted.

http://codereview.adblockplus.org/6390087684194304/diff/5629499534213120/src/...
src/plugin/PluginWbPassThrough.cpp:313: auto acceptHeader = [&]() -> std::string
On 2014/09/11 08:58:54, Oleksandr wrote:
> I'd just like to say this looks too cool :D

Thanks!

http://codereview.adblockplus.org/6390087684194304/diff/5629499534213120/src/...
src/plugin/PluginWbPassThrough.cpp:352:
//client->GetHostFromUrl(m_boundDomain.operator LPCWSTR()).c_str(), /*debug flag
but must be set*/true);
On 2014/09/11 08:58:54, Oleksandr wrote:
> Leftover code here?

Thanks, removed.

http://codereview.adblockplus.org/6390087684194304/diff/5629499534213120/src/...
src/plugin/PluginWbPassThrough.cpp:355: {
On 2014/09/11 08:58:54, Oleksandr wrote:
> Log blocking here as well
`client->ShouldBlock` puts into log

[Oleksandr, 2014-10-01 09:15:39]

http://codereview.adblockplus.org/6390087684194304/diff/5766466041282560/src/...
File src/plugin/PluginWbPassThrough.cpp (right):

http://codereview.adblockplus.org/6390087684194304/diff/5766466041282560/src/...
src/plugin/PluginWbPassThrough.cpp:194: if (nullptr == tab)
I personally don't like yoda conditions, and I think we have it in our coding
style not to use them. 

if (tab == nullptr) looks better, imho.

http://codereview.adblockplus.org/6390087684194304/diff/5766466041282560/src/...
src/plugin/PluginWbPassThrough.cpp:201: && client->ShouldBlock(src,
m_contentType, m_boundDomain, true))
Comment about yoda conditions applies here as well. And further below for all
the m_contentType checks.

http://codereview.adblockplus.org/6390087684194304/diff/5766466041282560/src/...
src/plugin/PluginWbPassThrough.cpp:231: if
(CFilter::EContentType::contentTypeXmlHttpRequest == m_contentType)
Looks like we don't need a special case for contentTypeXmlHttpRequest do we? The
'if' below will do the same anyway.

http://codereview.adblockplus.org/6390087684194304/diff/5766466041282560/src/...
src/plugin/PluginWbPassThrough.cpp:240:
m_spInternetProtocolSink->ReportResult(INET_E_REDIRECTING, 301, L"data:");
I don't think redirecting all requests to 'data:' is a good idea. This should be
done only for predefined types (contentTypeObject and
contentTypeObjectSubrequest, I think). Otherwise IE might present a download
prompt to the user, if we block CSS, for example. Redirecting to
res://mshtml.dll/blank.htm is a safer option here from my experience.