Roll libjingle to r124 and roll webrtc to 1888.

Roll libjingle to r124. Removed overrides for the ws2 fixes.
Roll webrtc to 1888.

BUG=115702, 117252
TEST=apprtc.appspot.com on windows and linux
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=126793

[Ronghua Wu (Left Chromium), 2012-03-07 01:22:38]

I'm making a new libjingle release (r121) with one extra fix and some missing
files. But this cl should be the same except the 120 in DEPS will be updated to
121.

[Sergey Ulanov, 2012-03-07 01:55:44]

+wez
Wez, I suppose that now we don't have those ugly defines in win32.h it will be
safe to revert crrev.com/125110. Can you please confirm it?

[Wez, 2012-03-07 01:57:38]

On 2012/03/07 01:55:44, sergeyu wrote:
> +wez
> Wez, I suppose that now we don't have those ugly defines in win32.h it will be
> safe to revert crrev.com/125110. Can you please confirm it?

Not quite; we still need crrev.com/125110 in place for now.

Once the libjingle roll lands I'll replace crrev.com/125110 with a cleaner
patch.

[Sergey Ulanov, 2012-03-07 01:58:31]

lgtm

[Ronghua Wu (Left Chromium), 2012-03-07 19:06:23]

Updated the DEPS to 121.

[commit-bot: I haz the power, 2012-03-07 19:48:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ronghuawu@chromium.org/9600066/8007

[commit-bot: I haz the power, 2012-03-07 20:19:27]

Try job failure for 9600066-8007 (retry) on win_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[Ronghua Wu (Left Chromium), 2012-03-07 22:53:41]

The try jobs found a static variable in network.h:166. Will need to fix that in
libjingle first before we can roll this. Looks like it's going to take a bit
longer.

http://code.google.com/p/libjingle/source/browse/trunk/talk/base/network.h

[Ronghua Wu (Left Chromium), 2012-03-09 17:24:00]

Roll to libjingle r122, which removed one global non pod. Overrides to the
thread and messagequeue remain as discussed.

[Ronghua Wu (Left Chromium), 2012-03-09 22:23:01]

Sergey is OOO today. Need to wait for his LG on the new patch set.

[Wez, 2012-03-09 22:51:52]

LGTM, with nits, although you'll still need an OWNER for the content/renderer/
changes.  I'm assuming that the still-overridden files (e.g. messagequeue.h and
threading.h) haven't changed in libjingle, so the overrides don't need updating
either.

Rietvald seems confused by the byteorder.h changes - has the file changed or
been removed?

http://codereview.chromium.org/9600066/diff/39003/content/renderer/p2p/ipc_ne...
File content/renderer/p2p/ipc_network_manager.cc (right):

http://codereview.chromium.org/9600066/diff/39003/content/renderer/p2p/ipc_ne...
content/renderer/p2p/ipc_network_manager.cc:57: it->name, it->name,
talk_base::IPAddress(address), 32);
nit: This 32 is really net::kIPv4AddressSize * 8, I think?

http://codereview.chromium.org/9600066/diff/39003/jingle/glue/fake_network_ma...
File jingle/glue/fake_network_manager.cc (right):

http://codereview.chromium.org/9600066/diff/39003/jingle/glue/fake_network_ma...
jingle/glue/fake_network_manager.cc:20: talk_base::SocketAddress socket_address;
nit: Can't we SocketAddress socket_address(IPAddress(0), 0) here and avoid the
IPEndPoint definition & conversion?

[Ronghua Wu (Left Chromium), 2012-03-09 23:28:47]

Yea, the byteorder.h should be a M.

http://codereview.chromium.org/9600066/diff/39003/content/renderer/p2p/ipc_ne...
File content/renderer/p2p/ipc_network_manager.cc (right):

http://codereview.chromium.org/9600066/diff/39003/content/renderer/p2p/ipc_ne...
content/renderer/p2p/ipc_network_manager.cc:57: it->name, it->name,
talk_base::IPAddress(address), 32);
On 2012/03/09 22:51:52, Wez wrote:
> nit: This 32 is really net::kIPv4AddressSize * 8, I think?

