[DevTools] A setting for TCP discovery enablement

chrome/browser/devtools/device/devtools_android_bridge_browsertest.cc

Index: chrome/browser/devtools/device/devtools_android_bridge_browsertest.cc
diff --git a/chrome/browser/devtools/device/devtools_android_bridge_browsertest.cc b/chrome/browser/devtools/device/devtools_android_bridge_browsertest.cc
index 12717117382c697034c01912a6527956a7023a7c..fdfd5a4b01fdc8ecb55d2553b4950d8b3b4c6f2d 100644
--- a/chrome/browser/devtools/device/devtools_android_bridge_browsertest.cc
+++ b/chrome/browser/devtools/device/devtools_android_bridge_browsertest.cc
@@ -2,6 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <algorithm>
+#include <array>
+
 #include "base/values.h"
 #include "chrome/browser/devtools/device/devtools_android_bridge.h"
 #include "chrome/browser/devtools/device/tcp_device_provider.h"
@@ -23,11 +26,24 @@ static void assign_from_callback(scoped_refptr<TCPDeviceProvider>* store,
   *store = value;
 }
 
-IN_PROC_BROWSER_TEST_F(DevToolsAndroidBridgeTest, UpdatesTargetDiscovery) {
+static std::string all_targets_string(

[dgozman, 2016/08/29 22:28:21]

AllTargetsString

[eostroukhov, 2016/08/29 23:19:36]

Done.

+    scoped_refptr<TCPDeviceProvider> provider) {
+  std::set<std::string> actual;
+  for (const net::HostPortPair& hostport : provider->get_targets_for_test())
+    actual.insert(hostport.ToString());
+  std::ostringstream result;
+  std::copy(actual.begin(), actual.end(),
+            std::ostream_iterator<std::string>(result, ", "));
+  std::string result_string = result.str();
+  return result_string.substr(0, result_string.length() - 2);
+}
+
+IN_PROC_BROWSER_TEST_F(DevToolsAndroidBridgeTest, DiscoveryListChanges) {
   Profile* profile = browser()->profile();
 
   PrefService* service = profile->GetPrefs();
-  service->ClearPref(prefs::kDevToolsTargetDiscoveryConfig);
+  service->ClearPref(prefs::kDevToolsTCPDiscoveryConfig);
+  service->SetBoolean(prefs::kDevToolsDiscoverTCPTargetsEnabled, true);
 
   DevToolsAndroidBridge* bridge =
       DevToolsAndroidBridge::Factory::GetForProfile(profile);
@@ -38,25 +54,24 @@ IN_PROC_BROWSER_TEST_F(DevToolsAndroidBridgeTest, UpdatesTargetDiscovery) {
       base::Bind(assign_from_callback, &provider, &called));
 
   EXPECT_LT(0, called);
-  EXPECT_EQ(nullptr, provider);
+  EXPECT_NE(nullptr, provider);
+
+  EXPECT_STREQ("localhost:9222, localhost:9229",
+               all_targets_string(provider).c_str());
 
   int invocations = called;
   base::ListValue list;
   list.AppendString("somehost:2000");
 
-  service->Set(prefs::kDevToolsTargetDiscoveryConfig, list);
+  service->Set(prefs::kDevToolsTCPDiscoveryConfig, list);
 
   EXPECT_LT(invocations, called);
   EXPECT_NE(nullptr, provider);
-  std::set<net::HostPortPair> pairs = provider->get_targets_for_test();
-  EXPECT_EQ(1UL, pairs.size());
-  net::HostPortPair pair = *pairs.begin();
-  EXPECT_EQ(2000, pair.port());
-  EXPECT_EQ("somehost", pair.HostForURL());
+  EXPECT_STREQ("somehost:2000", all_targets_string(provider).c_str());
 
   invocations = called;
   list.Clear();
-  service->Set(prefs::kDevToolsTargetDiscoveryConfig, list);
+  service->Set(prefs::kDevToolsTCPDiscoveryConfig, list);
 
   EXPECT_LT(invocations, called);
   EXPECT_EQ(nullptr, provider);
@@ -67,13 +82,71 @@ IN_PROC_BROWSER_TEST_F(DevToolsAndroidBridgeTest, UpdatesTargetDiscovery) {
   list.AppendString("<not really a good address.");
   list.AppendString("d:3");
   list.AppendString("c:2");
-  service->Set(prefs::kDevToolsTargetDiscoveryConfig, list);
+  service->Set(prefs::kDevToolsTCPDiscoveryConfig, list);
 
   EXPECT_LT(invocations, called);
   EXPECT_NE(nullptr, provider);
-  pairs = provider->get_targets_for_test();
-  EXPECT_EQ(3UL, pairs.size());
-  for (const net::HostPortPair pair : pairs) {
-    EXPECT_EQ(pair.port(), pair.HostForURL()[0] - 'a');
+  EXPECT_STREQ("b:1, c:2, d:3", all_targets_string(provider).c_str());
+}
+
+
+IN_PROC_BROWSER_TEST_F(DevToolsAndroidBridgeTest, DefaultValues) {
+  Profile* profile = browser()->profile();
+
+  PrefService* service = profile->GetPrefs();
+  DevToolsAndroidBridge::Factory::GetForProfile(profile);
+  service->ClearPref(prefs::kDevToolsDiscoverTCPTargetsEnabled);
+  service->ClearPref(prefs::kDevToolsTCPDiscoveryConfig);
+
+  const base::ListValue* targets =
+    service->GetList(prefs::kDevToolsTCPDiscoveryConfig);
+  EXPECT_NE(nullptr, targets);
+  EXPECT_EQ(2ul, targets->GetSize());
+
+  int matched_mask = 0;
+  std::array<std::string, 2> EXPECTED_TARGETS =

[dgozman, 2016/08/29 22:28:21]

expected_targets

[eostroukhov, 2016/08/29 23:19:36]

Done (by removing this line - no longer needed).

+      {{"localhost:9222", "localhost:9229"}};
+
+  for (size_t i = 0; i < targets->GetSize(); i++) {
+    std::string target;
+    EXPECT_TRUE(targets->GetString(i, &target));
+    for (size_t j = 0; j < EXPECTED_TARGETS.size(); j++) {
+      if (target == EXPECTED_TARGETS[j]) {
+        matched_mask |= 1 << j;
+      }
+    }
   }
+
+  EXPECT_FALSE(service->GetBoolean(prefs::kDevToolsDiscoverTCPTargetsEnabled));
+
+  EXPECT_EQ(3, matched_mask);

[dgozman, 2016/08/29 22:28:21]

I still don't understand why not just sort them?

[eostroukhov, 2016/08/29 23:19:36]

At some point I was outputting indexes... Fixed.

+}
+
+
+IN_PROC_BROWSER_TEST_F(DevToolsAndroidBridgeTest, TCPEnableChange) {
+  Profile* profile = browser()->profile();
+
+  PrefService* service = profile->GetPrefs();
+  service->ClearPref(prefs::kDevToolsTCPDiscoveryConfig);
+  service->ClearPref(prefs::kDevToolsDiscoverTCPTargetsEnabled);
+
+  DevToolsAndroidBridge* bridge =
+      DevToolsAndroidBridge::Factory::GetForProfile(profile);
+
+  scoped_refptr<TCPDeviceProvider> provider;
+  int called = 0;
+  bridge->set_tcp_provider_callback_for_test(
+      base::Bind(assign_from_callback, &provider, &called));
+
+  EXPECT_EQ(nullptr, provider);
+
+  service->SetBoolean(prefs::kDevToolsDiscoverTCPTargetsEnabled, true);
+
+  EXPECT_NE(nullptr, provider);
+  EXPECT_STREQ("localhost:9222, localhost:9229",
+               all_targets_string(provider).c_str());
+
+  service->SetBoolean(prefs::kDevToolsDiscoverTCPTargetsEnabled, false);
+
+  EXPECT_EQ(nullptr, provider);
 }