Simplify PortAllocatorBase and make PortAllocator creation synchronous.

Simplify PortAllocatorBase and make PortAllocator creation synchronous.

Previously PortAllocator instances were created asynchronously in
TransportContext. Making PortAllocator synchronous allows to simplify
logic in other places, particularly in WebrtcTransport. Also simplified
PortAllocatorBase - it no longer stores STUN and Relay information and
instead instances of PortAllocatorSessionBase call TransportContext
to request fresh information when allocating ports.

Also removed duplicate logic from PepperPortAllocator and
ChromiumPortAllocator. Specifically they were filtering TCP-based relay
configs, but that's not longer necessary because PortAllocatorBase
is responsible for that now.

BUG=547158
Committed: https://crrev.com/33312d6d01ec88fb6e686f03c886e2448ee238bd
Cr-Commit-Position: refs/heads/master@{#368701}

[Sergey Ulanov, 2016-01-08 23:19:57]

Description was changed from

==========
Simplify PortAllocatorBase and make PortAllocator creation synchronous.

Previously PortAllocator instances were created asynchronously in
TransportContext. Making PortAllocator synchronous allows to simplify
logic in other places, particularly in WebrtcTransport. Also simplified
PortAllocatorBase - it no longer stores STUN and Relay information and
instead instances of PortAllocatorSessionBase call TransportContext
to request fresh information when allocating ports.

BUG=547158
==========

to

==========
Simplify PortAllocatorBase and make PortAllocator creation synchronous.

Previously PortAllocator instances were created asynchronously in
TransportContext. Making PortAllocator synchronous allows to simplify
logic in other places, particularly in WebrtcTransport. Also simplified
PortAllocatorBase - it no longer stores STUN and Relay information and
instead instances of PortAllocatorSessionBase call TransportContext
to request fresh information when allocating ports.

Also removed duplicate logic from PepperPortAllocator and 
ChromiumPortAllocator. Specifically they were filtering TCP-based relay
configs, but that's not longer necessary because PortAllocatorBase
is responsible for that now.

BUG=547158
==========

[Sergey Ulanov, 2016-01-08 23:25:12]

sergeyu@chromium.org changed reviewers:
+ jamiewalch@chromium.org

[Jamie, 2016-01-09 00:12:08]

lgtm

https://codereview.chromium.org/1571943002/diff/20001/remoting/client/plugin/...
File remoting/client/plugin/pepper_port_allocator.cc (right):

https://codereview.chromium.org/1571943002/diff/20001/remoting/client/plugin/...
remoting/client/plugin/pepper_port_allocator.cc:49:
BasicPortAllocatorSession::allocator());
Optional: I'm not a big fan of this cast either, though at least there's an
inheritance relation that implies a somewhat tight coupling between the two
classes. Personally, I'd probably extract the pp::InstanceHandle explicitly in
the ctor and save it, but perhaps that's overkill.

I also question why allocator() needs to be virtual in the base class (it's the
only one of the inline getters that is) but that's out of scope for this CL.

https://codereview.chromium.org/1571943002/diff/20001/remoting/protocol/chrom...
File remoting/protocol/chromium_port_allocator.cc (right):

https://codereview.chromium.org/1571943002/diff/20001/remoting/protocol/chrom...
remoting/protocol/chromium_port_allocator.cc:44: return
static_cast<ChromiumPortAllocator*>(
Similarly, consider saving the URL context in the ctor.

[Sergey Ulanov, 2016-01-11 20:01:04]

https://codereview.chromium.org/1571943002/diff/20001/remoting/client/plugin/...
File remoting/client/plugin/pepper_port_allocator.cc (right):

https://codereview.chromium.org/1571943002/diff/20001/remoting/client/plugin/...
remoting/client/plugin/pepper_port_allocator.cc:49:
BasicPortAllocatorSession::allocator());
On 2016/01/09 00:12:07, Jamie wrote:
> Optional: I'm not a big fan of this cast either, though at least there's an
> inheritance relation that implies a somewhat tight coupling between the two
> classes. Personally, I'd probably extract the pp::InstanceHandle explicitly in
> the ctor and save it, but perhaps that's overkill.
> 
> I also question why allocator() needs to be virtual in the base class (it's
the
> only one of the inline getters that is) but that's out of scope for this CL.

