Remove the last of the TLS fallback code.

Remove the last of the TLS fallback code.

It's gone and TLS 1.3 is not going to force it back.
Remove the last of the dead code.

Note this has a small behavior change: if we load a
cached top-level resource which previously used the
fallback and wasn't revalidated since the fallback was
removed, the website_settings.cc logic will no longer
show a message about this. But that message was only
ever shown on Android and only if you went looking for the
connection info.

(There's some other stale code there like downgrading
the connection status on SSLv3, but we may wish to
drop the cache entry instead in that case. Though in
general we don't really do the full provenance tracking
so it's largely a UI thing. Notably revalidations and
range requests will clobber one set of flags with another.
Will poke at that in a follow-up.)

BUG=621780
Committed: https://crrev.com/471d313caec220449197a77ae369787f34fbbc47
Cr-Commit-Position: refs/heads/master@{#423545}

[davidben, 2016-09-30 04:51:16]

The CQ bit was checked by davidben@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-30 04:51:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-30 07:21:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-30 07:21:56]

Dry run: This issue passed the CQ dry run.

[davidben, 2016-09-30 13:13:21]

davidben@chromium.org changed reviewers:
+ mmenke@chromium.org

[mmenke, 2016-10-03 16:01:04]

I guess the production code that actually initiates fallback has already been
removed?

Could you please reference whatever bug was used for that removal in this CL
description?

https://codereview.chromium.org/2382983002/diff/1/net/socket/ssl_client_socke...
File net/socket/ssl_client_socket_impl.cc (left):

https://codereview.chromium.org/2382983002/diff/1/net/socket/ssl_client_socke...
net/socket/ssl_client_socket_impl.cc:799: ssl_info->connection_status |=
SSL_CONNECTION_VERSION_FALLBACK;
Can we remove SSL_CONNECTION_VERSION_FALLBACK from
ssl_connection_status_flags.h?

https://codereview.chromium.org/2382983002/diff/1/net/url_request/url_request...
File net/url_request/url_request_unittest.cc (left):

https://codereview.chromium.org/2382983002/diff/1/net/url_request/url_request...
net/url_request/url_request_unittest.cc:8767: SSL_CONNECTION_VERSION_FALLBACK);
Can we remove SSL_CONNECTION_VERSION_FALLBACK?

[davidben, 2016-10-03 18:52:39]

Description was changed from

==========
Remove the last of the TLS fallback code.

BUG=none
==========

to

==========
Remove the last of the TLS fallback code.

BUG=621780
==========

[davidben, 2016-10-03 18:52:56]

Description was changed from

==========
Remove the last of the TLS fallback code.

BUG=621780
==========

to

==========
Remove the last of the TLS fallback code.

It's gone and TLS 1.3 is not going to force it back.

BUG=621780
==========

[davidben, 2016-10-03 19:08:08]

Description was changed from

==========
Remove the last of the TLS fallback code.

It's gone and TLS 1.3 is not going to force it back.

BUG=621780
==========

to

==========
Remove the last of the TLS fallback code.

It's gone and TLS 1.3 is not going to force it back.

Note this CL comes with a behavior change: if we load
a cache entry which was previously fetched under fallback
and hasn't been revalidated since the fallback was
removed, the website_settings.cc logic will no longer
show a message about this. (Note that message was only
ever shown on Android and if you went looking for the
connection info.)

BUG=621780
==========

[davidben, 2016-10-03 19:08:57]

Description was changed from

==========
Remove the last of the TLS fallback code.

It's gone and TLS 1.3 is not going to force it back.

Note this CL comes with a behavior change: if we load
a cache entry which was previously fetched under fallback
and hasn't been revalidated since the fallback was
removed, the website_settings.cc logic will no longer
show a message about this. (Note that message was only
ever shown on Android and if you went looking for the
connection info.)

BUG=621780
==========

to

==========
Remove the last of the TLS fallback code.

It's gone and TLS 1.3 is not going to force it back.

Note this CL comes with a behavior change: if we load
a cache entry which was previously fetched under fallback
and hasn't been revalidated since the fallback was
removed, the website_settings.cc logic will no longer
show a message about this. Note that message was only
ever shown on Android and if you went looking for the
connection info.

(There's some other stale code there like downgrading
the connection status on SSLv3, but we may wish to
drop the cache entry instead in that case.)

BUG=621780
==========

[davidben, 2016-10-03 19:09:24]

