DevTools: Make port forwarding part of DevToolsAndroidBridge.

DevTools: Make port forwarding part of DevToolsAndroidBridge.

Rather than being a subscriber to DTAB port forwarding becomes part of it so that DTAB could stop discovering new devices when chrome://inspect is closed.

BUG=387067
Committed: https://crrev.com/4e7c1817734ef4ccf8749fffe3cb4e18be1f1e5a
Cr-Commit-Position: refs/heads/master@{#293143}

[vkuzkokov, 2014-08-26 14:39:53]

vkuzkokov@chromium.org changed reviewers:
+ dgozman@chromium.org

[dgozman, 2014-08-27 08:08:34]

This patch does not actually stop port forwarding when chrome://inspect is
closed, right? It just stops the polling and port forwarding updates.

Overall looks good, needs a patch description change.

https://codereview.chromium.org/500373004/diff/1/chrome/browser/devtools/devi...
File chrome/browser/devtools/device/devtools_android_bridge.h (right):

https://codereview.chromium.org/500373004/diff/1/chrome/browser/devtools/devi...
chrome/browser/devtools/device/devtools_android_bridge.h:212: typedef int
PortStatus;
Why this class doesn't expose status constants? What I am supposed to compare
status with?

https://codereview.chromium.org/500373004/diff/1/chrome/browser/devtools/devi...
File chrome/browser/devtools/device/port_forwarding_controller.cc (right):

https://codereview.chromium.org/500373004/diff/1/chrome/browser/devtools/devi...
chrome/browser/devtools/device/port_forwarding_controller.cc:588: listening_ =
true;
I don't see where |listening_| is used.

https://codereview.chromium.org/500373004/diff/1/chrome/browser/devtools/devi...
File chrome/browser/devtools/device/port_forwarding_controller.h (right):

https://codereview.chromium.org/500373004/diff/1/chrome/browser/devtools/devi...
chrome/browser/devtools/device/port_forwarding_controller.h:11: #include
"chrome/browser/devtools/device/devtools_android_bridge.h"
Including the owner class header sounds strange.

[vkuzkokov, 2014-09-01 16:13:41]

Altered description.

https://codereview.chromium.org/500373004/diff/1/chrome/browser/devtools/devi...
File chrome/browser/devtools/device/devtools_android_bridge.h (right):

https://codereview.chromium.org/500373004/diff/1/chrome/browser/devtools/devi...
chrome/browser/devtools/device/devtools_android_bridge.h:212: typedef int
PortStatus;
On 2014/08/27 08:08:34, dgozman wrote:
> Why this class doesn't expose status constants? What I am supposed to compare
> status with?

Status is compared only in JS.

https://codereview.chromium.org/500373004/diff/1/chrome/browser/devtools/devi...
File chrome/browser/devtools/device/port_forwarding_controller.cc (right):

https://codereview.chromium.org/500373004/diff/1/chrome/browser/devtools/devi...
chrome/browser/devtools/device/port_forwarding_controller.cc:588: listening_ =
true;
On 2014/08/27 08:08:34, dgozman wrote:
> I don't see where |listening_| is used.

Good point.

https://codereview.chromium.org/500373004/diff/1/chrome/browser/devtools/devi...
File chrome/browser/devtools/device/port_forwarding_controller.h (right):

https://codereview.chromium.org/500373004/diff/1/chrome/browser/devtools/devi...
chrome/browser/devtools/device/port_forwarding_controller.h:11: #include
"chrome/browser/devtools/device/devtools_android_bridge.h"
On 2014/08/27 08:08:34, dgozman wrote:
> Including the owner class header sounds strange.

We need this for inner classes of DTAB.

[dgozman, 2014-09-01 16:48:17]

lgtm

[vkuzkokov, 2014-09-02 11:07:18]

The CQ bit was checked by vkuzkokov@chromium.org

[commit-bot: I haz the power, 2014-09-02 11:07:26]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vkuzkokov@chromium.org/500373004/20001

[commit-bot: I haz the power, 2014-09-02 15:02:19]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-09-02 15:32:15]

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

[commit-bot: I haz the power, 2014-09-02 15:32:16]

Try jobs failed on following builders:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[vkuzkokov, 2014-09-03 11:29:35]

The CQ bit was checked by vkuzkokov@chromium.org

[commit-bot: I haz the power, 2014-09-03 11:31:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vkuzkokov@chromium.org/500373004/40001

[commit-bot: I haz the power, 2014-09-03 15:17:44]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[commit-bot: I haz the power, 2014-09-03 16:06:51]

Committed patchset #3 (id:40001) as 090147a442356a83a68c01ed4731038d9dea4767

[commit-bot: I haz the power, 2014-09-10 03:25:57]

Patchset 3 (id:??) landed as
https://crrev.com/4e7c1817734ef4ccf8749fffe3cb4e18be1f1e5a
Cr-Commit-Position: refs/heads/master@{#293143}