[DevTools] Introduce a target discovery preference

[DevTools] Introduce a target discovery preference

BUG=636084
R=dgozman@chromium.org, pfeldman@chromium.org

Committed: https://crrev.com/ce843997eacb347b2fdae01daf2a93f398eb5083
Cr-Commit-Position: refs/heads/master@{#412409}

[eostroukhov, 2016-08-16 18:30:43]

The CQ bit was checked by eostroukhov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-16 18:31:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[eostroukhov, 2016-08-16 18:31:19]

Please take a look.

[commit-bot: I haz the power, 2016-08-16 19:16:20]

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

[commit-bot: I haz the power, 2016-08-16 19:16:20]

Dry run: This issue passed the CQ dry run.

[dgozman, 2016-08-16 20:24:33]

Can we test this easily?

https://codereview.chromium.org/2253683002/diff/1/chrome/browser/devtools/dev...
File chrome/browser/devtools/device/devtools_android_bridge.cc (right):

https://codereview.chromium.org/2253683002/diff/1/chrome/browser/devtools/dev...
chrome/browser/devtools/device/devtools_android_bridge.cc:950:
discoveryPref->GetValue()->GetAsList(&targets);
What if it's not there?

https://codereview.chromium.org/2253683002/diff/1/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/2253683002/diff/1/chrome/common/pref_names.cc...
chrome/common/pref_names.cc:1541: // A list of strings representing devtools
target discovery servers
nit: please add a full stop at the end.

[eostroukhov, 2016-08-16 23:47:48]

I added a test case. Please take another look.

[eostroukhov, 2016-08-16 23:47:56]

The CQ bit was checked by eostroukhov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-16 23:48:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[eostroukhov-old, 2016-08-16 23:55:32]

eostroukhov@google.com changed reviewers:
+ eostroukhov@google.com

[eostroukhov-old, 2016-08-16 23:55:33]

https://codereview.chromium.org/2253683002/diff/1/chrome/browser/devtools/dev...
File chrome/browser/devtools/device/devtools_android_bridge.cc (right):

https://codereview.chromium.org/2253683002/diff/1/chrome/browser/devtools/dev...
chrome/browser/devtools/device/devtools_android_bridge.cc:950:
discoveryPref->GetValue()->GetAsList(&targets);
On 2016/08/16 20:24:33, dgozman wrote:
> What if it's not there?

I updated the code - please let me know if I understand the API correctly.

https://codereview.chromium.org/2253683002/diff/1/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/2253683002/diff/1/chrome/common/pref_names.cc...
chrome/common/pref_names.cc:1541: // A list of strings representing devtools
target discovery servers
On 2016/08/16 20:24:33, dgozman wrote:
> nit: please add a full stop at the end.

Done.

[dgozman, 2016-08-17 00:22:19]

lgtm

https://codereview.chromium.org/2253683002/diff/20001/chrome/browser/devtools...
File chrome/browser/devtools/device/devtools_android_bridge.h (right):

https://codereview.chromium.org/2253683002/diff/20001/chrome/browser/devtools...
chrome/browser/devtools/device/devtools_android_bridge.h:221: typedef
base::Callback<void(scoped_refptr<TCPDeviceProvider>)>
typedef -> using

https://codereview.chromium.org/2253683002/diff/20001/chrome/browser/devtools...
chrome/browser/devtools/device/devtools_android_bridge.h:222:
tcp_provider_callback;
Types are TitleCase.

https://codereview.chromium.org/2253683002/diff/20001/chrome/browser/devtools...
chrome/browser/devtools/device/devtools_android_bridge.h:223: void
set_tcp_provider_callback_for_test(tcp_provider_callback callback) {
Move function definition to cc.

https://codereview.chromium.org/2253683002/diff/20001/chrome/browser/devtools...
File chrome/browser/devtools/device/devtools_android_bridge_browsertest.cc
(right):

https://codereview.chromium.org/2253683002/diff/20001/chrome/browser/devtools...
chrome/browser/devtools/device/devtools_android_bridge_browsertest.cc:4:
#include "base/values.h"
style: empty line before includes

https://codereview.chromium.org/2253683002/diff/20001/chrome/browser/devtools...
File chrome/browser/devtools/device/tcp_device_provider.h (right):

https://codereview.chromium.org/2253683002/diff/20001/chrome/browser/devtools...
chrome/browser/devtools/device/tcp_device_provider.h:36: HostPortSet
get_targets_for_test() {
Trivial getters are either one-line or put in .cc file.

[eostroukhov-old, 2016-08-17 00:44:22]

Thank you for the review. I updated the CL.

https://codereview.chromium.org/2253683002/diff/20001/chrome/browser/devtools...
File chrome/browser/devtools/device/devtools_android_bridge.h (right):

https://codereview.chromium.org/2253683002/diff/20001/chrome/browser/devtools...
chrome/browser/devtools/device/devtools_android_bridge.h:221: typedef
base::Callback<void(scoped_refptr<TCPDeviceProvider>)>
On 2016/08/17 00:22:19, dgozman wrote:
> typedef -> using

Did it for all typedefs in this file.

https://codereview.chromium.org/2253683002/diff/20001/chrome/browser/devtools...
chrome/browser/devtools/device/devtools_android_bridge.h:222:
tcp_provider_callback;
On 2016/08/17 00:22:19, dgozman wrote:
> Types are TitleCase.

Done.

https://codereview.chromium.org/2253683002/diff/20001/chrome/browser/devtools...
chrome/browser/devtools/device/devtools_android_bridge.h:223: void
set_tcp_provider_callback_for_test(tcp_provider_callback callback) {
On 2016/08/17 00:22:19, dgozman wrote:
> Move function definition to cc.

Done.

https://codereview.chromium.org/2253683002/diff/20001/chrome/browser/devtools...
File chrome/browser/devtools/device/devtools_android_bridge_browsertest.cc
(right):

https://codereview.chromium.org/2253683002/diff/20001/chrome/browser/devtools...
chrome/browser/devtools/device/devtools_android_bridge_browsertest.cc:4:
#include "base/values.h"
On 2016/08/17 00:22:19, dgozman wrote:
> style: empty line before includes

Done.

https://codereview.chromium.org/2253683002/diff/20001/chrome/browser/devtools...
File chrome/browser/devtools/device/tcp_device_provider.h (right):

https://codereview.chromium.org/2253683002/diff/20001/chrome/browser/devtools...
chrome/browser/devtools/device/tcp_device_provider.h:36: HostPortSet
get_targets_for_test() {
On 2016/08/17 00:22:19, dgozman wrote:
> Trivial getters are either one-line or put in .cc file.

Done.

[eostroukhov-old, 2016-08-17 00:44:23]

Thank you for the review. I updated the CL.

[eostroukhov-old, 2016-08-17 00:44:30]

The CQ bit was checked by eostroukhov@google.com

[eostroukhov-old, 2016-08-17 00:44:30]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2253683002/#ps40001
(title: "Comments were addressed.")

[commit-bot: I haz the power, 2016-08-17 00:44:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-17 01:19:33]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-17 01:22:07]

Description was changed from

==========
[DevTools] Introduce a target discovery preference

BUG=636084
R=dgozman@chromium.org, pfeldman@chromium.org
==========

to

==========
[DevTools] Introduce a target discovery preference

BUG=636084
R=dgozman@chromium.org, pfeldman@chromium.org

Committed: https://crrev.com/ce843997eacb347b2fdae01daf2a93f398eb5083
Cr-Commit-Position: refs/heads/master@{#412409}
==========

[commit-bot: I haz the power, 2016-08-17 01:22:08]

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