Change QuicStreamRequest::Request() to take a preferred QuicVersion so that

Change QuicStreamRequest::Request() to take a preferred QuicVersion so that 
the created QuicConnection will use the designated QuicVersion instead
of hardcoded QuicVersion in HttpNetworkSession::Param().quic_supported_versions.

BUG=689762
Review-Url: https://codereview.chromium.org/2958133002
Cr-Commit-Position: refs/heads/master@{#484769}
Committed: https://chromium.googlesource.com/chromium/src/+/a00ca01914a13a1ff5e5fa5d24c746246dd89449

[Zhongyi Shi, 2017-06-28 17:19:37]

Description was changed from

==========
Change QuicStreamFactory::Request to take a preferred QuicVersion so that
the created QuicConnection will use the designated QuicVersion in stead
of hardcoded QuicVersion in HttpNetworkSession::Param().quic_supported_versions.

BUG=689762
==========

to

==========
Change QuicStreamFactory::Request to take a preferred QuicVersion so that
the created QuicConnection will use the designated QuicVersion instead
of hardcoded QuicVersion in HttpNetworkSession::Param().quic_supported_versions.

BUG=689762
==========

[Zhongyi Shi, 2017-06-28 17:22:11]

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

[commit-bot: I haz the power, 2017-06-28 17:22:34]

Dry run: CQ is trying da patch.

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

[Zhongyi Shi, 2017-06-28 18:14:11]

zhongyi@chromium.org changed reviewers:
+ bnc@chromium.org, rch@chromium.org

[Zhongyi Shi, 2017-06-28 18:14:12]

Ryan and Bence: PTAL, I will add QuicNetworkTransactionTests once this early
patch looks good, thanks!

[commit-bot: I haz the power, 2017-06-28 18:39:01]

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

[commit-bot: I haz the power, 2017-06-28 18:39:02]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Ryan Hamilton, 2017-06-28 18:53:15]

Wow, this CL is astonishingly small!

https://codereview.chromium.org/2958133002/diff/20001/net/http/http_server_pr...
File net/http/http_server_properties.h (right):

