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

Issue 8362027: websocket-to-TCP proxy: observe value of listening port in right place. (Closed)

Created:
9 years, 2 months ago by Denis Lagno
Modified:
9 years, 2 months ago
CC:
chromium-reviews, nkostylev+watch_chromium.org, Erik does not do reviews, mihaip+watch_chromium.org, Aaron Boodman, stevenjb+watch_chromium.org, davemoore+watch_chromium.org
Visibility:
Public.

Description

websocket-to-TCP proxy: observe value of listening port in right place. Also minor change for new API: increase timeout for waiting WEB_SOCKET_PROXY_STARTED notification. New API (GetURLForTCP as opposed to GetPassportForTCP) runs on ephemeral port so if port is not known then we cannot construct URL at all. 3 seconds may be too little to launch proxy on slow overloaded machine. BUG=chromium-os:21942 TEST=Manual+Apitest Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107125

Patch Set 1 : p #

Total comments: 11

Patch Set 2 : reflected comments + rebased #

Total comments: 2

Patch Set 3 : DLOG #

Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -12 lines) Patch
M chrome/browser/chromeos/web_socket_proxy_controller.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/chromeos/web_socket_proxy_controller.cc View 1 2 5 chunks +32 lines, -5 lines 0 comments Download
M chrome/browser/extensions/extension_web_socket_proxy_private_api.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_web_socket_proxy_private_api.cc View 1 2 5 chunks +10 lines, -6 lines 0 comments Download
M chrome/browser/extensions/extension_web_socket_proxy_private_apitest.cc View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
Denis Lagno
please take a look: Dmitry: overall Mike: as owner of chrome/browser/extensions/*
9 years, 2 months ago (2011-10-21 13:43:10 UTC) #1
Dmitry Polukhin
LGTM
9 years, 2 months ago (2011-10-21 13:51:37 UTC) #2
Denis Lagno
> Mike: as owner of chrome/browser/extensions/* ping
9 years, 2 months ago (2011-10-24 17:32:58 UTC) #3
miket_OOO
Small comments. Otherwise LGTM. http://codereview.chromium.org/8362027/diff/4002/chrome/browser/chromeos/web_socket_proxy_controller.cc File chrome/browser/chromeos/web_socket_proxy_controller.cc (right): http://codereview.chromium.org/8362027/diff/4002/chrome/browser/chromeos/web_socket_proxy_controller.cc#newcode128 chrome/browser/chromeos/web_socket_proxy_controller.cc:128: return port_; I can't say ...
9 years, 2 months ago (2011-10-24 17:50:46 UTC) #4
Denis Lagno
http://codereview.chromium.org/8362027/diff/4002/chrome/browser/chromeos/web_socket_proxy_controller.cc File chrome/browser/chromeos/web_socket_proxy_controller.cc (right): http://codereview.chromium.org/8362027/diff/4002/chrome/browser/chromeos/web_socket_proxy_controller.cc#newcode128 chrome/browser/chromeos/web_socket_proxy_controller.cc:128: return port_; On 2011/10/24 17:50:46, miket wrote: > I ...
9 years, 2 months ago (2011-10-24 19:47:58 UTC) #5
miket_OOO
http://codereview.chromium.org/8362027/diff/4002/chrome/browser/chromeos/web_socket_proxy_controller.cc File chrome/browser/chromeos/web_socket_proxy_controller.cc (right): http://codereview.chromium.org/8362027/diff/4002/chrome/browser/chromeos/web_socket_proxy_controller.cc#newcode128 chrome/browser/chromeos/web_socket_proxy_controller.cc:128: return port_; > It should be infrequent operation, it ...
9 years, 2 months ago (2011-10-24 19:52:59 UTC) #6
Denis Lagno
9 years, 2 months ago (2011-10-25 11:19:37 UTC) #7
http://codereview.chromium.org/8362027/diff/7002/chrome/browser/chromeos/web_...
File chrome/browser/chromeos/web_socket_proxy_controller.cc (right):

http://codereview.chromium.org/8362027/diff/7002/chrome/browser/chromeos/web_...
chrome/browser/chromeos/web_socket_proxy_controller.cc:108: LOG(INFO) <<
"WebSocketProxyController initiation";
On 2011/10/24 19:53:00, miket wrote:
> Sorry if I missed this earlier. Can this be a DLOG instead to avoid shipping
the
> string in release builds?

Done.

Powered by Google App Engine
This is Rietveld 408576698