Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(703)

Issue 2639933005: Rename WS handshake success error code; update enums. (Closed)

Created:
3 years, 11 months ago by pkalinnikov
Modified:
3 years, 10 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, asvitkine+watch_chromium.org, mmenke
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Rename WS handshake success error code; update enums. The new error code needs to be added to histograms.xml, as well as some other values that apparently have been forgotten to be added. The reason for renaming the error code is that WebSocket error codes start with a WS_ prefix. BUG=663672 Review-Url: https://codereview.chromium.org/2639933005 Cr-Commit-Position: refs/heads/master@{#444771} Committed: https://chromium.googlesource.com/chromium/src/+/2ff3fe13ff8f01226d36f63e9e695bbaf747dbdb

Patch Set 1 #

Patch Set 2 : Fix build. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -5 lines) Patch
M net/base/net_error_list.h View 1 chunk +3 lines, -3 lines 0 comments Download
M net/websockets/websocket_stream.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M net/websockets/websocket_stream_test.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M tools/metrics/histograms/histograms.xml View 6 chunks +10 lines, -0 lines 0 comments Download

Messages

Total messages: 25 (14 generated)
pkalinnikov
PTAL.
3 years, 11 months ago (2017-01-19 12:50:14 UTC) #3
Adam Rice
Awesome, thanks. You will also need to rename WEBSOCKET_HANDSHAKE_SUCCESS in //net/websockets/websocket_stream.cc and //net/websockets/websocket_stream_test.cc.
3 years, 11 months ago (2017-01-19 12:55:10 UTC) #6
pkalinnikov
Sure, forgot to commit this. Thanks!
3 years, 11 months ago (2017-01-19 12:59:37 UTC) #9
Adam Rice
lgtm
3 years, 11 months ago (2017-01-19 13:12:00 UTC) #10
tyoshino (SeeGerritForStatus)
lgtm
3 years, 11 months ago (2017-01-19 13:52:42 UTC) #11
yhirano
lgtm
3 years, 11 months ago (2017-01-19 14:14:13 UTC) #12
davidben
lgtm
3 years, 11 months ago (2017-01-19 17:21:29 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2639933005/20001
3 years, 11 months ago (2017-01-19 17:23:46 UTC) #17
commit-bot: I haz the power
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/2ff3fe13ff8f01226d36f63e9e695bbaf747dbdb
3 years, 11 months ago (2017-01-19 17:29:33 UTC) #20
Wez
Hallo ericwilligers@chromium.org! Due to a depot_tools patch which mistakenly removed the OWNERS check for non-source ...
3 years, 10 months ago (2017-02-07 22:33:20 UTC) #22
Steven Holte
3 years, 10 months ago (2017-02-08 00:34:35 UTC) #25
Message was sent while issue was closed.
histograms lgtm

Powered by Google App Engine
This is Rietveld 408576698