https://codereview.chromium.org/2958133002/diff/20001/net/http/http_server_pr...
net/http/http_server_properties.h:180: HostPortPair host_port_pair() const {
Should this be a const& to avoid the (possible) string copy?

https://codereview.chromium.org/2958133002/diff/20001/net/http/http_stream_fa...
File net/http/http_stream_factory_impl_job.h (right):

https://codereview.chromium.org/2958133002/diff/20001/net/http/http_stream_fa...
net/http/http_stream_factory_impl_job.h:151: virtual const QuicVersionVector&
GetAdvertisedQuicVersions() const = 0;
nit: Comment.

https://codereview.chromium.org/2958133002/diff/20001/net/http/http_stream_fa...
File net/http/http_stream_factory_impl_job_controller.cc (right):

https://codereview.chromium.org/2958133002/diff/20001/net/http/http_stream_fa...
net/http/http_stream_factory_impl_job_controller.cc:827: origin_url,
alternative_service_info_.protocol(),
Since we're already passing in some of the AlternateServiceInfo fields into the
AltJob constructor, how 'bout passing in either all of it, or just the QUIC
versions and avoiding the upcall to fetch advertised QUIC versions from the Job?

https://codereview.chromium.org/2958133002/diff/20001/net/quic/chromium/quic_...
File net/quic/chromium/quic_stream_factory.h (right):

https://codereview.chromium.org/2958133002/diff/20001/net/quic/chromium/quic_...
net/quic/chromium/quic_stream_factory.h:118: const QuicVersionVector&
advertised_versions,
The CL description talks about adding a single version to
QuicStreamFactory::Request. Does that class exist? This change is to
QuicStreamRequest (which is close), but it adds a list of versions not a single
version.

[Bence, 2017-06-28 19:08:58]

https://codereview.chromium.org/2958133002/diff/40001/net/http/http_server_pr...
File net/http/http_server_properties.h (right):

https://codereview.chromium.org/2958133002/diff/40001/net/http/http_server_pr...
net/http/http_server_properties.h:178: const NextProto& protocol() const {
return alternative_service_.protocol; }
NextProto is an enum, might be better to return by value not by reference.

https://codereview.chromium.org/2958133002/diff/40001/net/quic/chromium/quic_...
File net/quic/chromium/quic_network_transaction_unittest.cc (right):

https://codereview.chromium.org/2958133002/diff/40001/net/quic/chromium/quic_...
net/quic/chromium/quic_network_transaction_unittest.cc:1421: // Client and
Server mutualy support more than one QUIC_VERSION.
s/mutualy/mutually/

https://codereview.chromium.org/2958133002/diff/40001/net/quic/chromium/quic_...
File net/quic/chromium/quic_stream_factory.cc (right):

https://codereview.chromium.org/2958133002/diff/40001/net/quic/chromium/quic_...
net/quic/chromium/quic_stream_factory.cc:1615: return supported_versions_[0];
Optional: add DCHECK(!supported_versions_.empty()); to QuicStreamFactory
constructor.

https://codereview.chromium.org/2958133002/diff/40001/net/quic/chromium/quic_...
File net/quic/chromium/quic_stream_factory_test.cc (right):

https://codereview.chromium.org/2958133002/diff/40001/net/quic/chromium/quic_...
net/quic/chromium/quic_stream_factory_test.cc:953: // supportd.
QuicStreamFactory::Request should select the mutually supported
s/supportd/supported/

[Bence, 2017-06-28 20:01:02]

Can you tell me more about how QUIC version negotiation works during the
handshake between the client and the server?  While I think it is great to store
information about what QUIC versions the server claimed to support so that no
QUIC connection is tried if there is no overlap, but I'm worried that if the
server is upgraded since the Chromium last received an AltSvc header, or if the
server is sending outdated AltSvc headers, this change might hinder the ability
to establish a QUIC connection.  Why not being open to any QUIC version that the
network stack supports when creating a new connection, instead of constraining
to only one that we think the server supports but it might not any more?

[Zhongyi Shi, 2017-07-05 19:55:27]

Patchset #5 (id:80001) has been deleted

[Zhongyi Shi, 2017-07-05 22:31:55]

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

[commit-bot: I haz the power, 2017-07-05 22:32:34]

Dry run: CQ is trying da patch.

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

[Zhongyi Shi, 2017-07-05 22:50:18]

Patchset #7 (id:140001) has been deleted

[Zhongyi Shi, 2017-07-05 22:50:25]

Patchset #4 (id:60001) has been deleted

[Zhongyi Shi, 2017-07-05 22:52:02]

Description was changed from

==========
Change QuicStreamFactory::Request to take a preferred QuicVersion so that
the created QuicConnection will use the designated QuicVersion instead
of hardcoded QuicVersion in HttpNetworkSession::Param().quic_supported_versions.

BUG=689762
==========

to

==========
Change QuicStreamRequest::Request() to take a preferred QuicVersion so that 
the created QuicConnection will use the designated QuicVersion instead
of hardcoded QuicVersion in HttpNetworkSession::Param().quic_supported_versions.

BUG=689762
==========

[Zhongyi Shi, 2017-07-05 23:06:56]

On 2017/06/28 20:01:02, Bence wrote:
> Can you tell me more about how QUIC version negotiation works during the
> handshake between the client and the server? While I think it is great to
store
> information about what QUIC versions the server claimed to support so that no
> QUIC connection is tried if there is no overlap, but I'm worried that if the
> server is upgraded since the Chromium last received an AltSvc header, or if
the
> server is sending outdated AltSvc headers, this change might hinder the
ability
> to establish a QUIC connection.  Why not being open to any QUIC version that
the
> network stack supports when creating a new connection, instead of constraining
> to only one that we think the server supports but it might not any more?

The design doc is at go/chrome-multi-version-quic.

Bence, I am happy to help explain the QUIC version negotiation if you are
interested. However, to be clear, this project has nothing to do with version
negotiation. The reason we want to store the information about server advertised
versions is to make sure we can create a QUIC connection with correct QUIC
version. Different servers might want to use different QUIC versions and we are
adding support for that case. If a server upgraded, it should advertises new
AltSvc to client. If the server sends an outdated AltSvc header, client will
also receive new AltSvc header. Depends on the versions advertised in AltSvc
header, the client might update HttpServerProperties. If updated (i.e., there's
a shared version), next time QuicStreamFactory will pick up a QuicVersion based
on its best knowledge to create the QUIC connection. Otherwise,
QuicStreamFactory will pick up the same QUIC version as it does last time, and
connect to the server. If the version doesn't work, we might mark QUIC broken
and retry later. Does that make sense?

[Zhongyi Shi, 2017-07-05 23:08:59]

Rebased after splitting the logic to always pick up an AlternativeServiceInfo
advertising a QUIC version that is supported. 

Ryan, PTAL, thanks!

https://codereview.chromium.org/2958133002/diff/20001/net/http/http_server_pr...
File net/http/http_server_properties.h (right):

https://codereview.chromium.org/2958133002/diff/20001/net/http/http_server_pr...
net/http/http_server_properties.h:180: HostPortPair host_port_pair() const {
On 2017/06/28 18:53:14, Ryan Hamilton wrote:
> Should this be a const& to avoid the (possible) string copy?

alternative_service_.host_port_pair() is a local temporary copy. There is no
HostPort field in AlternativeService.

https://codereview.chromium.org/2958133002/diff/20001/net/http/http_stream_fa...
File net/http/http_stream_factory_impl_job.h (right):

https://codereview.chromium.org/2958133002/diff/20001/net/http/http_stream_fa...
net/http/http_stream_factory_impl_job.h:151: virtual const QuicVersionVector&
GetAdvertisedQuicVersions() const = 0;
On 2017/06/28 18:53:14, Ryan Hamilton wrote:
> nit: Comment.

This method has been removed.

https://codereview.chromium.org/2958133002/diff/20001/net/http/http_stream_fa...
File net/http/http_stream_factory_impl_job_controller.cc (right):

https://codereview.chromium.org/2958133002/diff/20001/net/http/http_stream_fa...
net/http/http_stream_factory_impl_job_controller.cc:827: origin_url,
alternative_service_info_.protocol(),
On 2017/06/28 18:53:14, Ryan Hamilton wrote:
> Since we're already passing in some of the AlternateServiceInfo fields into
the
> AltJob constructor, how 'bout passing in either all of it, or just the QUIC
> versions and avoiding the upcall to fetch advertised QUIC versions from the
Job?

Done. Changed to only pass the selected QUIC version.

https://codereview.chromium.org/2958133002/diff/20001/net/quic/chromium/quic_...
File net/quic/chromium/quic_stream_factory.h (right):

https://codereview.chromium.org/2958133002/diff/20001/net/quic/chromium/quic_...
net/quic/chromium/quic_stream_factory.h:118: const QuicVersionVector&
advertised_versions,
On 2017/06/28 18:53:14, Ryan Hamilton wrote:
> The CL description talks about adding a single version to
> QuicStreamFactory::Request. Does that class exist? This change is to
> QuicStreamRequest (which is close), but it adds a list of versions not a
single
> version.

Thanks for pointing this out. I have updated the CL description.

https://codereview.chromium.org/2958133002/diff/40001/net/http/http_server_pr...
File net/http/http_server_properties.h (right):

https://codereview.chromium.org/2958133002/diff/40001/net/http/http_server_pr...
net/http/http_server_properties.h:178: const NextProto& protocol() const {
return alternative_service_.protocol; }
On 2017/06/28 19:08:58, Bence wrote:
> NextProto is an enum, might be better to return by value not by reference.

Done.

https://codereview.chromium.org/2958133002/diff/40001/net/quic/chromium/quic_...
File net/quic/chromium/quic_network_transaction_unittest.cc (right):

https://codereview.chromium.org/2958133002/diff/40001/net/quic/chromium/quic_...
net/quic/chromium/quic_network_transaction_unittest.cc:1421: // Client and
Server mutualy support more than one QUIC_VERSION.
On 2017/06/28 19:08:58, Bence wrote:
> s/mutualy/mutually/

Done.

https://codereview.chromium.org/2958133002/diff/40001/net/quic/chromium/quic_...
File net/quic/chromium/quic_stream_factory.cc (right):

https://codereview.chromium.org/2958133002/diff/40001/net/quic/chromium/quic_...
net/quic/chromium/quic_stream_factory.cc:1615: return supported_versions_[0];
On 2017/06/28 19:08:58, Bence wrote:
> Optional: add DCHECK(!supported_versions_.empty()); to QuicStreamFactory
> constructor.

Acknowledged.

https://codereview.chromium.org/2958133002/diff/40001/net/quic/chromium/quic_...
File net/quic/chromium/quic_stream_factory_test.cc (right):

https://codereview.chromium.org/2958133002/diff/40001/net/quic/chromium/quic_...
net/quic/chromium/quic_stream_factory_test.cc:953: // supportd.
QuicStreamFactory::Request should select the mutually supported
On 2017/06/28 19:08:58, Bence wrote:
> s/supportd/supported/

Acknowledged.

[Ryan Hamilton, 2017-07-06 13:27:45]

Looks great. Just a few comments.

https://codereview.chromium.org/2958133002/diff/160001/net/http/http_stream_f...
File net/http/http_stream_factory_impl_job.cc (right):

https://codereview.chromium.org/2958133002/diff/160001/net/http/http_stream_f...
net/http/http_stream_factory_impl_job.cc:214: ShouldForceQuic(session,
destination, origin_url, proxy_info))
Why does this only apply when "ShouldForceQuic" is true?

https://codereview.chromium.org/2958133002/diff/160001/net/http/http_stream_f...
net/http/http_stream_factory_impl_job.cc:215: quic_version_ =
session->params().quic_supported_versions[0];
nit: {}s since the if predicate is more than one line.

https://codereview.chromium.org/2958133002/diff/160001/net/quic/chromium/quic...
File net/quic/chromium/quic_stream_factory.cc (left):

https://codereview.chromium.org/2958133002/diff/160001/net/quic/chromium/quic...
net/quic/chromium/quic_stream_factory.cc:1505: Perspective::IS_CLIENT,
supported_versions_);
Can we get rid of the supported_versions_ member?

