QUIC - minor reorg of QuicCryptoClientStream::DoHandshakeLoop.

QUIC - minor reorg of QuicCryptoClientStream::DoHandshakeLoop.

Added Do... methods for each state in DoHandshakeLoop (similar to the
conventions that we typically use in our state machines).

DoVerifyProof and DoGetChannelID which could do async operation
return  QuicAsyncStatus.

In DoHandshakeLoop changed "for (;;) {" to be similar to other
do { ...} while loops of other state machines in chromium.

R=rch@chromium.org,agl@chromium.org

Committed: https://crrev.com/46f73cea6f512782fff8697cbdc5c8bdb1b63edd
Cr-Commit-Position: refs/heads/master@{#297316}

[ramant (doing other things), 2014-09-19 03:13:07]

Hi Ryan and Adam,
  This CL doesn't make any functional change (just moving code into Do...
methods).

https://codereview.chromium.org/588443002/diff/1/net/quic/quic_crypto_client_...
File net/quic/quic_crypto_client_stream.h (right):

https://codereview.chromium.org/588443002/diff/1/net/quic/quic_crypto_client_...
net/quic/quic_crypto_client_stream.h:123: void
DoInitialize(QuicCryptoClientConfig::CachedState* cached);
Hi Ryan and Adam,
  Added crude comments. Would appreciate help in documenting code better.

Thanks much.

[agl, 2014-09-19 17:26:52]

agl@chromium.org changed reviewers:
+ agl@chromium.org

[agl, 2014-09-19 17:26:53]

https://codereview.chromium.org/588443002/diff/1/net/quic/quic_crypto_client_...
File net/quic/quic_crypto_client_stream.cc (right):

https://codereview.chromium.org/588443002/diff/1/net/quic/quic_crypto_client_...
net/quic/quic_crypto_client_stream.cc:204: if (QUIC_SUCCESS !=
DoGetChannelID(cached)) {
In other cases you have changed returns into next_state_ = STATE_DONE. Why is
this one different?

[ramant (doing other things), 2014-09-19 21:25:25]

https://codereview.chromium.org/588443002/diff/1/net/quic/quic_crypto_client_...
File net/quic/quic_crypto_client_stream.cc (right):

https://codereview.chromium.org/588443002/diff/1/net/quic/quic_crypto_client_...
net/quic/quic_crypto_client_stream.cc:204: if (QUIC_SUCCESS !=
DoGetChannelID(cached)) {
On 2014/09/19 17:26:52, agl wrote:
> In other cases you have changed returns into next_state_ = STATE_DONE. Why is
> this one different?

In DoGetChannelID and DoVerifyProof we shouldn't modify next_state_ to
STATE_DONE when there is IO_PENDING (next_state_ should remain as ...COMPLETE).

Need to change for loop to exit if the return value is ERR_IO_PENDING and we
could change DoGetChannelID and DoVerifyProof to return ERR_IO_PENDING when it
wants to exit from the for loop, but leave the next_state_ as ...COMPLETE. (I
hadn't modified the forever for loop in this CL). 

I am not feeling well. Will modify the code and upload it asap.

thanks,
raman

[agl, 2014-09-20 00:13:14]

(Please ping when ready for review.)

[ramant (doing other things), 2014-09-22 17:58:26]

Patchset #2 (id:20001) has been deleted

[ramant (doing other things), 2014-09-23 22:55:03]

PTAL. thanks.

[ramant (doing other things), 2014-09-23 23:04:21]

rtenneti@chromium.org changed reviewers:
- agl@chromium.org, rch@chromium.org

[ramant (doing other things), 2014-09-23 23:05:47]

On 2014/09/23 22:55:03, ramant wrote:
> PTAL. thanks.

Please don't review this CL yet. thanks.

[ramant (doing other things), 2014-09-24 02:21:15]

Patchset #2 (id:40001) has been deleted

[ramant (doing other things), 2014-09-24 02:21:22]

Patchset #2 (id:60001) has been deleted

[ramant (doing other things), 2014-09-24 02:21:27]

Patchset #2 (id:80001) has been deleted

[ramant (doing other things), 2014-09-24 02:21:35]

Patchset #2 (id:100001) has been deleted

[ramant (doing other things), 2014-09-24 03:17:21]

Patchset #2 (id:120001) has been deleted

[ramant (doing other things), 2014-09-24 03:48:15]

rtenneti@chromium.org changed reviewers:
+ rch@chromium.org

[ramant (doing other things), 2014-09-24 03:50:20]

Hi Ryan,
  I have moved the code into Do... methods (would like to tighten the code in
the next CL). Made the state machine similar to other network library state
machines. Would appreciate your comments, before publishing it for review.

thanks much
raman

[Ryan Hamilton, 2014-09-24 15:36:48]

Hm, I notice that you're returning net error codes. I don't think we have access
to them in google3, do we? Otherwise, i love this change!

https://codereview.chromium.org/588443002/diff/1/net/quic/quic_crypto_client_...
File net/quic/quic_crypto_client_stream.h (right):

https://codereview.chromium.org/588443002/diff/1/net/quic/quic_crypto_client_...
net/quic/quic_crypto_client_stream.h:123: void
DoInitialize(QuicCryptoClientConfig::CachedState* cached);
On 2014/09/19 03:13:07, ramant wrote:
> Hi Ryan and Adam,
>   Added crude comments. Would appreciate help in documenting code better.
> 
> Thanks much.

These seem fine. We often don't even comment our various DoFoo() method.

[ramant (doing other things), 2014-09-27 02:02:18]

Patchset #3 (id:160001) has been deleted

[ramant (doing other things), 2014-09-27 02:07:34]

Patchset #3 (id:180001) has been deleted

[ramant (doing other things), 2014-09-27 02:33:24]

rtenneti@chromium.org changed reviewers:
+ agl@chromium.org

[ramant (doing other things), 2014-09-27 02:34:44]

Hi Ryan and Adam,
  Sorry for the delay in sending this review (was working on other M39 stuff).
Will submit this change after M39 branch is cut. PTAL.

thanks
raman

[Ryan Hamilton, 2014-09-29 17:27:09]

LGTM. The structure looks great! I confess, I didn't read the bodies of the new
methods too closely, as they look like they're simply the result of moving code
around.

[agl, 2014-09-29 17:41:44]

lgtm

[ramant (doing other things), 2014-09-29 17:44:05]

On 2014/09/29 17:41:44, agl wrote:
> lgtm

Thanks very much Adam and Ryan,
raman

[ramant (doing other things), 2014-09-29 17:50:53]

The CQ bit was checked by rtenneti@chromium.org

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

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

[commit-bot: I haz the power, 2014-09-29 23:53:08]

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

[commit-bot: I haz the power, 2014-09-29 23:53:09]

Exceeded time limit waiting for builds to trigger.

[ramant (doing other things), 2014-09-29 23:55:37]

The CQ bit was checked by rtenneti@chromium.org

[commit-bot: I haz the power, 2014-09-29 23:56:41]

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

[commit-bot: I haz the power, 2014-09-30 00:29:04]

Committed patchset #3 (id:200001) as a1aec104df7776193ec33369aa1e0fb22714ad6e

[commit-bot: I haz the power, 2014-09-30 00:29:55]

Patchset 3 (id:??) landed as
https://crrev.com/46f73cea6f512782fff8697cbdc5c8bdb1b63edd
Cr-Commit-Position: refs/heads/master@{#297316}