Refactor WebSocketTransportClientSocketPool's socket handing out code

Refactor WebSocketTransportClientSocketPool's socket handing out code

Tries to make the code a little more readable to help debugging the
associated bug.

For the OK result, BoundNetLog::EndEventWithNetErrorCode() was called
after BoundNetLog::EndEvent() in
WebSocketTransportClientSocketPool::RequestSocket(). Fixed this to call
only EndEvent().

Removed pre-condition checks before ActivateStalledRequest() which are
already performed by its while loop.

Added DCHECK_NE(error, OK) to FlushWithError() to make sure OK is never
received from that path.

Reordered some lines.

R=yhirano@chromium.org,mmenke@chromium.org
BUG=642363
Committed: https://crrev.com/913f8b743b1922bd332f01d7456ad688b80463cf
Cr-Commit-Position: refs/heads/master@{#419127}

[tyoshino (SeeGerritForStatus), 2016-09-08 08:08:09]

Description was changed from

==========
Refactor WebSocketTransportClientSocketPool's socket handing out code

Tries to make the code a little more readable to help debugging the
associated bug.

BUG=642363
==========

to

==========
Refactor WebSocketTransportClientSocketPool's socket handing out code

Tries to make the code a little more readable to help debugging the
associated bug.

For the OK result, BoundNetLog::EndEventWithNetErrorCode() was called
after BoundNetLog::EndEvent() in
WebSocketTransportClientSocketPool::RequestSocket(). Fixed this to call
only EndEvent().

Removed pre-condition checks before ActivateStalledRequest() which are
already performed by its while loop.

Added DCHECK_NE(error, OK) to FlushWithError() to make sure OK is never
received from that path.

Reordered some lines.

BUG=642363
==========

[tyoshino (SeeGerritForStatus), 2016-09-08 08:09:35]

Description was changed from

==========
Refactor WebSocketTransportClientSocketPool's socket handing out code

Tries to make the code a little more readable to help debugging the
associated bug.

For the OK result, BoundNetLog::EndEventWithNetErrorCode() was called
after BoundNetLog::EndEvent() in
WebSocketTransportClientSocketPool::RequestSocket(). Fixed this to call
only EndEvent().

Removed pre-condition checks before ActivateStalledRequest() which are
already performed by its while loop.

Added DCHECK_NE(error, OK) to FlushWithError() to make sure OK is never
received from that path.

Reordered some lines.

BUG=642363
==========

to

==========
Refactor WebSocketTransportClientSocketPool's socket handing out code

Tries to make the code a little more readable to help debugging the
associated bug.

For the OK result, BoundNetLog::EndEventWithNetErrorCode() was called
after BoundNetLog::EndEvent() in
WebSocketTransportClientSocketPool::RequestSocket(). Fixed this to call
only EndEvent().

Removed pre-condition checks before ActivateStalledRequest() which are
already performed by its while loop.

Added DCHECK_NE(error, OK) to FlushWithError() to make sure OK is never
received from that path.

Reordered some lines.

BUG=642363
==========

[tyoshino (SeeGerritForStatus), 2016-09-08 08:09:36]

tyoshino@chromium.org changed reviewers:
+ yhirano@chromium.org

[yhirano, 2016-09-12 10:29:33]

lgtm

https://codereview.chromium.org/2328453002/diff/1/net/socket/websocket_transp...
File net/socket/websocket_transport_client_socket_pool.cc (right):

https://codereview.chromium.org/2328453002/diff/1/net/socket/websocket_transp...
net/socket/websocket_transport_client_socket_pool.cc:505: DCHECK(socket.get());
.get() is not needed.

https://codereview.chromium.org/2328453002/diff/1/net/socket/websocket_transp...
net/socket/websocket_transport_client_socket_pool.cc:519: if (socket.get()) {
.get() is not needed.

[tyoshino (SeeGerritForStatus), 2016-09-13 02:20:24]

tyoshino@chromium.org changed reviewers:
+ mmenke@chromium.org

[tyoshino (SeeGerritForStatus), 2016-09-13 02:21:02]

+mmenke

https://codereview.chromium.org/2328453002/diff/1/net/socket/websocket_transp...
File net/socket/websocket_transport_client_socket_pool.cc (right):

https://codereview.chromium.org/2328453002/diff/1/net/socket/websocket_transp...
net/socket/websocket_transport_client_socket_pool.cc:505: DCHECK(socket.get());
On 2016/09/12 at 10:29:33, yhirano (slow) wrote:
> .get() is not needed.

Done

https://codereview.chromium.org/2328453002/diff/1/net/socket/websocket_transp...
net/socket/websocket_transport_client_socket_pool.cc:519: if (socket.get()) {
On 2016/09/12 at 10:29:33, yhirano (slow) wrote:
> .get() is not needed.

Done

[mmenke, 2016-09-13 14:38:28]

LGTM, rubberstamp.  If you guys want to add per-file owners entries for these
files that delegates them to the owners in net/websocket/OWNERS, I'm happy to
sign off on it.  Also happy to just rubberstamp changes to this file.

[tyoshino (SeeGerritForStatus), 2016-09-14 07:12:35]

Description was changed from

==========
Refactor WebSocketTransportClientSocketPool's socket handing out code

Tries to make the code a little more readable to help debugging the
associated bug.

For the OK result, BoundNetLog::EndEventWithNetErrorCode() was called
after BoundNetLog::EndEvent() in
WebSocketTransportClientSocketPool::RequestSocket(). Fixed this to call
only EndEvent().

Removed pre-condition checks before ActivateStalledRequest() which are
already performed by its while loop.

Added DCHECK_NE(error, OK) to FlushWithError() to make sure OK is never
received from that path.

Reordered some lines.

BUG=642363
==========

to

==========
Refactor WebSocketTransportClientSocketPool's socket handing out code

Tries to make the code a little more readable to help debugging the
associated bug.

For the OK result, BoundNetLog::EndEventWithNetErrorCode() was called
after BoundNetLog::EndEvent() in
WebSocketTransportClientSocketPool::RequestSocket(). Fixed this to call
only EndEvent().

Removed pre-condition checks before ActivateStalledRequest() which are
already performed by its while loop.

Added DCHECK_NE(error, OK) to FlushWithError() to make sure OK is never
received from that path.

Reordered some lines.

R=yhirano@chromium.org,mmenke@chromium.org
BUG=642363
==========

[tyoshino (SeeGerritForStatus), 2016-09-14 07:14:59]

The CQ bit was checked by tyoshino@chromium.org

[tyoshino (SeeGerritForStatus), 2016-09-14 07:15:00]

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

[commit-bot: I haz the power, 2016-09-14 07:15:17]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tyoshino (SeeGerritForStatus), 2016-09-14 07:39:13]

On 2016/09/13 14:38:28, mmenke wrote:
> LGTM, rubberstamp.  If you guys want to add per-file owners entries for these
> files that delegates them to the owners in net/websocket/OWNERS, I'm happy to
> sign off on it.  Also happy to just rubberstamp changes to this file.

Thanks. Created https://codereview.chromium.org/2335133005/

[commit-bot: I haz the power, 2016-09-14 09:46:44]

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

[commit-bot: I haz the power, 2016-09-14 09:46:45]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tyoshino (SeeGerritForStatus), 2016-09-14 12:29:37]

The CQ bit was checked by tyoshino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-14 12:29:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-14 13:31:12]

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

