Do not wrap non-HTTP(s) requests.

Do not wrap non-HTTP(s) requests.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=52494

[tommi, 2010-07-14 19:18:43]

lgtm with one comment.

http://codereview.chromium.org/2893015/diff/4001/5001
File chrome_frame/protocol_sink_wrap.cc (right):

http://codereview.chromium.org/2893015/diff/4001/5001#newcode152
chrome_frame/protocol_sink_wrap.cc:152: if ((url != StrStrW(url, L"http://")) &&
(url != StrStrW(url, L"https://")))
use StartsWith?

[stoyan, 2010-07-14 19:34:10]

http://codereview.chromium.org/2893015/diff/4001/5001
File chrome_frame/protocol_sink_wrap.cc (right):

http://codereview.chromium.org/2893015/diff/4001/5001#newcode152
chrome_frame/protocol_sink_wrap.cc:152: if ((url != StrStrW(url, L"http://")) &&
(url != StrStrW(url, L"https://")))
StartsWith wants me to create std::string object.
And apparently there is not StringPieceW::starts_with()

[tommi, 2010-07-14 19:39:31]

crap.  I guess you could use base::strncasecmp like StartsWith does? :-/
also, you don't need do include the //, right?
E.g. just compare against "http:" or "https:"?

On Wed, Jul 14, 2010 at 3:34 PM, <stoyan@chromium.org> wrote:

>
> http://codereview.chromium.org/2893015/diff/4001/5001
> File chrome_frame/protocol_sink_wrap.cc (right):
>
> http://codereview.chromium.org/2893015/diff/4001/5001#newcode152
> chrome_frame/protocol_sink_wrap.cc:152: if ((url != StrStrW(url,
> L"http://")) && (url != StrStrW(url, L"https://")))
> StartsWith wants me to create std::string object.
> And apparently there is not StringPieceW::starts_with()
>
>
> http://codereview.chromium.org/2893015/show
>

[stoyan, 2010-07-14 19:46:52]

On 2010/07/14 19:39:31, tommi wrote:
> crap.  I guess you could use base::strncasecmp like StartsWith does? :-/

strncasecmp - 1) ignores the case 2) is ASCII only.

> also, you don't need do include the //, right?

Is including "//" wrong? I was aiming for the longest match.

> E.g. just compare against "http:" or "https:"?

[tommi, 2010-07-14 19:54:07]

On Wed, Jul 14, 2010 at 3:46 PM, <stoyan@chromium.org> wrote:

> On 2010/07/14 19:39:31, tommi wrote:
>
>> crap.  I guess you could use base::strncasecmp like StartsWith does? :-/
>>
>
> strncasecmp - 1) ignores the case 2) is ASCII only.


ah, right! details details.


>
>  also, you don't need do include the //, right?
>>
>
> Is including "//" wrong? I was aiming for the longest match.

nope, it's not wrong but neither is http:.  I was thinking that there's
probably a constant string for that somewhere already.
It's up to you though and it's just hairsplitting.  I think we've already
spent enough time, so just submit :)

>
>
>  E.g. just compare against "http:" or "https:"?
>>
>
>
>
> http://codereview.chromium.org/2893015/show
>