Revert of Update ICE role on all ports, not just ones used for new connections.

Revert of Update ICE role on all ports, not just ones used for new connections. (patchset #3 id:40001 of https://codereview.webrtc.org/2053043003/ )

Reason for revert:
Speculative revert: breaks video quality tests on Win and Mac (???): https://build.chromium.org/p/chromium.webrtc.fyi/builders/Mac%20Tester/builds/31209

Original issue's description:
> Update ICE role on all ports, not just ones used for new connections.
>
> Previously, if the ICE role changed, SetIceRole was only called on
> the ports from the most recent ICE generation. However, STUN pings
> may still be sent and received by older generation ports, so they
> should receive an updated role as well.
>
> This was previously triggering an ASSERT, because a P2PTransportChannel
> expects the ICE role of each of its ports to match its own role.
>
> Committed: https://crrev.com/370544594e18deb7f560f961295c8cf3f0a679f1
> Cr-Commit-Position: refs/heads/master@{#13226}

TBR=pthatcher@webrtc.org,honghaiz@webrtc.org,deadbeef@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Committed: https://crrev.com/8d2248ff30a7d72c9224f73828e36e030742f6e3
Cr-Commit-Position: refs/heads/master@{#13240}

[phoglund, 2016-06-21 15:28:55]

The CQ bit was checked by phoglund@webrtc.org

[phoglund, 2016-06-21 15:28:56]

Created Revert of Update ICE role on all ports, not just ones used for new
connections.

[phoglund, 2016-06-21 15:29:02]

The CQ bit was unchecked by phoglund@webrtc.org

[phoglund, 2016-06-21 15:29:08]

The CQ bit was checked by phoglund@webrtc.org

[commit-bot: I haz the power, 2016-06-21 15:29:17]

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

[commit-bot: I haz the power, 2016-06-21 15:29:25]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-06-21 15:29:35]

Description was changed from

==========
Revert of Update ICE role on all ports, not just ones used for new connections.
(patchset #3 id:40001 of https://codereview.webrtc.org/2053043003/ )

Reason for revert:
Speculative revert: breaks video quality tests on Win and Mac (???):
https://build.chromium.org/p/chromium.webrtc.fyi/builders/Mac%20Tester/builds...

Original issue's description:
> Update ICE role on all ports, not just ones used for new connections.
> 
> Previously, if the ICE role changed, SetIceRole was only called on
> the ports from the most recent ICE generation. However, STUN pings
> may still be sent and received by older generation ports, so they
> should receive an updated role as well.
> 
> This was previously triggering an ASSERT, because a P2PTransportChannel
> expects the ICE role of each of its ports to match its own role.
> 
> Committed: https://crrev.com/370544594e18deb7f560f961295c8cf3f0a679f1
> Cr-Commit-Position: refs/heads/master@{#13226}

TBR=pthatcher@webrtc.org,honghaiz@webrtc.org,deadbeef@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
==========

to

==========
Revert of Update ICE role on all ports, not just ones used for new connections.
(patchset #3 id:40001 of https://codereview.webrtc.org/2053043003/ )

Reason for revert:
Speculative revert: breaks video quality tests on Win and Mac (???):
https://build.chromium.org/p/chromium.webrtc.fyi/builders/Mac%20Tester/builds...

Original issue's description:
> Update ICE role on all ports, not just ones used for new connections.
>
> Previously, if the ICE role changed, SetIceRole was only called on
> the ports from the most recent ICE generation. However, STUN pings
> may still be sent and received by older generation ports, so they
> should receive an updated role as well.
>
> This was previously triggering an ASSERT, because a P2PTransportChannel
> expects the ICE role of each of its ports to match its own role.
>
> Committed: https://crrev.com/370544594e18deb7f560f961295c8cf3f0a679f1
> Cr-Commit-Position: refs/heads/master@{#13226}

TBR=pthatcher@webrtc.org,honghaiz@webrtc.org,deadbeef@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Committed: https://crrev.com/8d2248ff30a7d72c9224f73828e36e030742f6e3
Cr-Commit-Position: refs/heads/master@{#13240}
==========

[commit-bot: I haz the power, 2016-06-21 15:29:36]

Patchset 1 (id:??) landed as
https://crrev.com/8d2248ff30a7d72c9224f73828e36e030742f6e3
Cr-Commit-Position: refs/heads/master@{#13240}

[phoglund, 2016-06-21 15:38:03]

On 2016/06/21 15:29:36, commit-bot: I haz the power wrote:
> Patchset 1 (id:??) landed as
> https://crrev.com/8d2248ff30a7d72c9224f73828e36e030742f6e3
> Cr-Commit-Position: refs/heads/master@{#13240}

This all being said, this is a strange error and I don't see the connection to
this patch. It's getting reverted because it happened to be in the blame list
when this started happening.

[Taylor Brandstetter, 2016-06-21 19:38:56]

On 2016/06/21 15:38:03, phoglund wrote:
> On 2016/06/21 15:29:36, commit-bot: I haz the power wrote:
> > Patchset 1 (id:??) landed as
> > https://crrev.com/8d2248ff30a7d72c9224f73828e36e030742f6e3
> > Cr-Commit-Position: refs/heads/master@{#13240}
> 
> This all being said, this is a strange error and I don't see the connection to
> this patch. It's getting reverted because it happened to be in the blame list
> when this started happening.

If this doesn't fix the failures, can you undo the revert? I don't think there's
any way this CL is related. It only makes a difference if the application is
doing an ICE restart, which the quality tests should not be doing.

[kjellander_webrtc, 2016-06-21 19:52:08]

On 2016/06/21 15:38:03, phoglund wrote:
> On 2016/06/21 15:29:36, commit-bot: I haz the power wrote:
> > Patchset 1 (id:??) landed as
> > https://crrev.com/8d2248ff30a7d72c9224f73828e36e030742f6e3
> > Cr-Commit-Position: refs/heads/master@{#13240}
> 
> This all being said, this is a strange error and I don't see the connection to
> this patch. It's getting reverted because it happened to be in the blame list
> when this started happening.

I can confirm that this revert indeed did fix the video quality tests in
browser_tests: 
https://build.chromium.org/p/chromium.webrtc.fyi/builders/Win7%20Tester/build...
https://build.chromium.org/p/chromium.webrtc.fyi/builders/Win10%20Tester/buil...

I tried to analyze the logs to see how much longer the individual tests took and
I'm unable to calculate any useful number out of the HD versions of the tests,
the ones that are failing:
WebRtcVideoQualityBrowserTests/WebRtcVideoQualityBrowserTest.MANUAL_TestVideoQualityH264/1

WebRtcVideoQualityBrowserTests/WebRtcVideoQualityBrowserTest.MANUAL_TestVideoQualityVp8/1
WebRtcVideoQualityBrowserTests/WebRtcVideoQualityBrowserTest.MANUAL_TestVideoQualityVp9/1

And when I look at the VGA versions (ending with /0), they take rougly the same
amount of time (i.e. not slower with this reverted patch)

[Taylor Brandstetter, 2016-06-21 20:05:05]

On 2016/06/21 19:52:08, kjellander_webrtc wrote:
> On 2016/06/21 15:38:03, phoglund wrote:
> > On 2016/06/21 15:29:36, commit-bot: I haz the power wrote:
> > > Patchset 1 (id:??) landed as
> > > https://crrev.com/8d2248ff30a7d72c9224f73828e36e030742f6e3
> > > Cr-Commit-Position: refs/heads/master@{#13240}
> > 
> > This all being said, this is a strange error and I don't see the connection
to
> > this patch. It's getting reverted because it happened to be in the blame
list
> > when this started happening.
> 
> I can confirm that this revert indeed did fix the video quality tests in
> browser_tests: 
>
https://build.chromium.org/p/chromium.webrtc.fyi/builders/Win7%20Tester/build...
>
https://build.chromium.org/p/chromium.webrtc.fyi/builders/Win10%20Tester/buil...
> 
> I tried to analyze the logs to see how much longer the individual tests took
and
> I'm unable to calculate any useful number out of the HD versions of the tests,
> the ones that are failing:
>
WebRtcVideoQualityBrowserTests/WebRtcVideoQualityBrowserTest.MANUAL_TestVideoQualityH264/1
> 
>
WebRtcVideoQualityBrowserTests/WebRtcVideoQualityBrowserTest.MANUAL_TestVideoQualityVp8/1
>
WebRtcVideoQualityBrowserTests/WebRtcVideoQualityBrowserTest.MANUAL_TestVideoQualityVp9/1
> 
> And when I look at the VGA versions (ending with /0), they take rougly the
same
> amount of time (i.e. not slower with this reverted patch)

Oh, I see what the issue is now. I was a really simple and dumb mistake; I can't
believe our unit tests didn't catch it.