Spdy{RstStream,GoAway}Status -> SpdyErrorCode.

Spdy{RstStream,GoAway}Status -> SpdyErrorCode.

Combine enums SpdyRstStreamStatus and SpdyGoawayStatus into
SpdyErrorCode, and change underlying type to uint32.

Also rename *status arguments, local variables, members, accessors, and
log messages to *error_code.  It seems to me that "status" is leftover
terminology from early SPDY versions: RFC 7540 clearly talks about
"error code".

This CL lands server changes 144330546 and 145469431,
and parts of 145013960, by bnc.

BUG=488484
Review-Url: https://codereview.chromium.org/2675593002
Cr-Commit-Position: refs/heads/master@{#447994}
Committed: https://chromium.googlesource.com/chromium/src/+/9904e270bbb387015073b4fe0153e667c09e155a

[diannahu, 2017-02-02 03:50:38]

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

[commit-bot: I haz the power, 2017-02-02 03:51:05]

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, 2017-02-02 04:14:01]

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

[commit-bot: I haz the power, 2017-02-02 04:14:02]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[diannahu, 2017-02-02 05:26:25]

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

[commit-bot: I haz the power, 2017-02-02 05:26:39]

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, 2017-02-02 06:50:50]

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

[commit-bot: I haz the power, 2017-02-02 06:50:51]

Dry run: This issue passed the CQ dry run.

[Bence, 2017-02-02 15:12:24]

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

[Bence, 2017-02-02 15:12:25]