No, the last two parameters should be the network prefix and prefix length. But
we don't have that information here. So I looked into the network.cc, the prefix
and prefix length are just used to identify the network, so I put the actually
ip address and 32 here as the prefix and prefix length. hmm, maybe I should
double check with the author of the network.cc.

http://codereview.chromium.org/9600066/diff/39003/jingle/glue/fake_network_ma...
File jingle/glue/fake_network_manager.cc (right):

http://codereview.chromium.org/9600066/diff/39003/jingle/glue/fake_network_ma...
jingle/glue/fake_network_manager.cc:20: talk_base::SocketAddress socket_address;
On 2012/03/09 22:51:52, Wez wrote:
> nit: Can't we SocketAddress socket_address(IPAddress(0), 0) here and avoid the
> IPEndPoint definition & conversion?

Then we didn't use the input argument |address|, correct?

[Ronghua Wu (Left Chromium), 2012-03-10 02:42:04]

+djw to review the changes to ipc_network_manager.cc and fake_network_manager.cc

Hi Darren&Sergey,

Wez brought my attention to the new Network interface change. Now the Network
ctor takes the network prefix and prefix length as the parameter. Looking into
the network.cc, it seems the prefix and prefix length are just used to identify
a network interface. Because the net::NetworkInterfaceList doesn't have the
information about network prefix (see in ipc_network_manager.cc), so I used the
ip address as the prefix and 32 has the prefix length.

I think the real solution should be adding network prefix information to the
net::NetworkInterface. But do you think we can also use it the way this patch
does? From my understanding this patch should get the same behavior as it was
before, correct?

[djw_chromium.org, 2012-03-12 03:20:43]

Hi Ronghua,
Yes, we're now identifying networks by a pair of interface name +
network prefix, and we group together IPs that are inside the same
network prefix, but only if they're on the same interface.
Since they have to be on the same interface to get grouped, and as far
as I know, basically everyone uses IPv4 with only one address per
interface, your scheme should be fine.
You'll probably want to move to getting prefix information when you
start looking at IPv6 compatibility though - multiple addresses with
the same prefix on one interface is actually a pretty common scenario
in IPv6 (temporary/privacy addresses).

On Sat, Mar 10, 2012 at 11:42,  <ronghuawu@chromium.org> wrote:
> +djw to review the changes to ipc_network_manager.cc and
> fake_network_manager.cc
>
> Hi Darren&Sergey,
>
> Wez brought my attention to the new Network interface change. Now the
> Network
> ctor takes the network prefix and prefix length as the parameter. Looking
> into
> the network.cc, it seems the prefix and prefix length are just used to
> identify
> a network interface. Because the net::NetworkInterfaceList doesn't have the
> information about network prefix (see in ipc_network_manager.cc), so I used
> the
> ip address as the prefix and 32 has the prefix length.
>
> I think the real solution should be adding network prefix information to the
> net::NetworkInterface. But do you think we can also use it the way this
> patch
> does? From my understanding this patch should get the same behavior as it
> was
> before, correct?
>
> http://codereview.chromium.org/9600066/

[Ronghua Wu (Left Chromium), 2012-03-12 05:36:36]

Thanks Darren.

Hi Sergey,

Do we need to consider the IPv6 compatibility in this cl? Or you may add
the prefix information to the NetworkInterfaceList later?


On Sun, Mar 11, 2012 at 8:20 PM, Darren Willis <djw@chromium.org> wrote:

> Hi Ronghua,
> Yes, we're now identifying networks by a pair of interface name +
> network prefix, and we group together IPs that are inside the same
> network prefix, but only if they're on the same interface.
> Since they have to be on the same interface to get grouped, and as far
> as I know, basically everyone uses IPv4 with only one address per
> interface, your scheme should be fine.
> You'll probably want to move to getting prefix information when you
> start looking at IPv6 compatibility though - multiple addresses with
> the same prefix on one interface is actually a pretty common scenario
> in IPv6 (temporary/privacy addresses).
>
> On Sat, Mar 10, 2012 at 11:42,  <ronghuawu@chromium.org> wrote:
> > +djw to review the changes to ipc_network_manager.cc and
> > fake_network_manager.cc
> >
> > Hi Darren&Sergey,
> >
> > Wez brought my attention to the new Network interface change. Now the
> > Network
> > ctor takes the network prefix and prefix length as the parameter. Looking
> > into
> > the network.cc, it seems the prefix and prefix length are just used to
> > identify
> > a network interface. Because the net::NetworkInterfaceList doesn't have
> the
> > information about network prefix (see in ipc_network_manager.cc), so I
> used
> > the
> > ip address as the prefix and 32 has the prefix length.
> >
> > I think the real solution should be adding network prefix information to
> the
> > net::NetworkInterface. But do you think we can also use it the way this
> > patch
> > does? From my understanding this patch should get the same behavior as it
> > was
> > before, correct?
> >
> > http://codereview.chromium.org/9600066/
>

