Visualize status of port forwarding sockets in chrome:inspect Devices tab

Visualize status of port forwarding sockets in chrome:inspect Devices tab

BUG=270046
R=pfeldman@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=216973

[pfeldman, 2013-08-08 16:43:22]

lgtm

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/devto...
File chrome/browser/devtools/devtools_adb_bridge.h (right):

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/devto...
chrome/browser/devtools/devtools_adb_bridge.h:181: void SetPortStatus(const
PortStatusMap& port_status) {
set_port_status

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/devto...
File chrome/browser/devtools/devtools_protocol.h (right):

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/devto...
chrome/browser/devtools/devtools_protocol.h:85: static Response*
ParseResponse(const std::string& json);
Please add comment suggesting that ownership is passed to the caller.

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/tethe...
File chrome/browser/devtools/tethering_adb_filter.cc (right):

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/tethe...
chrome/browser/devtools/tethering_adb_filter.cc:36: const int kStatusOK = 0;
kStatusOk. Is that enum?

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/tethe...
chrome/browser/devtools/tethering_adb_filter.cc:307: bool
TetheringAdbFilter::ProcessResponse(const std::string& message) {
Sounds like a part of devtools_protocol.

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/tethe...
chrome/browser/devtools/tethering_adb_filter.cc:362: if (message.length() > 200)
This check is no longer needed - you are the only protocol client.

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/tethe...
File chrome/browser/devtools/tethering_adb_filter.h (right):

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/tethe...
chrome/browser/devtools/tethering_adb_filter.h:84: void
UpdateDeviceList(DevToolsAdbBridge::RemoteDevices& devices);
Why is that now mutable?

[Vladislav Kaznacheev, 2013-08-09 08:24:58]

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/devto...
File chrome/browser/devtools/devtools_adb_bridge.h (right):

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/devto...
chrome/browser/devtools/devtools_adb_bridge.h:181: void SetPortStatus(const
PortStatusMap& port_status) {
On 2013/08/08 16:43:22, pfeldman wrote:
> set_port_status

Done.

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/devto...
File chrome/browser/devtools/devtools_protocol.h (right):

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/devto...
chrome/browser/devtools/devtools_protocol.h:85: static Response*
ParseResponse(const std::string& json);
On 2013/08/08 16:43:22, pfeldman wrote:
> Please add comment suggesting that ownership is passed to the caller.

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/tethe...
File chrome/browser/devtools/tethering_adb_filter.cc (right):

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/tethe...
chrome/browser/devtools/tethering_adb_filter.cc:36: const int kStatusOK = 0;
Done
On 2013/08/08 16:43:22, pfeldman wrote:
> kStatusOk. Is that enum?

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/tethe...
chrome/browser/devtools/tethering_adb_filter.cc:307: bool
TetheringAdbFilter::ProcessResponse(const std::string& message) {
You mean move the pending_responses_ part into DevToolsProtocol? Lets wait for
the second usage before generalizing.

On 2013/08/08 16:43:22, pfeldman wrote:
> Sounds like a part of devtools_protocol.

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/tethe...
chrome/browser/devtools/tethering_adb_filter.cc:362: if (message.length() > 200)
Removed this check and the one looking for kTetheringAccepted string.
On 2013/08/08 16:43:22, pfeldman wrote:
> This check is no longer needed - you are the only protocol client.

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/tethe...
File chrome/browser/devtools/tethering_adb_filter.h (right):

https://codereview.chromium.org/22685003/diff/1/chrome/browser/devtools/tethe...
chrome/browser/devtools/tethering_adb_filter.h:84: void
UpdateDeviceList(DevToolsAdbBridge::RemoteDevices& devices);
Oops. The method calls DevToolsAdbBridge::RemoteDevice::set_port_status which is
non-const but it does not require removing the const from the container.

On 2013/08/08 16:43:22, pfeldman wrote:
> Why is that now mutable?

[commit-bot: I haz the power, 2013-08-09 08:38:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaznacheev@chromium.org/22685003/9001

[commit-bot: I haz the power, 2013-08-09 09:04:12]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-08-09 11:04:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaznacheev@chromium.org/22685003/31001

[commit-bot: I haz the power, 2013-08-09 18:41:04]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-08-12 08:10:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaznacheev@chromium.org/22685003/51001

[Vladislav Kaznacheev, 2013-08-12 11:40:05]

Committed patchset #5 manually as r216973 (presubmit successful).