I know you didn't ask for a review, I jumped ahead proactively just to find that
we need to wait for server change 145087791 to be merged.  :(

https://codereview.chromium.org/2675593002/diff/20001/net/spdy/spdy_framer.cc
File net/spdy/spdy_framer.cc (right):

https://codereview.chromium.org/2675593002/diff/20001/net/spdy/spdy_framer.cc...
net/spdy/spdy_framer.cc:406: const char* SpdyFramer::StatusCodeToString(int
status_code) {
This method is removed in server change 145087791, please wait for me to merge
that.

https://codereview.chromium.org/2675593002/diff/20001/net/spdy/spdy_protocol.cc
File net/spdy/spdy_protocol.cc (right):

https://codereview.chromium.org/2675593002/diff/20001/net/spdy/spdy_protocol....
net/spdy/spdy_protocol.cc:139: const char* ErrorCodeToString(SpdyErrorCode
error_code) {
This method in only introduced in server change 145087791, please wait for me to
merge that.  (And it is only later, in 145536832, that the argument is changed
from uint32 to SpdyErrorCode.)

https://codereview.chromium.org/2675593002/diff/20001/net/spdy/spdy_session.h
File net/spdy/spdy_session.h (right):

https://codereview.chromium.org/2675593002/diff/20001/net/spdy/spdy_session.h...
net/spdy/spdy_session.h:133: PROTOCOL_ERROR_ERROR_CODE_FOR_NON_ACTIVE_STREAM =
23,
Please leave this as RST_STREAM (since GOAWAY does not have an associated
stream, so "for non-active stream" must be RST_STREAM).

[diannahu, 2017-02-02 15:58:09]

Thanks for your proactive review! I will rebase on your merge of 145087791 and
then take another look through the diffs to make sure things look reasonable.
Thanks!

https://codereview.chromium.org/2675593002/diff/20001/net/spdy/spdy_framer.cc
File net/spdy/spdy_framer.cc (right):

https://codereview.chromium.org/2675593002/diff/20001/net/spdy/spdy_framer.cc...
net/spdy/spdy_framer.cc:406: const char* SpdyFramer::StatusCodeToString(int
status_code) {
On 2017/02/02 15:12:25, Bence wrote:
> This method is removed in server change 145087791, please wait for me to merge
> that.

Got it, will do! (I was wondering why I didn't see this method at HEAD. :P)

https://codereview.chromium.org/2675593002/diff/20001/net/spdy/spdy_protocol.cc
File net/spdy/spdy_protocol.cc (right):

https://codereview.chromium.org/2675593002/diff/20001/net/spdy/spdy_protocol....
net/spdy/spdy_protocol.cc:139: const char* ErrorCodeToString(SpdyErrorCode
error_code) {
On 2017/02/02 15:12:25, Bence wrote:
> This method in only introduced in server change 145087791, please wait for me
to
> merge that.  (And it is only later, in 145536832, that the argument is changed
> from uint32 to SpdyErrorCode.)

Got it, will merge after 145087791 is merged. (I actually have no idea how I
added this method. :D:) Should I keep it as uint32_t? Thanks!

https://codereview.chromium.org/2675593002/diff/20001/net/spdy/spdy_session.h
File net/spdy/spdy_session.h (right):

https://codereview.chromium.org/2675593002/diff/20001/net/spdy/spdy_session.h...
net/spdy/spdy_session.h:133: PROTOCOL_ERROR_ERROR_CODE_FOR_NON_ACTIVE_STREAM =
23,
On 2017/02/02 15:12:25, Bence wrote:
> Please leave this as RST_STREAM (since GOAWAY does not have an associated
> stream, so "for non-active stream" must be RST_STREAM).

Whoops, I must've missed this when doing the find/replace. Good catch, thanks!

[Bence, 2017-02-02 16:12:00]

https://codereview.chromium.org/2675593002/diff/20001/net/spdy/spdy_protocol.cc
File net/spdy/spdy_protocol.cc (right):

https://codereview.chromium.org/2675593002/diff/20001/net/spdy/spdy_protocol....
net/spdy/spdy_protocol.cc:139: const char* ErrorCodeToString(SpdyErrorCode
error_code) {
On 2017/02/02 15:58:09, diannahu wrote:
> On 2017/02/02 15:12:25, Bence wrote:
> > This method in only introduced in server change 145087791, please wait for
me
> to
> > merge that.  (And it is only later, in 145536832, that the argument is
changed
> > from uint32 to SpdyErrorCode.)
> 
> Got it, will merge after 145087791 is merged. (I actually have no idea how I
> added this method. :D:) Should I keep it as uint32_t? Thanks!

It's really up to you.  You can include 145536832 in this CL if you think it's
trivial to do so.  I did it in a separate server side CL because some callers
must have been updated to use static_cast<>, and I preferred not to do too much
in a single CL.

[diannahu, 2017-02-02 16:57:37]

On 2017/02/02 16:12:00, Bence wrote:
>
https://codereview.chromium.org/2675593002/diff/20001/net/spdy/spdy_protocol.cc
> File net/spdy/spdy_protocol.cc (right):
> 
>
https://codereview.chromium.org/2675593002/diff/20001/net/spdy/spdy_protocol....
> net/spdy/spdy_protocol.cc:139: const char* ErrorCodeToString(SpdyErrorCode
> error_code) {
> On 2017/02/02 15:58:09, diannahu wrote:
> > On 2017/02/02 15:12:25, Bence wrote:
> > > This method in only introduced in server change 145087791, please wait for
> me
> > to
> > > merge that.  (And it is only later, in 145536832, that the argument is
> changed
> > > from uint32 to SpdyErrorCode.)
> > 
> > Got it, will merge after 145087791 is merged. (I actually have no idea how I
> > added this method. :D:) Should I keep it as uint32_t? Thanks!
> 
> It's really up to you.  You can include 145536832 in this CL if you think it's
> trivial to do so.  I did it in a separate server side CL because some callers
> must have been updated to use static_cast<>, and I preferred not to do too
much
> in a single CL.

That makes sense, and this merge already feels like my magnum opus merge. :D: I
will keep it as uint32_t and leave more work for you to merge 145536832 if
that's alright. :P Thanks Bence!

[diannahu, 2017-02-02 22:04:13]

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

[commit-bot: I haz the power, 2017-02-02 22:05:01]

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, 2017-02-02 23:19:09]

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

[commit-bot: I haz the power, 2017-02-02 23:19:10]

Dry run: This issue passed the CQ dry run.

[diannahu, 2017-02-03 06:27:41]

My magnum opus merge. :) Thanks for your reviews!

[Bence, 2017-02-03 14:29:02]

Description was changed from

==========
Spdy{RstStream,GoAway}Status -> SpdyErrorCode.

Combine enums SpdyRstStreamStatus and SpdyGoawayStatus into
SpdyErrorCode.

Also rename *status arguments, local variables, members, accessors, and
log messages to *error_code.  It seems to me that "status" is leftover
terminology from early SPDY versions: RFC 7540 clearly talks about
"error code".

This CL lands server changes 144330546 and 145469431 by bnc.

BUG=488484
==========

to

==========
Spdy{RstStream,GoAway}Status -> SpdyErrorCode.

Combine enums SpdyRstStreamStatus and SpdyGoawayStatus into
SpdyErrorCode.

Also rename *status arguments, local variables, members, accessors, and
log messages to *error_code.  It seems to me that "status" is leftover
terminology from early SPDY versions: RFC 7540 clearly talks about
"error code".

This CL lands server changes 144330546 and 145469431,
and parts of 145013960, by bnc.

BUG=488484
==========

[Bence, 2017-02-03 14:29:34]

Description was changed from

==========
Spdy{RstStream,GoAway}Status -> SpdyErrorCode.

Combine enums SpdyRstStreamStatus and SpdyGoawayStatus into
SpdyErrorCode.

Also rename *status arguments, local variables, members, accessors, and
log messages to *error_code.  It seems to me that "status" is leftover
terminology from early SPDY versions: RFC 7540 clearly talks about
"error code".

This CL lands server changes 144330546 and 145469431,
and parts of 145013960, by bnc.

BUG=488484
==========

to

==========
Spdy{RstStream,GoAway}Status -> SpdyErrorCode.

Combine enums SpdyRstStreamStatus and SpdyGoawayStatus into
SpdyErrorCode, and change underlying type to uint32.

Also rename *status arguments, local variables, members, accessors, and
log messages to *error_code.  It seems to me that "status" is leftover
terminology from early SPDY versions: RFC 7540 clearly talks about
"error code".

This CL lands server changes 144330546 and 145469431,
and parts of 145013960, by bnc.

BUG=488484
==========

[Bence, 2017-02-03 14:30:05]

LGTM.  FYI you also accidentally included parts of 145013960, I changed the CL
description accordingly.

[Bence, 2017-02-03 14:30:11]

The CQ bit was checked by bnc@chromium.org

[commit-bot: I haz the power, 2017-02-03 14:30:23]

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, 2017-02-03 14:40:12]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1486132211510230,
"parent_rev": "68ecde39165b821aab14012aa2d8e062d7507fe5", "commit_rev":
"9904e270bbb387015073b4fe0153e667c09e155a"}

[commit-bot: I haz the power, 2017-02-03 14:40:43]

Description was changed from

==========
Spdy{RstStream,GoAway}Status -> SpdyErrorCode.

Combine enums SpdyRstStreamStatus and SpdyGoawayStatus into
SpdyErrorCode, and change underlying type to uint32.

Also rename *status arguments, local variables, members, accessors, and
log messages to *error_code.  It seems to me that "status" is leftover
terminology from early SPDY versions: RFC 7540 clearly talks about
"error code".

This CL lands server changes 144330546 and 145469431,
and parts of 145013960, by bnc.

BUG=488484
==========

to

==========
Spdy{RstStream,GoAway}Status -> SpdyErrorCode.

Combine enums SpdyRstStreamStatus and SpdyGoawayStatus into
SpdyErrorCode, and change underlying type to uint32.

Also rename *status arguments, local variables, members, accessors, and
log messages to *error_code.  It seems to me that "status" is leftover
terminology from early SPDY versions: RFC 7540 clearly talks about
"error code".

This CL lands server changes 144330546 and 145469431,
and parts of 145013960, by bnc.

BUG=488484

Review-Url: https://codereview.chromium.org/2675593002
Cr-Commit-Position: refs/heads/master@{#447994}
Committed:
https://chromium.googlesource.com/chromium/src/+/9904e270bbb387015073b4fe0153...
==========

[commit-bot: I haz the power, 2017-02-03 14:40:44]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/9904e270bbb387015073b4fe0153...