[Bence, 2017-07-06 17:08:28]

On 2017/07/05 23:06:56, Zhongyi Shi wrote:
> On 2017/06/28 20:01:02, Bence wrote:
> > Can you tell me more about how QUIC version negotiation works during the
> > handshake between the client and the server? While I think it is great to
> store
> > information about what QUIC versions the server claimed to support so that
no
> > QUIC connection is tried if there is no overlap, but I'm worried that if the
> > server is upgraded since the Chromium last received an AltSvc header, or if
> the
> > server is sending outdated AltSvc headers, this change might hinder the
> ability
> > to establish a QUIC connection.  Why not being open to any QUIC version that
> the
> > network stack supports when creating a new connection, instead of
constraining
> > to only one that we think the server supports but it might not any more?
> 
> The design doc is at go/chrome-multi-version-quic.
> 
> Bence, I am happy to help explain the QUIC version negotiation if you are
> interested. However, to be clear, this project has nothing to do with version
> negotiation. The reason we want to store the information about server
advertised
> versions is to make sure we can create a QUIC connection with correct QUIC
> version. Different servers might want to use different QUIC versions and we
are
> adding support for that case. If a server upgraded, it should advertises new
> AltSvc to client. If the server sends an outdated AltSvc header, client will
> also receive new AltSvc header. Depends on the versions advertised in AltSvc
> header, the client might update HttpServerProperties. If updated (i.e.,
there's
> a shared version), next time QuicStreamFactory will pick up a QuicVersion
based
> on its best knowledge to create the QUIC connection. Otherwise,
> QuicStreamFactory will pick up the same QUIC version as it does last time, and
> connect to the server. If the version doesn't work, we might mark QUIC broken
> and retry later. Does that make sense?