[Sergey Ulanov, 2012-03-12 17:54:41]

On 2012/03/12 05:36:36, Ronghua Wu wrote:
> Thanks Darren.
> 
> Hi Sergey,
> 
> Do we need to consider the IPv6 compatibility in this cl? Or you may add
> the prefix information to the NetworkInterfaceList later?

No. IpcNetworkManager currently doesn't support IPv4 for other reasons, so it's
fine to keep it as is. We can add IPv6 support later.

> 
> 
> On Sun, Mar 11, 2012 at 8:20 PM, Darren Willis <mailto:djw@chromium.org>
wrote:
> 
> > Hi Ronghua,
> > Yes, we're now identifying networks by a pair of interface name +
> > network prefix, and we group together IPs that are inside the same
> > network prefix, but only if they're on the same interface.
> > Since they have to be on the same interface to get grouped, and as far
> > as I know, basically everyone uses IPv4 with only one address per
> > interface, your scheme should be fine.
> > You'll probably want to move to getting prefix information when you
> > start looking at IPv6 compatibility though - multiple addresses with
> > the same prefix on one interface is actually a pretty common scenario
> > in IPv6 (temporary/privacy addresses).
> >
> > On Sat, Mar 10, 2012 at 11:42,  <mailto:ronghuawu@chromium.org> wrote:
> > > +djw to review the changes to ipc_network_manager.cc and
> > > fake_network_manager.cc
> > >
> > > Hi Darren&Sergey,
> > >
> > > Wez brought my attention to the new Network interface change. Now the
> > > Network
> > > ctor takes the network prefix and prefix length as the parameter. Looking
> > > into
> > > the network.cc, it seems the prefix and prefix length are just used to
> > > identify
> > > a network interface. Because the net::NetworkInterfaceList doesn't have
> > the
> > > information about network prefix (see in ipc_network_manager.cc), so I
> > used
> > > the
> > > ip address as the prefix and 32 has the prefix length.
> > >
> > > I think the real solution should be adding network prefix information to
> > the
> > > net::NetworkInterface. But do you think we can also use it the way this
> > > patch
> > > does? From my understanding this patch should get the same behavior as it
> > > was
> > > before, correct?
> > >
> > > http://codereview.chromium.org/9600066/
> >

[commit-bot: I haz the power, 2012-03-12 17:56:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ronghuawu@chromium.org/9600066/39003

[commit-bot: I haz the power, 2012-03-12 18:25:19]

The commit queue went berserk retrying too often for a
seemingly flaky test. Builder is android, revision is 126171, job name
was 9600066-39003 (retry) (retry) (retry) (retry).

[Ronghua Wu (Left Chromium), 2012-03-14 21:14:34]

Updated to libjingle r124 with the fixes for android build.

Update the webrtc to r1888 which is the version that works with the new
libjingle.

Patch itself remained unchanged.

If no objection, I will try to commit again.

[commit-bot: I haz the power, 2012-03-14 21:25:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ronghuawu@chromium.org/9600066/60002

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

Try job failure for 9600066-60002 (retry) on mac_rel for step "update".
It's a second try, previously, steps "ui_tests, browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

Step "update" is always a major failure.
Look at the try server FAQ for more details.

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

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ronghuawu@chromium.org/9600066/64018

[Ronghua Wu (Left Chromium), 2012-03-15 15:31:04]

On 2012/03/14 23:28:54, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
> https://chromium-status.appspot.com/cq/ronghuawu%40chromium.org/9600066/64018

FYI, this is committed as r126793.