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

Issue 29332980: Issue #1484 - Use the currently distributed version of PassthroughAPP unaltered

Created:
Dec. 22, 2015, 3:01 p.m. by Eric
Modified:
May 19, 2016, 4:10 p.m.
Reviewers:
sergei, Oleksandr
CC:
Felix Dahlke
Visibility:
Public.

Description

Issue #1484 - Use the currently distributed version of PassthroughAPP unaltered Replace the PassthroughAPP code with the currently-released version. https://github.com/salsita/passthruapp This change set does not change anything about the build. It continues to use the same directory for PassthroughAPP as before. Add definition for minimum IE version of 7.0. New version of PassthroughAPP uses interfaces introduced with that version, including 'OnStartEx()'. The new version of PassthroughAPP calls this entry point instead of the older one. Change 'WBPassthruSink::OnStart()' to 'OnStartEx()'.

Patch Set 1 : new version of PassthroughAPP #

Patch Set 2 : use OnStartEx() instead of OnStart() #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+1303 lines, -989 lines) Patch
M src/plugin/PluginStdAfx.h View 1 chunk +6 lines, -0 lines 2 comments Download
M src/plugin/PluginWbPassThrough.h View 1 1 chunk +1 line, -1 line 2 comments Download
M src/plugin/PluginWbPassThrough.cpp View 1 1 chunk +2 lines, -2 lines 0 comments Download
M src/plugin/passthroughapp/PassthroughObject.h View 1 chunk +8 lines, -25 lines 0 comments Download
M src/plugin/passthroughapp/ProtocolCF.h View 2 chunks +54 lines, -68 lines 0 comments Download
M src/plugin/passthroughapp/ProtocolCF.inl View 4 chunks +3 lines, -23 lines 0 comments Download
M src/plugin/passthroughapp/ProtocolImpl.h View 2 chunks +367 lines, -300 lines 0 comments Download
M src/plugin/passthroughapp/ProtocolImpl.inl View 1 chunk +674 lines, -531 lines 0 comments Download
A src/plugin/passthroughapp/README.md View 1 chunk +107 lines, -0 lines 0 comments Download
M src/plugin/passthroughapp/SinkPolicy.h View 4 chunks +8 lines, -18 lines 0 comments Download
M src/plugin/passthroughapp/SinkPolicy.inl View 4 chunks +49 lines, -21 lines 0 comments Download
A src/plugin/passthroughapp/UNLICENSE View 1 chunk +24 lines, -0 lines 0 comments Download

Messages

Total messages: 5
Eric
New PassthroughAPP code is identical to what's on github. The review tool diff is not ...
Dec. 22, 2015, 5:23 p.m. (2015-12-22 17:23:04 UTC) #1
Eric
Patch set 1 is the external version of PassthroughAPP and a definition needed to get ...
Dec. 23, 2015, 3:23 p.m. (2015-12-23 15:23:39 UTC) #2
Oleksandr
LGTM
Feb. 5, 2016, 7:02 a.m. (2016-02-05 07:02:12 UTC) #3
sergei
https://codereview.adblockplus.org/29332980/diff/29333053/src/plugin/PluginStdAfx.h File src/plugin/PluginStdAfx.h (right): https://codereview.adblockplus.org/29332980/diff/29333053/src/plugin/PluginStdAfx.h#newcode41 src/plugin/PluginStdAfx.h:41: #define _WIN32_IE _WIN32_IE_IE70 We support IE starting from IE ...
Feb. 26, 2016, 7:53 p.m. (2016-02-26 19:53:57 UTC) #4
Eric
May 19, 2016, 4:10 p.m. (2016-05-19 16:10:20 UTC) #5
https://codereview.adblockplus.org/29332980/diff/29333053/src/plugin/PluginSt...
File src/plugin/PluginStdAfx.h (right):

https://codereview.adblockplus.org/29332980/diff/29333053/src/plugin/PluginSt...
src/plugin/PluginStdAfx.h:41: #define _WIN32_IE _WIN32_IE_IE70
On 2016/02/26 19:53:56, sergei wrote:
> We support IE starting from IE 8, wouldn't it be better to use
`_WIN32_IE_IE80`
> here?

The way these definitions work, they turn on new features introduced in each
browser, possibly changing behaviors. Using an older definition is a more
conservative choice, with less possibility of regression.

If there is some IE 8+ feature we need, we can always bump it up. The new
version of PassthroughAPP requires minimum IE 7, so that's what I used in order
to avoid potential complications.

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

https://codereview.adblockplus.org/29332980/diff/29333053/src/plugin/PluginWb...
src/plugin/PluginWbPassThrough.h:68: HRESULT OnStartEx(IUri* pUri,
IInternetProtocolSink* pOIProtSink,
On 2016/02/26 19:53:56, sergei wrote:
> There is still Start method which calls OnStart, is it really never called
now?

It doesn't appear to be called at all. I verified this with the debugger.

Frankly, I'm not sure why 'OnStart' is still in the PassthroughAPP code base.
Recall, though, that people other than the original author Tandetnik are now
maintaining the code. I suspect that it's an oversight.

Powered by Google App Engine
This is Rietveld