https://codereview.chromium.org/2382983002/diff/1/net/socket/ssl_client_socke...
File net/socket/ssl_client_socket_impl.cc (left):

https://codereview.chromium.org/2382983002/diff/1/net/socket/ssl_client_socke...
net/socket/ssl_client_socket_impl.cc:799: ssl_info->connection_status |=
SSL_CONNECTION_VERSION_FALLBACK;
On 2016/10/03 16:01:03, mmenke wrote:
> Can we remove SSL_CONNECTION_VERSION_FALLBACK from
> ssl_connection_status_flags.h?

Done. Note this changes behavior slightly, but I think it's fine? Updated commit
description accordingly. The other option is we drop those cache entries on the
floor, which isn't wrong.

https://codereview.chromium.org/2382983002/diff/1/net/url_request/url_request...
File net/url_request/url_request_unittest.cc (left):

https://codereview.chromium.org/2382983002/diff/1/net/url_request/url_request...
net/url_request/url_request_unittest.cc:8767: SSL_CONNECTION_VERSION_FALLBACK);
On 2016/10/03 16:01:03, mmenke wrote:
> Can we remove SSL_CONNECTION_VERSION_FALLBACK?

Done.

[davidben, 2016-10-03 19:14:02]

Description was changed from

==========
Remove the last of the TLS fallback code.

It's gone and TLS 1.3 is not going to force it back.

Note this CL comes with a behavior change: if we load
a cache entry which was previously fetched under fallback
and hasn't been revalidated since the fallback was
removed, the website_settings.cc logic will no longer
show a message about this. Note that message was only
ever shown on Android and if you went looking for the
connection info.

(There's some other stale code there like downgrading
the connection status on SSLv3, but we may wish to
drop the cache entry instead in that case.)

BUG=621780
==========

to

==========
Remove the last of the TLS fallback code.

It's gone and TLS 1.3 is not going to force it back.

Note this CL comes with a behavior change: if we load
a cache entry which was previously fetched under fallback
and hasn't been revalidated since the fallback was
removed, the website_settings.cc logic will no longer
show a message about this. Note that message was only
ever shown on Android and if you went looking for the
connection info.

(There's some other stale code there like downgrading
the connection status on SSLv3, but we may wish to
drop the cache entry instead in that case. Though in
general we don't really do the full provenance tracking
here. Notably revalidations and range requests will
clobber one set of flags with another.)

BUG=621780
==========

[mmenke, 2016-10-03 20:12:01]

LGTM

https://codereview.chromium.org/2382983002/diff/1/net/socket/ssl_client_socke...
File net/socket/ssl_client_socket_impl.cc (left):

https://codereview.chromium.org/2382983002/diff/1/net/socket/ssl_client_socke...
net/socket/ssl_client_socket_impl.cc:799: ssl_info->connection_status |=
SSL_CONNECTION_VERSION_FALLBACK;
On 2016/10/03 19:09:24, davidben wrote:
> On 2016/10/03 16:01:03, mmenke wrote:
> > Can we remove SSL_CONNECTION_VERSION_FALLBACK from
> > ssl_connection_status_flags.h?
> 
> Done. Note this changes behavior slightly, but I think it's fine? Updated
commit
> description accordingly. The other option is we drop those cache entries on
the
> floor, which isn't wrong.

I think it's fine, but I'd also be fine with dropping them.

[davidben, 2016-10-03 23:39:18]

Description was changed from

==========
Remove the last of the TLS fallback code.

It's gone and TLS 1.3 is not going to force it back.

Note this CL comes with a behavior change: if we load
a cache entry which was previously fetched under fallback
and hasn't been revalidated since the fallback was
removed, the website_settings.cc logic will no longer
show a message about this. Note that message was only
ever shown on Android and if you went looking for the
connection info.

(There's some other stale code there like downgrading
the connection status on SSLv3, but we may wish to
drop the cache entry instead in that case. Though in
general we don't really do the full provenance tracking
here. Notably revalidations and range requests will
clobber one set of flags with another.)

BUG=621780
==========

to

==========
Remove the last of the TLS fallback code.

It's gone and TLS 1.3 is not going to force it back.
Remove the last of the dead code.

Note this has a small behavior change: if we load a
cached top-level resource which previously used the
fallback and wasn't revalidated since the fallback was
removed, the website_settings.cc logic will no longer
show a message about this. But that message was only
ever shown on Android and only if you went looking for the
connection info.

