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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 68213017: Fix HTTP protocol scheme debugging checks for WebSockets (Closed)

Created:
7 years, 1 month ago by Adam Rice
Modified:
7 years, 1 month ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, jshin+watch_chromium.org
Base URL:
http://git.chromium.org/chromium/src.git@master
Visibility:
Public.

Description

Fix some debugging checks that expect HTTP protocol schemes to also accept WebSocket protocol schemes. No functional change to release builds. Debug builds will stop crashing when the new WebSocket implementation is used. BUG= TEST=net_unittests Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=236713

Patch Set 1 #

Total comments: 1

Patch Set 2 : Factor out HttpUtil::SchemeIsWsOrWss static method. #

Patch Set 3 : Fix typo. #

Total comments: 2

Patch Set 4 : Use SchemeIsWsOrWss() in HttpAuthHandlerDigest. #

Total comments: 2

Patch Set 5 : Add SchemeIsWSOrWSS to GURL. #

Patch Set 6 : Tweak header file inclusions. #

Patch Set 7 : Additional minor #include tweak. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -5 lines) Patch
M net/http/http_auth_cache.cc View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M net/http/http_auth_handler_digest.cc View 1 2 3 4 5 6 2 chunks +2 lines, -1 line 2 comments Download
M net/http/http_util_icu.cc View 1 2 3 4 2 chunks +4 lines, -3 lines 0 comments Download
M url/gurl.h View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M url/gurl.cc View 1 2 3 4 1 chunk +4 lines, -0 lines 0 comments Download
M url/gurl_unittest.cc View 1 2 3 4 1 chunk +12 lines, -0 lines 0 comments Download

Messages

Total messages: 18 (0 generated)
Adam Rice
7 years, 1 month ago (2013-11-14 10:49:08 UTC) #1
tyoshino (SeeGerritForStatus)
https://codereview.chromium.org/68213017/diff/1/net/http/http_util_icu.cc File net/http/http_util_icu.cc (right): https://codereview.chromium.org/68213017/diff/1/net/http/http_util_icu.cc#newcode18 net/http/http_util_icu.cc:18: url.SchemeIs("ws") || url.SchemeIs("wss"))); how about a static method for ...
7 years, 1 month ago (2013-11-15 04:17:16 UTC) #2
yhirano
lgtm https://codereview.chromium.org/68213017/diff/90001/net/http/http_util.h File net/http/http_util.h (right): https://codereview.chromium.org/68213017/diff/90001/net/http/http_util.h#newcode202 net/http/http_util.h:202: static bool SchemeIsWsOrWss(const GURL& url); [optional] Using this ...
7 years, 1 month ago (2013-11-18 04:14:06 UTC) #3
tyoshino (SeeGerritForStatus)
lgtm
7 years, 1 month ago (2013-11-18 04:30:24 UTC) #4
yhirano
https://codereview.chromium.org/68213017/diff/90001/net/http/http_util.h File net/http/http_util.h (right): https://codereview.chromium.org/68213017/diff/90001/net/http/http_util.h#newcode202 net/http/http_util.h:202: static bool SchemeIsWsOrWss(const GURL& url); On 2013/11/18 04:14:07, yhirano ...
7 years, 1 month ago (2013-11-18 04:32:36 UTC) #5
Adam Rice
+szym for OWNERS review.
7 years, 1 month ago (2013-11-18 05:15:59 UTC) #6
szym
https://codereview.chromium.org/68213017/diff/170001/net/http/http_auth_cache.cc File net/http/http_auth_cache.cc (right): https://codereview.chromium.org/68213017/diff/170001/net/http/http_auth_cache.cc#newcode50 net/http/http_auth_cache.cc:50: HttpUtil::SchemeIsWsOrWss(origin)); Add SchemeIsWSOrWSS to GURL.
7 years, 1 month ago (2013-11-19 02:42:50 UTC) #7
Adam Rice
+abarth for GURL OWNERS.
7 years, 1 month ago (2013-11-19 03:56:16 UTC) #8
Adam Rice
https://codereview.chromium.org/68213017/diff/170001/net/http/http_auth_cache.cc File net/http/http_auth_cache.cc (right): https://codereview.chromium.org/68213017/diff/170001/net/http/http_auth_cache.cc#newcode50 net/http/http_auth_cache.cc:50: HttpUtil::SchemeIsWsOrWss(origin)); On 2013/11/19 02:42:50, szym wrote: > Add SchemeIsWSOrWSS ...
7 years, 1 month ago (2013-11-19 03:56:43 UTC) #9
szym
https://codereview.chromium.org/68213017/diff/330001/net/http/http_auth_handler_digest.cc File net/http/http_auth_handler_digest.cc (right): https://codereview.chromium.org/68213017/diff/330001/net/http/http_auth_handler_digest.cc#newcode308 net/http/http_auth_handler_digest.cc:308: (url.SchemeIs("https") || url.SchemeIsWSOrWSS())) { Double checking: is "ws" okay ...
7 years, 1 month ago (2013-11-19 04:39:12 UTC) #10
tyoshino (SeeGerritForStatus)
https://codereview.chromium.org/68213017/diff/330001/net/http/http_auth_handler_digest.cc File net/http/http_auth_handler_digest.cc (right): https://codereview.chromium.org/68213017/diff/330001/net/http/http_auth_handler_digest.cc#newcode308 net/http/http_auth_handler_digest.cc:308: (url.SchemeIs("https") || url.SchemeIsWSOrWSS())) { On 2013/11/19 04:39:12, szym wrote: ...
7 years, 1 month ago (2013-11-19 04:53:46 UTC) #11
szym
lgtm
7 years, 1 month ago (2013-11-19 04:55:57 UTC) #12
Adam Rice
abarth, friendly reminder, please look at the GURL changes.
7 years, 1 month ago (2013-11-21 04:04:26 UTC) #13
abarth-chromium
Sorry, I missed this in my inbox. src/url <--- LGTM
7 years, 1 month ago (2013-11-21 21:31:48 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ricea@chromium.org/68213017/330001
7 years, 1 month ago (2013-11-22 01:52:04 UTC) #15
commit-bot: I haz the power
Retried try job too often on linux_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&number=193645
7 years, 1 month ago (2013-11-22 03:38:38 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ricea@chromium.org/68213017/330001
7 years, 1 month ago (2013-11-22 06:41:52 UTC) #17
commit-bot: I haz the power
7 years, 1 month ago (2013-11-22 07:40:51 UTC) #18
Message was sent while issue was closed. 
Change committed as 236713

Powered by Google App Engine
This is Rietveld 408576698