I did not understand why QuicStreamFactory needed to know what version the
server preferred if the protocol supported version negotiation in the handshake
anyway.  The answer is that the client must guess a protocol in the client
handshake, and if the guess is incorrect, then protocol negotiation takes place,
which incurs additional roundtrips.  See
https://quicwg.github.io/base-drafts/draft-ietf-quic-transport.html#rfc.secti....

Also, LGTM modulo Ryan's comments.

[Bence, 2017-07-06 17:09:02]

bnc@chromium.org changed required reviewers:
+ rch@chromium.org

[Zhongyi Shi, 2017-07-06 18:13:40]

Thank for helping the review. PTAL!

https://codereview.chromium.org/2958133002/diff/160001/net/http/http_stream_f...
File net/http/http_stream_factory_impl_job.cc (right):

https://codereview.chromium.org/2958133002/diff/160001/net/http/http_stream_f...
net/http/http_stream_factory_impl_job.cc:214: ShouldForceQuic(session,
destination, origin_url, proxy_info))
On 2017/07/06 13:27:45, Ryan Hamilton wrote:
> Why does this only apply when "ShouldForceQuic" is true?

When we create the Jobs in JobController, we pass over the quic_version if the
Job created is an alternative job and uses QUIC. When we create the main job, we
always pass in a QUIC_VERSION_UNSUPPORTED.

