Expose a spdy session's pooled aliases in the net-internals page.

Expose a spdy session's pooled aliases in the net-internals page.

BUG=106225
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=126668

[eroman, 2012-03-13 20:08:42]

https://chromiumcodereview.appspot.com/9696045/diff/1/chrome/browser/resource...
File chrome/browser/resources/net_internals/spdy_view.js (right):

https://chromiumcodereview.appspot.com/9696045/diff/1/chrome/browser/resource...
chrome/browser/resources/net_internals/spdy_view.js:163: host = host + ' ' +
session.aliases[j];
This can be simplified using string.join():

var host = session.host_port_pair;
if (session.aliases)
  host += ' ' + session.aliases.join(' ');

(I generally don't like nested counters i,j, since I have personally made many
mistakes in the past doing stuff like incrementing "i" when it should have been
"j", or testing "i" rather than "j" in the inner loop :-)

https://chromiumcodereview.appspot.com/9696045/diff/1/net/spdy/spdy_session.cc
File net/spdy/spdy_session.cc (right):

https://chromiumcodereview.appspot.com/9696045/diff/1/net/spdy/spdy_session.c...
net/spdy/spdy_session.cc:1140: dict->Set("aliases", alias_list);
I suggest only setting the aliases property if there are any aliases.

https://chromiumcodereview.appspot.com/9696045/diff/1/net/spdy/spdy_session_p...
File net/spdy/spdy_session_pool.cc (right):

https://chromiumcodereview.appspot.com/9696045/diff/1/net/spdy/spdy_session_p...
net/spdy/spdy_session_pool.cc:227: HostPortProxyPair key = it->first;
Please use |const HostPortProxyPair&| to avoid making a copy.

https://chromiumcodereview.appspot.com/9696045/diff/1/net/spdy/spdy_session_p...
net/spdy/spdy_session_pool.cc:228: HostPortProxyPair pair =
session->get()->host_port_proxy_pair();
ditto to above.

[Ryan Hamilton, 2012-03-13 21:55:37]

https://chromiumcodereview.appspot.com/9696045/diff/1/chrome/browser/resource...
File chrome/browser/resources/net_internals/spdy_view.js (right):

https://chromiumcodereview.appspot.com/9696045/diff/1/chrome/browser/resource...
chrome/browser/resources/net_internals/spdy_view.js:163: host = host + ' ' +
session.aliases[j];
On 2012/03/13 20:08:42, eroman wrote:
> This can be simplified using string.join():
> 
> var host = session.host_port_pair;
> if (session.aliases)
>   host += ' ' + session.aliases.join(' ');
> 
> (I generally don't like nested counters i,j, since I have personally made many
> mistakes in the past doing stuff like incrementing "i" when it should have
been
> "j", or testing "i" rather than "j" in the inner loop :-)

Done.  Totally agree with you, and as it happens, I made an i/j mistake when I
cooked up this cl :>

https://chromiumcodereview.appspot.com/9696045/diff/1/net/spdy/spdy_session.cc
File net/spdy/spdy_session.cc (right):

https://chromiumcodereview.appspot.com/9696045/diff/1/net/spdy/spdy_session.c...
net/spdy/spdy_session.cc:1140: dict->Set("aliases", alias_list);
On 2012/03/13 20:08:42, eroman wrote:
> I suggest only setting the aliases property if there are any aliases.

Done.  (I had thought we would prefer the property to always be there in .js
land.  Not sure why I thought this).

https://chromiumcodereview.appspot.com/9696045/diff/1/net/spdy/spdy_session_p...
File net/spdy/spdy_session_pool.cc (right):

https://chromiumcodereview.appspot.com/9696045/diff/1/net/spdy/spdy_session_p...
net/spdy/spdy_session_pool.cc:227: HostPortProxyPair key = it->first;
On 2012/03/13 20:08:42, eroman wrote:
> Please use |const HostPortProxyPair&| to avoid making a copy.

Done.

https://chromiumcodereview.appspot.com/9696045/diff/1/net/spdy/spdy_session_p...
net/spdy/spdy_session_pool.cc:228: HostPortProxyPair pair =
session->get()->host_port_proxy_pair();
On 2012/03/13 20:08:42, eroman wrote:
> ditto to above.

Done.

[eroman, 2012-03-13 23:15:58]

lgtm

https://chromiumcodereview.appspot.com/9696045/diff/8/chrome/browser/resource...
File chrome/browser/resources/net_internals/spdy_view.js (right):

https://chromiumcodereview.appspot.com/9696045/diff/8/chrome/browser/resource...
chrome/browser/resources/net_internals/spdy_view.js:161: if (session.aliases) {
minor-nit: I suggest removing the curlies, for consistency with this file.

[commit-bot: I haz the power, 2012-03-14 14:30:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/9696045/9001

[commit-bot: I haz the power, 2012-03-14 15:39:35]

Try job failure for 9696045-9001 (retry) (retry) on linux_rel for step
"remoting_unittests".
It's a second try, previously, step "remoting_unittests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-03-14 15:46:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/9696045/9001

[commit-bot: I haz the power, 2012-03-14 16:33:03]

Try job failure for 9696045-9001 (retry) on linux_rel for step
"remoting_unittests".
It's a second try, previously, step "remoting_unittests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...