Apply HSTS rules to also upgrade ws:// -> wss:// if appropriate. This avoids

Apply HSTS rules to also upgrade ws:// -> wss:// if appropriate. This avoids
a minor issue whereby failure to set a cookie "Secure" can get leaked via
a WebSocket when http itself is mitiagted.

TEST=WebSocketJobTest.HSTSUpgrade
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=82069

[Chris Evans, 2011-04-16 03:37:05]

(Simpler than it looks - some of the change is a minor refactor to share the new
IsSNIAvailable function).

[abarth-chromium, 2011-04-16 03:58:07]

Test?

[Chris Evans, 2011-04-16 06:25:31]

On 2011/04/16 03:58:07, abarth wrote:
> Test?

Good idea, test added. WebSocketJobTest.HSTSUpgrade

[abarth-chromium, 2011-04-16 06:56:05]

LGTM, but you'll probably also need review from a net OWNER.

[darin1, 2011-04-19 04:45:28]

LGTM

[willchan no longer on Chromium, 2011-04-29 18:47:13]

Who reviewed this that was in net/OWNERS? Do I have to use set noparent? :(

On 2011/04/19 04:45:28, darin1 wrote:
> LGTM

[willchan no longer on Chromium, 2011-04-29 19:09:46]

http://codereview.chromium.org/6873029/diff/3003/net/socket_stream/socket_str...
File net/socket_stream/socket_stream_job.h (right):

http://codereview.chromium.org/6873029/diff/3003/net/socket_stream/socket_str...
net/socket_stream/socket_stream_job.h:37: const URLRequestContext& context);
Please pass in the exact dependencies. I'm trying to reduce the scope of
URLRequestContext in net/.

http://codereview.chromium.org/6873029/diff/3003/net/url_request/url_request_...
File net/url_request/url_request_context.cc (right):

http://codereview.chromium.org/6873029/diff/3003/net/url_request/url_request_...
net/url_request/url_request_context.cc:61: bool
URLRequestContext::IsSNIAvailable() const {
This is purely dependent on the SSLConfigService. Can you make this a member of
SSLConfigService instead? Or a free/static function (in ssl_config_service.h)
that takes in a const SSLConfigService& parameter?

This will help eliminate the need to pass URLRequestContext everywhere.

[cevans, 2011-04-29 19:14:49]

Thanks Will and sorry you weren't on the original review. I filed
http://code.google.com/p/chromium/issues/detail?id=81009 against myself to
make sure this incremental update doesn't get forgotten. I should be able to
deal with it next week.

On Fri, Apr 29, 2011 at 12:09 PM, <willchan@chromium.org> wrote:

>
>
>
http://codereview.chromium.org/6873029/diff/3003/net/socket_stream/socket_str...
> File net/socket_stream/socket_stream_job.h (right):
>
>
>
http://codereview.chromium.org/6873029/diff/3003/net/socket_stream/socket_str...
> net/socket_stream/socket_stream_job.h:37: const URLRequestContext&
> context);
> Please pass in the exact dependencies. I'm trying to reduce the scope of
> URLRequestContext in net/.
>
>
>
http://codereview.chromium.org/6873029/diff/3003/net/url_request/url_request_...
> File net/url_request/url_request_context.cc (right):
>
>
>
http://codereview.chromium.org/6873029/diff/3003/net/url_request/url_request_...
> net/url_request/url_request_context.cc:61: bool
> URLRequestContext::IsSNIAvailable() const {
> This is purely dependent on the SSLConfigService. Can you make this a
> member of SSLConfigService instead? Or a free/static function (in
> ssl_config_service.h) that takes in a const SSLConfigService& parameter?
>
> This will help eliminate the need to pass URLRequestContext everywhere.
>
>
> http://codereview.chromium.org/6873029/
>

[willchan no longer on Chromium, 2011-04-29 19:16:32]

Np, thanks for being willing to address it post-commit. I'm doing a lot of
work cleaning up URLRequestContext and am trying to stay on top of changes
to it.

On Fri, Apr 29, 2011 at 12:14 PM, Chris Evans <cevans@google.com> wrote:

> Thanks Will and sorry you weren't on the original review. I filed
> http://code.google.com/p/chromium/issues/detail?id=81009 against myself to
> make sure this incremental update doesn't get forgotten. I should be able to
> deal with it next week.
>
>
> On Fri, Apr 29, 2011 at 12:09 PM, <willchan@chromium.org> wrote:
>
>>
>>
>>
http://codereview.chromium.org/6873029/diff/3003/net/socket_stream/socket_str...
>> File net/socket_stream/socket_stream_job.h (right):
>>
>>
>>
http://codereview.chromium.org/6873029/diff/3003/net/socket_stream/socket_str...
>> net/socket_stream/socket_stream_job.h:37: const URLRequestContext&
>> context);
>> Please pass in the exact dependencies. I'm trying to reduce the scope of
>> URLRequestContext in net/.
>>
>>
>>
http://codereview.chromium.org/6873029/diff/3003/net/url_request/url_request_...
>> File net/url_request/url_request_context.cc (right):
>>
>>
>>
http://codereview.chromium.org/6873029/diff/3003/net/url_request/url_request_...
>> net/url_request/url_request_context.cc:61: bool
>> URLRequestContext::IsSNIAvailable() const {
>> This is purely dependent on the SSLConfigService. Can you make this a
>> member of SSLConfigService instead? Or a free/static function (in
>> ssl_config_service.h) that takes in a const SSLConfigService& parameter?
>>
>> This will help eliminate the need to pass URLRequestContext everywhere.
>>
>>
>> http://codereview.chromium.org/6873029/
>>
>
>

[darin (slow to review), 2011-04-29 19:24:42]

IIRC, I was asked to review the webkit/* changes in this patch.  I should
have
made note of that in my LGTM comment.

-Darin


On Fri, Apr 29, 2011 at 11:47 AM, <willchan@chromium.org> wrote:

> Who reviewed this that was in net/OWNERS? Do I have to use set noparent? :(
>
> On 2011/04/19 04:45:28, darin1 wrote:
>
>> LGTM
>>
>
>
>
> http://codereview.chromium.org/6873029/
>