Done.

https://codereview.chromium.org/1571943002/diff/20001/remoting/protocol/chrom...
File remoting/protocol/chromium_port_allocator.cc (right):

https://codereview.chromium.org/1571943002/diff/20001/remoting/protocol/chrom...
remoting/protocol/chromium_port_allocator.cc:44: return
static_cast<ChromiumPortAllocator*>(
On 2016/01/09 00:12:07, Jamie wrote:
> Similarly, consider saving the URL context in the ctor.

Done. Also removed allocator() override from PortAllocatorBase.

[Sergey Ulanov, 2016-01-11 20:01:55]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-01-11 20:01:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/1571943002/#ps40001
(title: " ")

[commit-bot: I haz the power, 2016-01-11 20:04:29]

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

[commit-bot: I haz the power, 2016-01-11 20:31:24]

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

[commit-bot: I haz the power, 2016-01-11 20:31:25]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[Sergey Ulanov, 2016-01-11 21:54:29]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-01-11 21:54:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/1571943002/#ps60001
(title: " ")

[commit-bot: I haz the power, 2016-01-11 21:56:12]

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

[commit-bot: I haz the power, 2016-01-11 23:07:49]

Description was changed from

==========
Simplify PortAllocatorBase and make PortAllocator creation synchronous.

Previously PortAllocator instances were created asynchronously in
TransportContext. Making PortAllocator synchronous allows to simplify
logic in other places, particularly in WebrtcTransport. Also simplified
PortAllocatorBase - it no longer stores STUN and Relay information and
instead instances of PortAllocatorSessionBase call TransportContext
to request fresh information when allocating ports.

Also removed duplicate logic from PepperPortAllocator and 
ChromiumPortAllocator. Specifically they were filtering TCP-based relay
configs, but that's not longer necessary because PortAllocatorBase
is responsible for that now.

BUG=547158
==========

to

==========
Simplify PortAllocatorBase and make PortAllocator creation synchronous.

Previously PortAllocator instances were created asynchronously in
TransportContext. Making PortAllocator synchronous allows to simplify
logic in other places, particularly in WebrtcTransport. Also simplified
PortAllocatorBase - it no longer stores STUN and Relay information and
instead instances of PortAllocatorSessionBase call TransportContext
to request fresh information when allocating ports.

Also removed duplicate logic from PepperPortAllocator and 
ChromiumPortAllocator. Specifically they were filtering TCP-based relay
configs, but that's not longer necessary because PortAllocatorBase
is responsible for that now.

BUG=547158
==========

[commit-bot: I haz the power, 2016-01-11 23:07:49]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-01-11 23:08:51]

Description was changed from

==========
Simplify PortAllocatorBase and make PortAllocator creation synchronous.

Previously PortAllocator instances were created asynchronously in
TransportContext. Making PortAllocator synchronous allows to simplify
logic in other places, particularly in WebrtcTransport. Also simplified
PortAllocatorBase - it no longer stores STUN and Relay information and
instead instances of PortAllocatorSessionBase call TransportContext
to request fresh information when allocating ports.

Also removed duplicate logic from PepperPortAllocator and 
ChromiumPortAllocator. Specifically they were filtering TCP-based relay
configs, but that's not longer necessary because PortAllocatorBase
is responsible for that now.

BUG=547158
==========

to

==========
Simplify PortAllocatorBase and make PortAllocator creation synchronous.

Previously PortAllocator instances were created asynchronously in
TransportContext. Making PortAllocator synchronous allows to simplify
logic in other places, particularly in WebrtcTransport. Also simplified
PortAllocatorBase - it no longer stores STUN and Relay information and
instead instances of PortAllocatorSessionBase call TransportContext
to request fresh information when allocating ports.

Also removed duplicate logic from PepperPortAllocator and
ChromiumPortAllocator. Specifically they were filtering TCP-based relay
configs, but that's not longer necessary because PortAllocatorBase
is responsible for that now.

BUG=547158

Committed: https://crrev.com/33312d6d01ec88fb6e686f03c886e2448ee238bd
Cr-Commit-Position: refs/heads/master@{#368701}
==========

[commit-bot: I haz the power, 2016-01-11 23:08:51]

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