(There's some other stale code there like downgrading
the connection status on SSLv3, but we may wish to
drop the cache entry instead in that case. Though in
general we don't really do the full provenance tracking
so it's largely a UI thing. Notably revalidations and
range requests will clobber one set of flags with another.
Will poke at that in a follow-up.)

BUG=621780
==========

[davidben, 2016-10-03 23:39:32]

davidben@chromium.org changed reviewers:
+ blundell@chromium.org, felt@chromium.org

[davidben, 2016-10-03 23:39:32]

+felt for website_settings.cc
+blundell for components/pageinfo_strings.grdp

[blundell, 2016-10-04 06:03:39]

//components lgtm

[felt, 2016-10-06 01:11:03]

felt@chromium.org changed reviewers:
+ lgarron@chromium.org

[felt, 2016-10-06 01:11:54]

lgarron, can you pls review the change as it affects website_settings?

[lgarron, 2016-10-06 05:44:19]

website_settings and pageinfo_strings LGTM

[felt, 2016-10-06 10:08:55]

thx lucas. also lgtm

[davidben, 2016-10-06 13:39:22]

The CQ bit was checked by davidben@chromium.org

[commit-bot: I haz the power, 2016-10-06 13:39:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-06 15:39:58]

Description was changed from

==========
Remove the last of the TLS fallback code.

It's gone and TLS 1.3 is not going to force it back.
Remove the last of the dead code.

Note this has a small behavior change: if we load a
cached top-level resource which previously used the
fallback and wasn't revalidated since the fallback was
removed, the website_settings.cc logic will no longer
show a message about this. But that message was only
ever shown on Android and only if you went looking for the
connection info.

(There's some other stale code there like downgrading
the connection status on SSLv3, but we may wish to
drop the cache entry instead in that case. Though in
general we don't really do the full provenance tracking
so it's largely a UI thing. Notably revalidations and
range requests will clobber one set of flags with another.
Will poke at that in a follow-up.)

BUG=621780
==========

to

==========
Remove the last of the TLS fallback code.

It's gone and TLS 1.3 is not going to force it back.
Remove the last of the dead code.

Note this has a small behavior change: if we load a
cached top-level resource which previously used the
fallback and wasn't revalidated since the fallback was
removed, the website_settings.cc logic will no longer
show a message about this. But that message was only
ever shown on Android and only if you went looking for the
connection info.

(There's some other stale code there like downgrading
the connection status on SSLv3, but we may wish to
drop the cache entry instead in that case. Though in
general we don't really do the full provenance tracking
so it's largely a UI thing. Notably revalidations and
range requests will clobber one set of flags with another.
Will poke at that in a follow-up.)

BUG=621780
==========

[commit-bot: I haz the power, 2016-10-06 15:39:59]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-06 15:43:40]

Description was changed from

==========
Remove the last of the TLS fallback code.

It's gone and TLS 1.3 is not going to force it back.
Remove the last of the dead code.

Note this has a small behavior change: if we load a
cached top-level resource which previously used the
fallback and wasn't revalidated since the fallback was
removed, the website_settings.cc logic will no longer
show a message about this. But that message was only
ever shown on Android and only if you went looking for the
connection info.

(There's some other stale code there like downgrading
the connection status on SSLv3, but we may wish to
drop the cache entry instead in that case. Though in
general we don't really do the full provenance tracking
so it's largely a UI thing. Notably revalidations and
range requests will clobber one set of flags with another.
Will poke at that in a follow-up.)

BUG=621780
==========

to

==========
Remove the last of the TLS fallback code.

It's gone and TLS 1.3 is not going to force it back.
Remove the last of the dead code.

Note this has a small behavior change: if we load a
cached top-level resource which previously used the
fallback and wasn't revalidated since the fallback was
removed, the website_settings.cc logic will no longer
show a message about this. But that message was only
ever shown on Android and only if you went looking for the
connection info.

(There's some other stale code there like downgrading
the connection status on SSLv3, but we may wish to
drop the cache entry instead in that case. Though in
general we don't really do the full provenance tracking
so it's largely a UI thing. Notably revalidations and
range requests will clobber one set of flags with another.
Will poke at that in a follow-up.)

BUG=621780

Committed: https://crrev.com/471d313caec220449197a77ae369787f34fbbc47
Cr-Commit-Position: refs/heads/master@{#423545}
==========

[commit-bot: I haz the power, 2016-10-06 15:43:41]

Patchset 2 (id:??) landed as
https://crrev.com/471d313caec220449197a77ae369787f34fbbc47
Cr-Commit-Position: refs/heads/master@{#423545}