However, Job could also later figure out to use QUIC if the origin has forced
QUIC on. That's why we want to change |quic_version_| to a valid version in this
case.

https://codereview.chromium.org/2958133002/diff/160001/net/http/http_stream_f...
net/http/http_stream_factory_impl_job.cc:215: quic_version_ =
session->params().quic_supported_versions[0];
On 2017/07/06 13:27:45, Ryan Hamilton wrote:
> nit: {}s since the if predicate is more than one line.

Done.

[Zhongyi Shi, 2017-07-06 18:20:07]

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

[commit-bot: I haz the power, 2017-07-06 18:20:16]

Dry run: CQ is trying da patch.

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

[Zhongyi Shi, 2017-07-06 18:20:28]

zhongyi@chromium.org changed required reviewers:
- rch@chromium.org

[Ryan Hamilton, 2017-07-06 19:00:05]

LGTM, modulo one suggestion

https://codereview.chromium.org/2958133002/diff/160001/net/http/http_stream_f...
File net/http/http_stream_factory_impl_job.cc (right):

https://codereview.chromium.org/2958133002/diff/160001/net/http/http_stream_f...
net/http/http_stream_factory_impl_job.cc:214: ShouldForceQuic(session,
destination, origin_url, proxy_info))
On 2017/07/06 18:13:39, Zhongyi Shi wrote:
> On 2017/07/06 13:27:45, Ryan Hamilton wrote:
> > Why does this only apply when "ShouldForceQuic" is true?
> 
> When we create the Jobs in JobController, we pass over the quic_version if the
> Job created is an alternative job and uses QUIC. When we create the main job,
we
> always pass in a QUIC_VERSION_UNSUPPORTED.