[commit-bot: I haz the power, 2016-09-14 13:31:13]

Dry run: This issue passed the CQ dry run.

[tyoshino (SeeGerritForStatus), 2016-09-14 14:11:21]

Hi yhirano,

I've fixed ASAN build failure in PS5. Sorry, but please take a look.

[yhirano, 2016-09-16 03:41:04]

lgtm

https://codereview.chromium.org/2328453002/diff/80001/net/socket/websocket_tr...
File net/socket/websocket_transport_client_socket_pool.cc (right):

https://codereview.chromium.org/2328453002/diff/80001/net/socket/websocket_tr...
net/socket/websocket_transport_client_socket_pool.cc:554: job = NULL;
[optional] nullptr

[tyoshino (SeeGerritForStatus), 2016-09-16 06:50:01]

The CQ bit was checked by tyoshino@chromium.org

[tyoshino (SeeGerritForStatus), 2016-09-16 06:50:01]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org,
yhirano@chromium.org
Link to the patchset: https://codereview.chromium.org/2328453002/#ps100001
(title: "Rebase")

[commit-bot: I haz the power, 2016-09-16 06:50:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tyoshino (SeeGerritForStatus), 2016-09-16 06:55:12]

https://codereview.chromium.org/2328453002/diff/80001/net/socket/websocket_tr...
File net/socket/websocket_transport_client_socket_pool.cc (right):

