QUIC - Collect performance stats for QUIC vs non-QUIC when google

QUIC - Collect performance stats for QUIC vs non-QUIC when google
properties are accessed.

R=rch@chromium.org, asvitkine@chromium.org
BUG=481618
Committed: https://crrev.com/72928bb9aadd5c99114ca18ad2518d2641c86447
Cr-Commit-Position: refs/heads/master@{#327335}

[ramant (doing other things), 2015-04-26 23:13:36]

Patchset #1 (id:1) has been deleted

[ramant (doing other things), 2015-04-26 23:17:01]

rtenneti@chromium.org changed reviewers:
+ asvitkine@chromium.org

[ramant (doing other things), 2015-04-26 23:23:19]

https://codereview.chromium.org/1096783005/diff/20001/net/base/net_util.cc
File net/base/net_util.cc (right):

https://codereview.chromium.org/1096783005/diff/20001/net/base/net_util.cc#ne...
net/base/net_util.cc:1038: "youtube.com",
This lists lot of google servers. What do you all think of this list.

[Alexei Svitkine (slow), 2015-04-27 19:23:55]

https://codereview.chromium.org/1096783005/diff/40001/net/base/net_util.cc
File net/base/net_util.cc (right):

https://codereview.chromium.org/1096783005/diff/40001/net/base/net_util.cc#ne...
net/base/net_util.cc:1057: host[host.size() - google_host_length - 1] == '.')) {
This logic is quite complicated. In other places we have simpler logic - for
example, here:

https://code.google.com/p/chromium/codesearch#chromium/src/components/variati...

Any reason the simpler logic is not sufficient here?

https://codereview.chromium.org/1096783005/diff/40001/net/base/net_util.h
File net/base/net_util.h (right):

https://codereview.chromium.org/1096783005/diff/40001/net/base/net_util.h#new...
net/base/net_util.h:446: // Returns true if the host is a google server.
Nit: google -> Google

Maybe expand comment to mention the intention of this? i.e. that it should only
be used for histograms and not to affect behavior?

https://codereview.chromium.org/1096783005/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1096783005/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:68698: +<histogram_suffixes
name="TotalTimeToGoogle">
You need to add separator=".".

[ramant (doing other things), 2015-04-28 00:30:25]

Thanks Alexei for the comments. PTAL.

https://codereview.chromium.org/1096783005/diff/40001/net/base/net_util.cc
File net/base/net_util.cc (right):

https://codereview.chromium.org/1096783005/diff/40001/net/base/net_util.cc#ne...
net/base/net_util.cc:1057: host[host.size() - google_host_length - 1] == '.')) {
On 2015/04/27 19:23:55, Alexei Svitkine wrote:
> This logic is quite complicated. In other places we have simpler logic - for
> example, here:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/components/variati...
> 
> Any reason the simpler logic is not sufficient here?

The above is accepting a host name which could be similar to 
https://code.google.com/p/chromium/codesearch#chromium/src/net/http/http_stre...

Changed the API call to accept GURL.

https://codereview.chromium.org/1096783005/diff/40001/net/base/net_util.h
File net/base/net_util.h (right):

https://codereview.chromium.org/1096783005/diff/40001/net/base/net_util.h#new...
net/base/net_util.h:446: // Returns true if the host is a google server.
On 2015/04/27 19:23:55, Alexei Svitkine wrote:
> Nit: google -> Google
> 
> Maybe expand comment to mention the intention of this? i.e. that it should
only
> be used for histograms and not to affect behavior?

Done.

https://codereview.chromium.org/1096783005/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1096783005/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:68698: +<histogram_suffixes
name="TotalTimeToGoogle">
On 2015/04/27 19:23:55, Alexei Svitkine wrote:
> You need to add separator=".".

Done.

[Ryan Hamilton, 2015-04-28 04:25:18]

lgtm, assuming asvitkine is happy.

https://codereview.chromium.org/1096783005/diff/60001/net/base/net_util.cc
File net/base/net_util.cc (right):

https://codereview.chromium.org/1096783005/diff/60001/net/base/net_util.cc#ne...
net/base/net_util.cc:1035: static const char* kSuffixesToGoogleHosts[] = {
nit: kGoogleHostSuffixes ?

https://codereview.chromium.org/1096783005/diff/60001/net/base/net_util.cc#ne...
net/base/net_util.cc:1050: for (size_t i = 0; i <
arraysize(kSuffixesToGoogleHosts); ++i) {
nit: I think you can do something like:
  for (const char* suffix : kGoogleHostSuffixes) {

[Alexei Svitkine (slow), 2015-04-28 15:10:31]

LGTM % Ryan's comments

Thanks!

[ramant (doing other things), 2015-04-28 17:19:16]

Thanks much Alexei and Ryan for the comments. Made all the changes suggested.

https://codereview.chromium.org/1096783005/diff/60001/net/base/net_util.cc
File net/base/net_util.cc (right):

https://codereview.chromium.org/1096783005/diff/60001/net/base/net_util.cc#ne...
net/base/net_util.cc:1035: static const char* kSuffixesToGoogleHosts[] = {
On 2015/04/28 04:25:18, Ryan Hamilton wrote:
> nit: kGoogleHostSuffixes ?

Done.

https://codereview.chromium.org/1096783005/diff/60001/net/base/net_util.cc#ne...
net/base/net_util.cc:1050: for (size_t i = 0; i <
arraysize(kSuffixesToGoogleHosts); ++i) {
On 2015/04/28 04:25:18, Ryan Hamilton wrote:
> nit: I think you can do something like:
>   for (const char* suffix : kGoogleHostSuffixes) {

Done.

[ramant (doing other things), 2015-04-28 17:19:32]

The CQ bit was checked by rtenneti@chromium.org

[ramant (doing other things), 2015-04-28 17:19:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org, rch@chromium.org
Link to the patchset: https://codereview.chromium.org/1096783005/#ps80001
(title: "Fix comments for PatchSet 3")

[commit-bot: I haz the power, 2015-04-28 17:19:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1096783005/80001

[commit-bot: I haz the power, 2015-04-28 18:28:26]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2015-04-28 18:29:42]

Patchset 4 (id:??) landed as
https://crrev.com/72928bb9aadd5c99114ca18ad2518d2641c86447
Cr-Commit-Position: refs/heads/master@{#327335}