Send SignalClose to the clients of P2PSocket, if TCP socket is failed to

Send SignalClose to the clients of P2PSocket, if TCP socket is failed to
establish connection with remote endpoint.

If we don't send signal, TCP based cricket::Port(s) think they are still
connecting.

BUG=https://code.google.com/p/webrtc/issues/detail?id=3249
R=sergeyu@chromium.org

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

[Sergey Ulanov, 2014-05-16 23:03:21]

https://codereview.chromium.org/288923014/diff/1/content/renderer/p2p/ipc_soc...
File content/renderer/p2p/ipc_socket_factory.cc (right):

https://codereview.chromium.org/288923014/diff/1/content/renderer/p2p/ipc_soc...
content/renderer/p2p/ipc_socket_factory.cc:477: if (state_ == IS_OPENING &&
IsTcpClientSocket(type_)) {
Do you need this condition? Maybe call SignalClose() for all sockets?
Also I think it's better to set state_ to IS_ERROR before calling SignalClose().

[Mallinath (Gone from Chromium), 2014-05-16 23:34:57]

https://codereview.chromium.org/288923014/diff/1/content/renderer/p2p/ipc_soc...
File content/renderer/p2p/ipc_socket_factory.cc (right):

https://codereview.chromium.org/288923014/diff/1/content/renderer/p2p/ipc_soc...
content/renderer/p2p/ipc_socket_factory.cc:477: if (state_ == IS_OPENING &&
IsTcpClientSocket(type_)) {
On 2014/05/16 23:03:21, Sergey Ulanov wrote:
> Do you need this condition? Maybe call SignalClose() for all sockets?
> Also I think it's better to set state_ to IS_ERROR before calling
SignalClose().

actually we don't need, but only TCP sockets are listening to it this signal.

[Sergey Ulanov, 2014-05-16 23:49:57]

https://codereview.chromium.org/288923014/diff/20001/content/renderer/p2p/ipc...
File content/renderer/p2p/ipc_socket_factory.cc (right):

https://codereview.chromium.org/288923014/diff/20001/content/renderer/p2p/ipc...
content/renderer/p2p/ipc_socket_factory.cc:477: if (state_ == IS_OPENING) {
Should we also SignalClose() in the IS_OPEN state?

https://codereview.chromium.org/288923014/diff/20001/content/renderer/p2p/ipc...
content/renderer/p2p/ipc_socket_factory.cc:478: state_ = IS_ERROR;
this duplicates line 481. Maybe something like this:

  bool was_closed = state_ == IS_ERROR || state_ == IS_CLOSED;
  state_ = IS_ERROR;
  error_ = ECONNABORTED;
  if (!was_closed)
    SignalClose(this).

[Mallinath (Gone from Chromium), 2014-05-16 23:58:46]

https://codereview.chromium.org/288923014/diff/20001/content/renderer/p2p/ipc...
File content/renderer/p2p/ipc_socket_factory.cc (right):

https://codereview.chromium.org/288923014/diff/20001/content/renderer/p2p/ipc...
content/renderer/p2p/ipc_socket_factory.cc:477: if (state_ == IS_OPENING) {
On 2014/05/16 23:49:57, Sergey Ulanov wrote:
> Should we also SignalClose() in the IS_OPEN state?

Done.

https://codereview.chromium.org/288923014/diff/20001/content/renderer/p2p/ipc...
content/renderer/p2p/ipc_socket_factory.cc:477: if (state_ == IS_OPENING) {
On 2014/05/16 23:49:57, Sergey Ulanov wrote:
> Should we also SignalClose() in the IS_OPEN state?

Done.

https://codereview.chromium.org/288923014/diff/20001/content/renderer/p2p/ipc...
content/renderer/p2p/ipc_socket_factory.cc:478: state_ = IS_ERROR;
On 2014/05/16 23:49:57, Sergey Ulanov wrote:
> this duplicates line 481. Maybe something like this:
> 
>   bool was_closed = state_ == IS_ERROR || state_ == IS_CLOSED;
>   state_ = IS_ERROR;
>   error_ = ECONNABORTED;
>   if (!was_closed)
>     SignalClose(this).

Done.

[Sergey Ulanov, 2014-05-17 00:00:25]

lgtm

[Mallinath (Gone from Chromium), 2014-05-17 00:08:44]

The CQ bit was checked by mallinath@chromium.org

[commit-bot: I haz the power, 2014-05-17 00:09:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mallinath@chromium.org/288923014/40001

[Mallinath (Gone from Chromium), 2014-05-17 01:27:06]

Committed patchset #3 manually as r271138 (presubmit successful).