Cleanup a TODO and other nits in SocketApiTest.

chrome/browser/extensions/api/socket/socket_apitest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/memory/ref_counted.h"
 #include "base/path_service.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/extensions/api/dns/mock_host_resolver_creator.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_function_test_utils.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_test_message_listener.h"
 #include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/extensions/application_launch.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "extensions/browser/api/dns/host_resolver_wrapper.h"
 #include "extensions/browser/api/socket/socket_api.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
 
+#if !defined(DISABLE_NACL)
+#include "base/command_line.h"
+#include "chrome/browser/ui/extensions/application_launch.h"
+#include "ppapi/shared_impl/ppapi_switches.h"
+#endif
+
 using extensions::Extension;
 
 namespace utils = extension_function_test_utils;
 
 namespace {
 
 const std::string kHostname = "127.0.0.1";
 const int kPort = 8888;
 
 class SocketApiTest : public ExtensionApiTest {
@@ skipping 28 matching lines @@
 // move these Pepper API tests there.
 class SocketPpapiTest : public SocketApiTest {
  public:
   SocketPpapiTest() {
   }
   virtual ~SocketPpapiTest() {
   }
 
   virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
     SocketApiTest::SetUpCommandLine(command_line);
-    // TODO(yzshen): It is better to use switches::kEnablePepperTesting.
-    // However, that requires adding a new DEPS entry. Considering that we are
-    // going to move the Pepper API tests to a new place, use a string literal
-    // for now.
-    command_line->AppendSwitch("enable-pepper-testing");
+    command_line->AppendSwitch(switches::kEnablePepperTesting);
   }
 
   virtual void SetUpOnMainThread() OVERRIDE {
     SocketApiTest::SetUpOnMainThread();
 
-    PathService::Get(chrome::DIR_GEN_TEST_DATA, &app_dir_);
-    app_dir_ = app_dir_.AppendASCII("ppapi/tests/extensions/socket/newlib");
+    ASSERT_TRUE(PathService::Get(chrome::DIR_GEN_TEST_DATA, &app_dir_));
+    app_dir_ = app_dir_.AppendASCII("ppapi")
+                       .AppendASCII("tests")
+                       .AppendASCII("extensions")
+                       .AppendASCII("socket")
+                       .AppendASCII("newlib");
   }
 
  protected:
   void LaunchTestingApp() {
     const Extension* extension = LoadExtension(app_dir_);
     ASSERT_TRUE(extension);
 
     AppLaunchParams params(browser()->profile(),
                            extension,
                            extensions::LAUNCH_CONTAINER_NONE,
                            NEW_WINDOW);
     params.command_line = *CommandLine::ForCurrentProcess();
     OpenApplication(params);
   }
 
  private:
   base::FilePath app_dir_;
 };
-#endif
+#endif  // !defined(DISABLE_NACL)
 
 }  // namespace
 
 IN_PROC_BROWSER_TEST_F(SocketApiTest, SocketUDPCreateGood) {
   scoped_refptr<extensions::SocketCreateFunction> socket_create_function(
       new extensions::SocketCreateFunction());
   scoped_refptr<Extension> empty_extension(utils::CreateEmptyExtension());
 
   socket_create_function->set_extension(empty_extension.get());
   socket_create_function->set_has_callback(true);
 
   scoped_ptr<base::Value> result(utils::RunFunctionAndReturnSingleResult(
       socket_create_function.get(), "[\"udp\"]", browser(), utils::NONE));
   ASSERT_EQ(base::Value::TYPE_DICTIONARY, result->GetType());
   base::DictionaryValue *value =
       static_cast<base::DictionaryValue*>(result.get());
-  int socketId = -1;
-  EXPECT_TRUE(value->GetInteger("socketId", &socketId));
-  EXPECT_TRUE(socketId > 0);
+  int socket_id = -1;
+  EXPECT_TRUE(value->GetInteger("socketId", &socket_id));
+  EXPECT_GT(socket_id, 0);
 }
 
 IN_PROC_BROWSER_TEST_F(SocketApiTest, SocketTCPCreateGood) {
   scoped_refptr<extensions::SocketCreateFunction> socket_create_function(
       new extensions::SocketCreateFunction());
   scoped_refptr<Extension> empty_extension(utils::CreateEmptyExtension());
 
   socket_create_function->set_extension(empty_extension.get());
   socket_create_function->set_has_callback(true);
 
   scoped_ptr<base::Value> result(utils::RunFunctionAndReturnSingleResult(
       socket_create_function.get(), "[\"tcp\"]", browser(), utils::NONE));
   ASSERT_EQ(base::Value::TYPE_DICTIONARY, result->GetType());
   base::DictionaryValue *value =
       static_cast<base::DictionaryValue*>(result.get());
-  int socketId = -1;
-  EXPECT_TRUE(value->GetInteger("socketId", &socketId));
-  ASSERT_TRUE(socketId > 0);
+  int socket_id = -1;
+  EXPECT_TRUE(value->GetInteger("socketId", &socket_id));
+  ASSERT_GT(socket_id, 0);
 }
 
 IN_PROC_BROWSER_TEST_F(SocketApiTest, GetNetworkList) {
   scoped_refptr<extensions::SocketGetNetworkListFunction> socket_function(
       new extensions::SocketGetNetworkListFunction());
   scoped_refptr<Extension> empty_extension(utils::CreateEmptyExtension());
 
   socket_function->set_extension(empty_extension.get());
   socket_function->set_has_callback(true);
 
   scoped_ptr<base::Value> result(utils::RunFunctionAndReturnSingleResult(
       socket_function.get(), "[]", browser(), utils::NONE));
   ASSERT_EQ(base::Value::TYPE_LIST, result->GetType());
 
   // If we're invoking socket tests, all we can confirm is that we have at
   // least one address, but not what it is.
   base::ListValue *value = static_cast<base::ListValue*>(result.get());
-  ASSERT_TRUE(value->GetSize() > 0);
+  ASSERT_GT(value->GetSize(), 0U);
 }
 
 IN_PROC_BROWSER_TEST_F(SocketApiTest, SocketUDPExtension) {
   scoped_ptr<net::SpawnedTestServer> test_server(
       new net::SpawnedTestServer(
           net::SpawnedTestServer::TYPE_UDP_ECHO,
           net::SpawnedTestServer::kLocalhost,
           base::FilePath(FILE_PATH_LITERAL("net/data"))));
   EXPECT_TRUE(test_server->Start());
 
   net::HostPortPair host_port_pair = test_server->host_port_pair();
   int port = host_port_pair.port();
-  ASSERT_TRUE(port > 0);
+  ASSERT_GT(port, 0);
 
   // Test that sendTo() is properly resolving hostnames.
   host_port_pair.set_host("LOCALhost");
 
   ResultCatcher catcher;
   catcher.RestrictToProfile(browser()->profile());
 
   ExtensionTestMessageListener listener("info_please", true);
 
   ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("socket/api")));
   EXPECT_TRUE(listener.WaitUntilSatisfied());
   listener.Reply(
       base::StringPrintf("udp:%s:%d", host_port_pair.host().c_str(), port));
 
   EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
 
 IN_PROC_BROWSER_TEST_F(SocketApiTest, SocketTCPExtension) {
   scoped_ptr<net::SpawnedTestServer> test_server(
       new net::SpawnedTestServer(
           net::SpawnedTestServer::TYPE_TCP_ECHO,
           net::SpawnedTestServer::kLocalhost,
           base::FilePath(FILE_PATH_LITERAL("net/data"))));
   EXPECT_TRUE(test_server->Start());
 
   net::HostPortPair host_port_pair = test_server->host_port_pair();
   int port = host_port_pair.port();
-  ASSERT_TRUE(port > 0);
+  ASSERT_GT(port, 0);
 
   // Test that connect() is properly resolving hostnames.
   host_port_pair.set_host("lOcAlHoSt");
 
   ResultCatcher catcher;
   catcher.RestrictToProfile(browser()->profile());
 
   ExtensionTestMessageListener listener("info_please", true);
 
   ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("socket/api")));