https://codereview.chromium.org/2328453002/diff/80001/net/socket/websocket_tr...
net/socket/websocket_transport_client_socket_pool.cc:554: job = NULL;
On 2016/09/16 03:41:04, yhirano (slow) wrote:
> [optional] nullptr

Created another CL
https://codereview.chromium.org/2341233002/

[commit-bot: I haz the power, 2016-09-16 07:52:58]

Description was changed from

==========
Refactor WebSocketTransportClientSocketPool's socket handing out code

Tries to make the code a little more readable to help debugging the
associated bug.

For the OK result, BoundNetLog::EndEventWithNetErrorCode() was called
after BoundNetLog::EndEvent() in
WebSocketTransportClientSocketPool::RequestSocket(). Fixed this to call
only EndEvent().

Removed pre-condition checks before ActivateStalledRequest() which are
already performed by its while loop.

Added DCHECK_NE(error, OK) to FlushWithError() to make sure OK is never
received from that path.

Reordered some lines.

R=yhirano@chromium.org,mmenke@chromium.org
BUG=642363
==========

to

==========
Refactor WebSocketTransportClientSocketPool's socket handing out code

Tries to make the code a little more readable to help debugging the
associated bug.

For the OK result, BoundNetLog::EndEventWithNetErrorCode() was called
after BoundNetLog::EndEvent() in
WebSocketTransportClientSocketPool::RequestSocket(). Fixed this to call
only EndEvent().

Removed pre-condition checks before ActivateStalledRequest() which are
already performed by its while loop.

Added DCHECK_NE(error, OK) to FlushWithError() to make sure OK is never
received from that path.

Reordered some lines.

R=yhirano@chromium.org,mmenke@chromium.org
BUG=642363
==========

[commit-bot: I haz the power, 2016-09-16 07:52:59]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-16 07:54:28]

Description was changed from

==========
Refactor WebSocketTransportClientSocketPool's socket handing out code

Tries to make the code a little more readable to help debugging the
associated bug.

For the OK result, BoundNetLog::EndEventWithNetErrorCode() was called
after BoundNetLog::EndEvent() in
WebSocketTransportClientSocketPool::RequestSocket(). Fixed this to call
only EndEvent().

Removed pre-condition checks before ActivateStalledRequest() which are
already performed by its while loop.

Added DCHECK_NE(error, OK) to FlushWithError() to make sure OK is never
received from that path.

Reordered some lines.

R=yhirano@chromium.org,mmenke@chromium.org
BUG=642363
==========

to

==========
Refactor WebSocketTransportClientSocketPool's socket handing out code

Tries to make the code a little more readable to help debugging the
associated bug.

For the OK result, BoundNetLog::EndEventWithNetErrorCode() was called
after BoundNetLog::EndEvent() in
WebSocketTransportClientSocketPool::RequestSocket(). Fixed this to call
only EndEvent().

Removed pre-condition checks before ActivateStalledRequest() which are
already performed by its while loop.

Added DCHECK_NE(error, OK) to FlushWithError() to make sure OK is never
received from that path.

Reordered some lines.

R=yhirano@chromium.org,mmenke@chromium.org
BUG=642363

Committed: https://crrev.com/913f8b743b1922bd332f01d7456ad688b80463cf
Cr-Commit-Position: refs/heads/master@{#419127}
==========

[commit-bot: I haz the power, 2016-09-16 07:54:29]

Patchset 6 (id:??) landed as
https://crrev.com/913f8b743b1922bd332f01d7456ad688b80463cf
Cr-Commit-Position: refs/heads/master@{#419127}