Make it possible to keep the port allocator session running

If gather_continually is set to true, keep the last port allocator session running while stopping all existing process of getting ports (when p2ptransportchannel first becomes writable).

BUG=5034
Committed: https://crrev.com/98db68fdaa12c5bfca8b0004eba24d034f32de71
Cr-Commit-Position: refs/heads/master@{#10110}

[honghaiz3, 2015-09-25 23:37:06]

Patchset #1 (id:1) has been deleted

[honghaiz3, 2015-09-25 23:37:11]

Patchset #1 (id:20001) has been deleted

[honghaiz3, 2015-09-25 23:37:17]

Patchset #1 (id:40001) has been deleted

[honghaiz3, 2015-09-25 23:37:23]

Patchset #1 (id:60001) has been deleted

[honghaiz3, 2015-09-25 23:42:28]

Patchset #1 (id:80001) has been deleted

[honghaiz3, 2015-09-25 23:53:33]

Patchset #1 (id:100001) has been deleted

[honghaiz3, 2015-09-25 23:54:07]

honghaiz@webrtc.org changed reviewers:
+ pthatcher@webrtc.org

[honghaiz3, 2015-09-25 23:54:07]

PTAL.

[pthatcher1, 2015-09-26 00:56:44]

https://codereview.webrtc.org/1359363003/diff/120001/webrtc/p2p/base/portallo...
File webrtc/p2p/base/portallocator.h (right):

https://codereview.webrtc.org/1359363003/diff/120001/webrtc/p2p/base/portallo...
webrtc/p2p/base/portallocator.h:96: virtual void StopGettingPorts(bool
keep_session_running) = 0;
I think we should have two methods:

1. StopGettingPorts  // Stop forever
2. ClearGettingPorts  // Stop existing sequenes

Stop == Clear + running_ = false + maybe signal "done"


"Clear" could use a better name, though.

[honghaiz3, 2015-09-28 21:57:14]

Patchset #2 (id:140001) has been deleted

[honghaiz3, 2015-09-28 22:37:11]

Patchset #2 (id:160001) has been deleted

[honghaiz3, 2015-09-28 22:43:30]

Thanks!
I added that in p2ptransportchannel, it will choose StopGettingPorts or
ClearGettingPorts based on the gather_continually policy.

[pthatcher1, 2015-09-28 23:34:00]

lgtm

Just little style stuff.

https://codereview.webrtc.org/1359363003/diff/180001/webrtc/p2p/base/p2ptrans...
File webrtc/p2p/base/p2ptransportchannel.cc (right):

https://codereview.webrtc.org/1359363003/diff/180001/webrtc/p2p/base/p2ptrans...
webrtc/p2p/base/p2ptransportchannel.cc:1073: if (!writable()) {
Can you make an early return here?

if (writable()) {
  return;
}

https://codereview.webrtc.org/1359363003/diff/180001/webrtc/p2p/base/p2ptrans...
webrtc/p2p/base/p2ptransportchannel.cc:1085: }
I think this would be more readable like this:
 
  for (PortAllocactorSession* session : allocator_sessions_) {
    if (!session->IsGettingPorts()) {
      continue;
    }
    if (gather_continually && session == allocator_sessions_.back()) {
      // If gathering continually, keep the last session running so that it
      // will gather candidates if the networks change.
      session->ClearGettingPorts();
      continue;
    }
    session->StopGettingPorts();
  }

[honghaiz3, 2015-09-29 00:04:49]

https://codereview.webrtc.org/1359363003/diff/180001/webrtc/p2p/base/p2ptrans...
File webrtc/p2p/base/p2ptransportchannel.cc (right):

https://codereview.webrtc.org/1359363003/diff/180001/webrtc/p2p/base/p2ptrans...
webrtc/p2p/base/p2ptransportchannel.cc:1073: if (!writable()) {
On 2015/09/28 23:34:00, pthatcher1 wrote:
> Can you make an early return here?
> 
> if (writable()) {
>   return;
> }

Done.

https://codereview.webrtc.org/1359363003/diff/180001/webrtc/p2p/base/p2ptrans...
webrtc/p2p/base/p2ptransportchannel.cc:1085: }
On 2015/09/28 23:34:00, pthatcher1 wrote:
> I think this would be more readable like this:
>  
>   for (PortAllocactorSession* session : allocator_sessions_) {
>     if (!session->IsGettingPorts()) {
>       continue;
>     }
>     if (gather_continually && session == allocator_sessions_.back()) {
>       // If gathering continually, keep the last session running so that it
>       // will gather candidates if the networks change.
>       session->ClearGettingPorts();
>       continue;
>     }
>     session->StopGettingPorts();
>   }
Done. Replaced continue with break because that is the last session.

[honghaiz3, 2015-09-29 00:05:51]

The CQ bit was checked by honghaiz@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-29 00:06:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1359363003/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1359363003/200001

[commit-bot: I haz the power, 2015-09-29 00:28:05]

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

[commit-bot: I haz the power, 2015-09-29 00:28:06]

Dry run: Try jobs failed on following builders:
  win_x64_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_rel/builds/9673)

[honghaiz3, 2015-09-29 00:48:37]

The CQ bit was checked by honghaiz@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-29 00:48:51]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1359363003/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1359363003/220001

[commit-bot: I haz the power, 2015-09-29 01:32:01]

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

[commit-bot: I haz the power, 2015-09-29 01:32:02]

Dry run: This issue passed the CQ dry run.

[honghaiz3, 2015-09-29 14:56:59]

https://codereview.webrtc.org/1359363003/diff/120001/webrtc/p2p/base/portallo...
File webrtc/p2p/base/portallocator.h (right):

https://codereview.webrtc.org/1359363003/diff/120001/webrtc/p2p/base/portallo...
webrtc/p2p/base/portallocator.h:96: virtual void StopGettingPorts(bool
keep_session_running) = 0;
On 2015/09/26 00:56:43, pthatcher1 wrote:
> I think we should have two methods:
> 
> 1. StopGettingPorts  // Stop forever
> 2. ClearGettingPorts  // Stop existing sequenes
> 
> Stop == Clear + running_ = false + maybe signal "done"
> 
> 
> "Clear" could use a better name, though.

Done.

[honghaiz3, 2015-09-29 14:57:12]

The CQ bit was checked by honghaiz@webrtc.org

[honghaiz3, 2015-09-29 14:57:13]

The patchset sent to the CQ was uploaded after l-g-t-m from pthatcher@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1359363003/#ps220001 (title:
" ")

[commit-bot: I haz the power, 2015-09-29 14:57:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1359363003/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1359363003/220001

[commit-bot: I haz the power, 2015-09-29 14:58:22]

Committed patchset #4 (id:220001)

[commit-bot: I haz the power, 2015-09-29 14:58:35]

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