(I guess an alternative would be to pass in supported_versions_[0] for the Main
job, to handle this case?

> However, Job could also later figure out to use QUIC if the origin has forced
> QUIC on. That's why we want to change |quic_version_| to a valid version in
this
> case.

Ah! That makes sense. Can you add a comment to this effect? Also, I assume that
this DCHECK would be valid?

if (using_quic_) 
  DCHECK(quic_version != QUIC_VERSION_UNSUPPORTED)

[Zhongyi Shi, 2017-07-06 19:37:30]

The CQ bit was checked by zhongyi@chromium.org

[Zhongyi Shi, 2017-07-06 19:37:30]

The patchset sent to the CQ was uploaded after l-g-t-m from bnc@chromium.org,
rch@chromium.org
Link to the patchset: https://codereview.chromium.org/2958133002/#ps220001
(title: "Re #26")

[commit-bot: I haz the power, 2017-07-06 19:37:51]

CQ is trying da patch.

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

[Zhongyi Shi, 2017-07-06 19:40:56]

Thanks Ryan and Bence helping the review, landing now.

https://codereview.chromium.org/2958133002/diff/160001/net/http/http_stream_f...
File net/http/http_stream_factory_impl_job.cc (right):

https://codereview.chromium.org/2958133002/diff/160001/net/http/http_stream_f...
net/http/http_stream_factory_impl_job.cc:214: ShouldForceQuic(session,
destination, origin_url, proxy_info))
On 2017/07/06 19:00:04, Ryan Hamilton wrote:
> On 2017/07/06 18:13:39, Zhongyi Shi wrote:
> > On 2017/07/06 13:27:45, Ryan Hamilton wrote:
> > > Why does this only apply when "ShouldForceQuic" is true?
> > 
> > When we create the Jobs in JobController, we pass over the quic_version if
the
> > Job created is an alternative job and uses QUIC. When we create the main
job,
> we
> > always pass in a QUIC_VERSION_UNSUPPORTED.
> 
> (I guess an alternative would be to pass in supported_versions_[0] for the
Main
> job, to handle this case?
> 
This could work. However, it might also be confusing. A job is not using QUIC
but have a quic_version_ specified. Besides, JobController who creates the Job
could not designate a quic version for the Job when creating.
> > However, Job could also later figure out to use QUIC if the origin has
forced
> > QUIC on. That's why we want to change |quic_version_| to a valid version in
> this
> > case.
> 
> Ah! That makes sense. Can you add a comment to this effect? Also, I assume
that
> this DCHECK would be valid?
> 
> if (using_quic_) 
>   DCHECK(quic_version != QUIC_VERSION_UNSUPPORTED)

Done.

[commit-bot: I haz the power, 2017-07-06 21:19:13]

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

[commit-bot: I haz the power, 2017-07-06 21:19:15]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Zhongyi Shi, 2017-07-06 21:21:43]

The CQ bit was checked by zhongyi@chromium.org

[commit-bot: I haz the power, 2017-07-06 21:22:10]

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, 2017-07-06 21:56:41]

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

[commit-bot: I haz the power, 2017-07-06 21:56:42]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Zhongyi Shi, 2017-07-06 22:08:58]

The CQ bit was checked by zhongyi@chromium.org

[commit-bot: I haz the power, 2017-07-06 22:09:11]

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, 2017-07-06 23:36:42]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1499378938091480,
"parent_rev": "ef94a6e5a812838f0d96e8af9d45408f78a51a92", "commit_rev":
"052823c070ed0664da58020ad0a6fde96f081558"}

[commit-bot: I haz the power, 2017-07-06 23:37:02]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1499378938091480,
"parent_rev": "9d19e2e4529deb467950928a150b0f494047673f", "commit_rev":
"a00ca01914a13a1ff5e5fa5d24c746246dd89449"}

[commit-bot: I haz the power, 2017-07-06 23:37:16]

Description was changed from

==========
Change QuicStreamRequest::Request() to take a preferred QuicVersion so that 
the created QuicConnection will use the designated QuicVersion instead
of hardcoded QuicVersion in HttpNetworkSession::Param().quic_supported_versions.

BUG=689762
==========

to

==========
Change QuicStreamRequest::Request() to take a preferred QuicVersion so that 
the created QuicConnection will use the designated QuicVersion instead
of hardcoded QuicVersion in HttpNetworkSession::Param().quic_supported_versions.

BUG=689762

Review-Url: https://codereview.chromium.org/2958133002
Cr-Commit-Position: refs/heads/master@{#484769}
Committed:
https://chromium.googlesource.com/chromium/src/+/a00ca01914a13a1ff5e5fa5d24c7...
==========

[commit-bot: I haz the power, 2017-07-06 23:37:19]

Committed patchset #9 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/a00ca01914a13a1ff5e5fa5d24c7...