@@ skipping 53 matching lines @@
 IN_PROC_BROWSER_TEST_F(SocketPpapiTest, MAYBE_UDP) {
   scoped_ptr<net::SpawnedTestServer> test_server(
       new net::SpawnedTestServer(
           net::SpawnedTestServer::TYPE_UDP_ECHO,
           net::SpawnedTestServer::kLocalhost,
           base::FilePath(FILE_PATH_LITERAL("net/data"))));
   EXPECT_TRUE(test_server->Start());
 
   net::HostPortPair host_port_pair = test_server->host_port_pair();
   int port = host_port_pair.port();
-  ASSERT_TRUE(port > 0);
+  ASSERT_GT(port, 0);
 
   // Test that sendTo() is properly resolving hostnames.
   host_port_pair.set_host("LOCALhost");
 
   ResultCatcher catcher;
   catcher.RestrictToProfile(browser()->profile());
 
   ExtensionTestMessageListener listener("info_please", true);
 
   LaunchTestingApp();
@@ skipping 14 matching lines @@
 IN_PROC_BROWSER_TEST_F(SocketPpapiTest, MAYBE_TCP) {
   scoped_ptr<net::SpawnedTestServer> test_server(
       new net::SpawnedTestServer(
           net::SpawnedTestServer::TYPE_TCP_ECHO,
           net::SpawnedTestServer::kLocalhost,
           base::FilePath(FILE_PATH_LITERAL("net/data"))));
   EXPECT_TRUE(test_server->Start());
 
   net::HostPortPair host_port_pair = test_server->host_port_pair();
   int port = host_port_pair.port();
-  ASSERT_TRUE(port > 0);
+  ASSERT_GT(port, 0);
 
   // Test that connect() is properly resolving hostnames.
   host_port_pair.set_host("lOcAlHoSt");
 
   ResultCatcher catcher;
   catcher.RestrictToProfile(browser()->profile());
 
   ExtensionTestMessageListener listener("info_please", true);
 
   LaunchTestingApp();
@@ skipping 35 matching lines @@
 
   LaunchTestingApp();
 
   EXPECT_TRUE(listener.WaitUntilSatisfied());
   listener.Reply(
       base::StringPrintf("multicast:%s:%d", kHostname.c_str(), kPort));
 
   EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
 #endif