Changed some API names. Wrapped existing UDPClientSocket. Added one unit test...

Changed some API names. Wrapped existing UDPClientSocket. Added one unit test for controller. The socket API itself isn't tested for two reasons: (1) this is an interim checkin, where Part 2 will add UDP receiving, and that will likely involve some API changes, so I didn't bother constructing a whole extension built on a temporary API; and (2) the wrapper itself is so thin around the existing UDPClientSocket API that a unit test of the wrapper wouldn't accomplish much.

BUG=none
TEST=see above.


Review URL: http://codereview.chromium.org/8743017

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@113069 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/socket_api.cc → chrome/browser/extensions/api/socket/socket_api.cc

@@ -2,16 +2,22 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/extensions/socket_api.h"
+#include "chrome/browser/extensions/api/socket/socket_api.h"
 
 #include "base/bind.h"
 #include "base/values.h"
+#include "chrome/browser/extensions/api/socket/socket_api_controller.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/browser_thread.h"
 
 using content::BrowserThread;
 
 namespace extensions {
 
+const char kBytesWrittenKey[] = "bytesWritten";
+const char kSocketIdKey[] = "socketId";
+const char kUDPSocketType[] = "udp";
+
 SocketCreateFunction::SocketCreateFunction() {
 }
 
@@ -22,13 +28,12 @@ bool SocketCreateFunction::RunImpl() {
   std::string socket_type;
   EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &socket_type));
 
-  // TODO(miket): this constitutes a second form of truth as to the
-  // enum validity. But our unit-test framework skips the enum
-  // validation. So in order to get an invalid-enum test to pass, we
-  // need duplicative value-checking. Too bad. Fix this if/when the
-  // argument validation code is moved to C++ rather than its current
-  // JavaScript form.
-  if (socket_type != "udp") {
+  // TODO(miket): this constitutes a second form of truth as to the enum
+  // validity. But our unit-test framework skips the enum validation. So in
+  // order to get an invalid-enum test to pass, we need duplicative
+  // value-checking. Too bad. Fix this if/when the argument validation code is
+  // moved to C++ rather than its current JavaScript form.
+  if (socket_type != kUDPSocketType) {
     return false;
   }
 
@@ -42,8 +47,13 @@ bool SocketCreateFunction::RunImpl() {
 void SocketCreateFunction::WorkOnIOThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DictionaryValue* result = new DictionaryValue();
-  result->SetInteger("socketId", 42);
+  SocketController* controller = SocketController::GetInstance();
+
+  int socket_id = controller->CreateUdp(profile(), extension_id(),
+                                       source_url());
+  result->SetInteger(kSocketIdKey, socket_id);
   result_.reset(result);
+
   bool rv = BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&SocketCreateFunction::RespondOnUIThread, this));
@@ -55,6 +65,38 @@ void SocketCreateFunction::RespondOnUIThread() {
   SendResponse(true);
 }
 
+SocketDestroyFunction::SocketDestroyFunction() {
+}
+
+SocketDestroyFunction::~SocketDestroyFunction() {
+}
+
+bool SocketDestroyFunction::RunImpl() {
+  EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &socket_id_));
+
+  bool rv = BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&SocketDestroyFunction::WorkOnIOThread, this));
+  DCHECK(rv);
+  return true;
+}
+
+void SocketDestroyFunction::WorkOnIOThread() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  SocketController* controller = SocketController::GetInstance();
+  controller->DestroyUdp(socket_id_);
+
+  bool rv = BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&SocketDestroyFunction::RespondOnUIThread, this));
+  DCHECK(rv);
+}
+
+void SocketDestroyFunction::RespondOnUIThread() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  SendResponse(true);
+}
+
 SocketConnectFunction::SocketConnectFunction() {
 }
 
@@ -62,8 +104,9 @@ SocketConnectFunction::~SocketConnectFunction() {
 }
 
 bool SocketConnectFunction::RunImpl() {
-  std::string socket_type;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &socket_type));
+  EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &socket_id_));
+  EXTENSION_FUNCTION_VALIDATE(args_->GetString(1, &address_));
+  EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(2, &port_));
 
   bool rv = BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
@@ -74,7 +117,11 @@ bool SocketConnectFunction::RunImpl() {
 
 void SocketConnectFunction::WorkOnIOThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  result_.reset(Value::CreateIntegerValue(4));
+
+  SocketController* controller = SocketController::GetInstance();
+  bool result = controller->ConnectUdp(socket_id_, address_, port_);
+  result_.reset(Value::CreateBooleanValue(result));
+
   bool rv = BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&SocketConnectFunction::RespondOnUIThread, this));
@@ -86,64 +133,73 @@ void SocketConnectFunction::RespondOnUIThread() {
   SendResponse(true);
 }
 
-SocketDisconnectFunction::SocketDisconnectFunction() {
+SocketCloseFunction::SocketCloseFunction() {
 }
 
-SocketDisconnectFunction::~SocketDisconnectFunction() {
+SocketCloseFunction::~SocketCloseFunction() {
 }
 
-bool SocketDisconnectFunction::RunImpl() {
-  std::string socket_type;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &socket_type));
+bool SocketCloseFunction::RunImpl() {
+  EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &socket_id_));
 
   bool rv = BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
-      base::Bind(&SocketDisconnectFunction::WorkOnIOThread, this));
+      base::Bind(&SocketCloseFunction::WorkOnIOThread, this));
   DCHECK(rv);
   return true;
 }
 
-void SocketDisconnectFunction::WorkOnIOThread() {
+void SocketCloseFunction::WorkOnIOThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  result_.reset(Value::CreateIntegerValue(4));
+
+  SocketController* controller = SocketController::GetInstance();
+  controller->CloseUdp(socket_id_);
+  result_.reset(Value::CreateNullValue());
+
   bool rv = BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
-      base::Bind(&SocketDisconnectFunction::RespondOnUIThread, this));
+      base::Bind(&SocketCloseFunction::RespondOnUIThread, this));
   DCHECK(rv);
 }
 
-void SocketDisconnectFunction::RespondOnUIThread() {
+void SocketCloseFunction::RespondOnUIThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   SendResponse(true);
 }
 
-SocketSendFunction::SocketSendFunction() {
+SocketWriteFunction::SocketWriteFunction() {
 }
 
-SocketSendFunction::~SocketSendFunction() {
+SocketWriteFunction::~SocketWriteFunction() {
 }
 
-bool SocketSendFunction::RunImpl() {
-  std::string socket_type;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &socket_type));
+bool SocketWriteFunction::RunImpl() {
+  EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &socket_id_));
+  EXTENSION_FUNCTION_VALIDATE(args_->GetString(1, &message_));
 
   bool rv = BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
-      base::Bind(&SocketSendFunction::WorkOnIOThread, this));
+      base::Bind(&SocketWriteFunction::WorkOnIOThread, this));
   DCHECK(rv);
   return true;
 }
 
-void SocketSendFunction::WorkOnIOThread() {
+void SocketWriteFunction::WorkOnIOThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  result_.reset(Value::CreateIntegerValue(4));
+
+  SocketController* controller = SocketController::GetInstance();
+  int bytesWritten = controller->WriteUdp(socket_id_, message_);
+
+  DictionaryValue* result = new DictionaryValue();
+  result->SetInteger(kBytesWrittenKey, bytesWritten);
+  result_.reset(result);
   bool rv = BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
-      base::Bind(&SocketSendFunction::RespondOnUIThread, this));
+      base::Bind(&SocketWriteFunction::RespondOnUIThread, this));
   DCHECK(rv);
 }
 
-void SocketSendFunction::RespondOnUIThread() {
+void SocketWriteFunction::RespondOnUIThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   SendResponse(true);
 }

chrome/browser/extensions/socket_api.h → chrome/browser/extensions/api/socket/socket_api.h

@@ -2,14 +2,26 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_EXTENSIONS_SOCKET_API_H_
-#define CHROME_BROWSER_EXTENSIONS_SOCKET_API_H_
+#ifndef CHROME_BROWSER_EXTENSIONS_API_SOCKET_SOCKET_API_H_
+#define CHROME_BROWSER_EXTENSIONS_API_SOCKET_SOCKET_API_H_
 #pragma once
 
 #include "chrome/browser/extensions/extension_function.h"
 
+#include <string>
+
 namespace extensions {
 
+extern const char kBytesWrittenKey[];
+extern const char kSocketIdKey[];
+extern const char kUdpSocketType[];
+
+// Many of these socket functions are synchronous in the sense that
+// they don't involve blocking operations, but we've made them all
+// AsyncExtensionFunctions because the underlying UDPClientSocket
+// library wants all operations to happen on the same thread as the
+// one that created the socket. Too bad.
+
 class SocketCreateFunction : public AsyncExtensionFunction {
  public:
   SocketCreateFunction();
@@ -23,6 +35,22 @@ class SocketCreateFunction : public AsyncExtensionFunction {
   DECLARE_EXTENSION_FUNCTION_NAME("experimental.socket.create")
 };
 
+class SocketDestroyFunction : public AsyncExtensionFunction {
+ public:
+  SocketDestroyFunction();
+  virtual ~SocketDestroyFunction();
+  virtual bool RunImpl() OVERRIDE;
+
+ protected:
+  void WorkOnIOThread();
+  void RespondOnUIThread();
+
+ private:
+  int socket_id_;
+
+  DECLARE_EXTENSION_FUNCTION_NAME("experimental.socket.destroy")
+};
+
 class SocketConnectFunction : public AsyncExtensionFunction {
  public:
   SocketConnectFunction();
@@ -33,35 +61,46 @@ class SocketConnectFunction : public AsyncExtensionFunction {
   void WorkOnIOThread();
   void RespondOnUIThread();
 
+ private:
+  int socket_id_;
+  std::string address_;
+  int port_;
+
   DECLARE_EXTENSION_FUNCTION_NAME("experimental.socket.connect")
 };
 
-class SocketDisconnectFunction : public AsyncExtensionFunction {
+class SocketCloseFunction : public AsyncExtensionFunction {
  public:
-  SocketDisconnectFunction();
-  virtual ~SocketDisconnectFunction();
+  SocketCloseFunction();
+  virtual ~SocketCloseFunction();
   virtual bool RunImpl() OVERRIDE;
 
  protected:
   void WorkOnIOThread();
   void RespondOnUIThread();
 
-  DECLARE_EXTENSION_FUNCTION_NAME("experimental.socket.disconnect")
+ private:
+  int socket_id_;
+
+  DECLARE_EXTENSION_FUNCTION_NAME("experimental.socket.close")
 };
 
-class SocketSendFunction : public AsyncExtensionFunction {
+class SocketWriteFunction : public AsyncExtensionFunction {
  public:
-  SocketSendFunction();
-  virtual ~SocketSendFunction();
+  SocketWriteFunction();
+  virtual ~SocketWriteFunction();
   virtual bool RunImpl() OVERRIDE;
 
  protected:
   void WorkOnIOThread();
   void RespondOnUIThread();
 
-  DECLARE_EXTENSION_FUNCTION_NAME("experimental.socket.send")
+  int socket_id_;
+  std::string message_;
+
+  DECLARE_EXTENSION_FUNCTION_NAME("experimental.socket.write")
 };
 
 }  // namespace extensions
 
-#endif  // CHROME_BROWSER_EXTENSIONS_SOCKET_API_H_
+#endif  // CHROME_BROWSER_EXTENSIONS_API_SOCKET_SOCKET_API_H_

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

+// Copyright (c) 2011 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/json/json_writer.h"
+#include "base/stl_util.h"
+#include "base/values.h"
+#include "chrome/browser/extensions/api/socket/socket_api_controller.h"
+#include "chrome/browser/profiles/profile.h"
+#include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
+#include "net/base/rand_callback.h"
+#include "net/udp/datagram_socket.h"
+#include "net/udp/udp_client_socket.h"
+#include "net/udp/udp_socket.h"
+
+using namespace net;
+
+namespace extensions {
+
+// A Socket wraps a low-level socket and includes housekeeping information that
+// we need to manage it in the context of an extension.
+class Socket {
+ public:
+  explicit Socket(const Profile* profile, const std::string& src_extension_id,
+                  const GURL& src_url);
+  ~Socket();
+
+  bool Connect(const net::IPEndPoint& ip_end_point);
+  void Close();
+  int Write(const std::string message);
+
+ private:
+  // TODO(miket): this metadata will enable us to pass events back to the
+  // extension that created this Socket.
+  const Profile* profile_;
+  int id_;
+  std::string src_extension_id_;
+  GURL src_url_;
+
+  net::UDPClientSocket* udp_client_socket_;
+  bool is_connected_;
+  net::OldCompletionCallbackImpl<Socket> io_callback_;
+
+  // A callback required by UDPClientSocket::Write().
+  void OnIOComplete(int result);
+};
+
+Socket::Socket(const Profile* profile, const std::string& src_extension_id,
+               const GURL& src_url)
+    : profile_(profile),
+      src_extension_id_(src_extension_id),
+      src_url_(src_url),
+      udp_client_socket_(new UDPClientSocket(
+          DatagramSocket::DEFAULT_BIND,
+          RandIntCallback(),
+          NULL,
+          NetLog::Source())),
+      is_connected_(false),
+      ALLOW_THIS_IN_INITIALIZER_LIST(
+          io_callback_(this, &Socket::OnIOComplete)) {}
+
+Socket::~Socket() {
+  if (is_connected_) {
+    Close();
+  }
+}
+
+void Socket::OnIOComplete(int result) {
+  // We don't need to do anything.
+}
+
+bool Socket::Connect(const net::IPEndPoint& ip_end_point) {
+  is_connected_ = udp_client_socket_->Connect(ip_end_point) == net::OK;
+  return is_connected_;
+}
+
+void Socket::Close() {
+  is_connected_ = false;
+  udp_client_socket_->Close();
+}
+
+int Socket::Write(const std::string message) {
+  int length = message.length();
+  scoped_refptr<StringIOBuffer> io_buffer(new StringIOBuffer(message));
+  scoped_refptr<DrainableIOBuffer> buffer(
+      new DrainableIOBuffer(io_buffer, length));
+
+  int bytes_sent = 0;
+  while (buffer->BytesRemaining()) {
+    int rv = udp_client_socket_->Write(buffer,
+                                       buffer->BytesRemaining(),
+                                       &io_callback_);
+    if (rv <= 0) {
+      // We pass all errors, including ERROR_IO_PENDING, back to the caller.
+      return bytes_sent > 0 ? bytes_sent : rv;
+    }
+    bytes_sent += rv;
+    buffer->DidConsume(rv);
+  }
+  return bytes_sent;
+}
+
+SocketController* SocketController::GetInstance() {
+  return Singleton<SocketController>::get();
+}
+
+SocketController::SocketController() : next_socket_id_(1) {}
+
+SocketController::~SocketController() {
+  STLDeleteValues(&socket_map_);
+}
+
+Socket* SocketController::GetSocket(int socket_id) {
+  // TODO(miket): we should verify that the extension asking for the
+  // socket is the same one that created it.
+  SocketMap::iterator i = socket_map_.find(socket_id);
+  if (i != socket_map_.end())
+    return i->second;
+  return NULL;
+}
+
+int SocketController::CreateUdp(const Profile* profile,
+                                const std::string& extension_id,
+                                const GURL& src_url) {
+  Socket* socket = new Socket(profile, extension_id, src_url);
+  CHECK(socket);
+  socket_map_[next_socket_id_] = socket;
+  return next_socket_id_++;
+}
+
+bool SocketController::DestroyUdp(int socket_id) {
+  Socket* socket = GetSocket(socket_id);
+  if (!socket)
+    return false;
+  delete socket;
+  socket_map_.erase(socket_id);
+  return true;
+}
+
+// TODO(miket): it *might* be nice to be able to resolve DNS. I am not putting
+// in interesting error reporting for this method because we clearly can't
+// leave experimental without DNS resolution.
+//
+// static
+bool SocketController::CreateIPEndPoint(const std::string address, int port,
+                                        net::IPEndPoint* ip_end_point) {
+  net::IPAddressNumber ip_number;
+  bool rv = net::ParseIPLiteralToNumber(address, &ip_number);
+  if (!rv)
+    return false;
+  *ip_end_point = net::IPEndPoint(ip_number, port);
+  return true;
+}
+
+bool SocketController::ConnectUdp(int socket_id, const std::string address,
+                                  int port) {
+  Socket* socket = GetSocket(socket_id);
+  if (!socket)
+    return false;
+  net::IPEndPoint ip_end_point;
+  if (!CreateIPEndPoint(address, port, &ip_end_point))
+    return false;
+  return socket->Connect(ip_end_point);
+}
+
+void SocketController::CloseUdp(int socket_id) {
+  Socket* socket = GetSocket(socket_id);
+  if (socket)
+    socket->Close();
+}
+
+int SocketController::WriteUdp(int socket_id, const std::string message) {
+  Socket* socket = GetSocket(socket_id);
+  if (!socket) {
+    return -1;
+  }
+  return socket->Write(message);
+}
+
+}  // namespace extensions

chrome/browser/extensions/api/socket/socket_api_controller.h

+// Copyright (c) 2011 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.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_API_SOCKET_SOCKET_API_CONTROLLER_H_
+#define CHROME_BROWSER_EXTENSIONS_API_SOCKET_SOCKET_API_CONTROLLER_H_
+#pragma once
+
+#include <string>
+#include <map>
+
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/singleton.h"
+#include "googleurl/src/gurl.h"
+#include "net/base/completion_callback.h"
+
+class Profile;
+
+namespace base {
+class ListValue;
+class Value;
+}
+
+namespace net {
+class UDPClientSocket;
+class IPEndPoint;
+}
+
+namespace extensions {
+
+class Socket;
+
+// The SocketController singleton keeps track of all our Sockets, and provides
+// a convenient set of methods to manipulate them.
+class SocketController {
+ public:
+  static SocketController* GetInstance();
+
+  SocketController();
+  virtual ~SocketController();
+
+  // Create/Destroy are a pair. They represent the allocation and deallocation
+  // of the Socket object in memory.
+  //
+  // TODO(miket): we currently require the app developer to remember to call
+  // Destroy, which is a buzzkill in JavaScript. I believe that to solve this,
+  // we'll have to associate each Socket with a creator extension, and then
+  // clean up when the extension goes out of scope. As the API is defined
+  // today, we're exposing only primitive socketIds to JavaScript, which seems
+  // to imply that we won't be able to garbage-collect when individual sockets
+  // "go out of scope" (in quotes because they never do).
+  int CreateUdp(const Profile* profile, const std::string& extension_id,
+                const GURL& src_url);
+  bool DestroyUdp(int socket_id);
+
+  // Connect, Close, Read, and Write map to the equivalent methods in
+  // UDPClientSocket.
+  //
+  // TODO(miket): Implement Read.
+  bool ConnectUdp(int socket_id, const std::string address, int port);
+  void CloseUdp(int socket_id);
+  int WriteUdp(int socket_id, const std::string msg);
+
+  // Converts a string IP address and integer port into a format that
+  // UDPClientSocket can deal with. Public so test harness can use it.
+  static bool CreateIPEndPoint(const std::string address, int port,
+                               net::IPEndPoint* ip_end_point);
+
+ private:
+  int next_socket_id_;
+  typedef std::map<int, Socket*> SocketMap;
+  SocketMap socket_map_;
+
+  // Convenience method for accessing SocketMap.
+  Socket* GetSocket(int socket_id);
+
+  friend struct DefaultSingletonTraits<SocketController>;
+
+  DISALLOW_COPY_AND_ASSIGN(SocketController);
+};
+
+}  // namespace extensions
+
+#endif  // CHROME_BROWSER_EXTENSIONS_API_SOCKET_SOCKET_API_CONTROLLER_H_

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

+// Copyright (c) 2011 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 "testing/gtest/include/gtest/gtest.h"
+
+#include "base/values.h"
+#include "chrome/browser/extensions/api/socket/socket_api.h"
+#include "chrome/browser/extensions/api/socket/socket_api_controller.h"
+
+namespace extensions {
+
+class SocketApiControllerTest : public testing::Test {
+};
+
+TEST_F(SocketApiControllerTest, TestSocketControllerLifetime) {
+  // We want to make sure that killing the controller while a bunch of
+  // sockets are alive doesn't crash.
+  SocketController* controller = SocketController::GetInstance();
+
+  // Create some sockets but don't do anything with them.
+  Profile* profile = NULL;
+  const std::string extension_id("xxxxxxxxx");
+  const GURL url;
+  for (int i = 0; i < 10; ++i) {
+    int socket_id = controller->CreateUdp(profile, extension_id, url);
+    ASSERT_TRUE(socket_id != 0);
+  }
+
+  // Create some more sockets and connect them. Note that because this is
+  // UDP, we can happily "connect" a UDP socket without anyone listening.
+  const int kPort = 38888;
+  const std::string address("127.0.0.1");
+  for (int i = 0; i < 10; ++i) {
+    int socket_id = controller->CreateUdp(profile, extension_id, url);
+    ASSERT_TRUE(socket_id != 0);
+    ASSERT_TRUE(controller->ConnectUdp(socket_id, address, kPort));
+  }
+
+  // At this point, we're done, and we're relying on the RAE mechanism
+  // of the Singleton class to delete the controller at process exit.
+  // We'd have to jump through some icky hoops to turn off RAE and
+  // manually delete in this test method, so we'll instead take it on
+  // faith that the singleton will indeed delete itself, and that if
+  // we had any heap management problems in the controller, they'd
+  // show up later in this test process. I (miket) hereby confirm that
+  // I manually added a temporary double-free in the controller
+  // destructor and verified that the unit_tests process segfaulted.
+}
+
+}  // namespace extensions

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

@@ -5,7 +5,7 @@
 #include "base/command_line.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_function_test_utils.h"
-#include "chrome/browser/extensions/socket_api.h"
+#include "chrome/browser/extensions/api/socket/socket_api.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -15,15 +15,23 @@ using namespace extension_function_test_utils;
 namespace {
 
 class SocketApiTest : public InProcessBrowserTest {
+ public:
+  virtual void SetUpMyCommandLine() {
+    DoCommandLineSetup();
+  }
+
+  // Exposed as static method so that SocketExtension can easily get to it.
+  static void DoCommandLineSetup() {
+    CommandLine::ForCurrentProcess()->AppendSwitch(
+        switches::kEnableExperimentalExtensionApis);
+    CommandLine::ForCurrentProcess()->AppendSwitch(
+        switches::kEnablePlatformApps);
+  }
 };
 
 }
 
 IN_PROC_BROWSER_TEST_F(SocketApiTest, SocketCreateGood) {
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableExperimentalExtensionApis);
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnablePlatformApps);
   scoped_refptr<extensions::SocketCreateFunction> socket_create_function(
       new extensions::SocketCreateFunction());
   scoped_refptr<Extension> empty_extension(CreateEmptyExtension());
@@ -37,14 +45,10 @@ IN_PROC_BROWSER_TEST_F(SocketApiTest, SocketCreateGood) {
   DictionaryValue *value = static_cast<DictionaryValue*>(result.get());
   int socketId = -1;
   EXPECT_TRUE(value->GetInteger("socketId", &socketId));
-  EXPECT_EQ(42, socketId);
+  EXPECT_TRUE(socketId > 0);
 }
 
 IN_PROC_BROWSER_TEST_F(SocketApiTest, SocketCreateBad) {
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableExperimentalExtensionApis);
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnablePlatformApps);
   scoped_refptr<extensions::SocketCreateFunction> socket_create_function(
       new extensions::SocketCreateFunction());
   scoped_refptr<Extension> empty_extension(CreateEmptyExtension());
@@ -59,10 +63,6 @@ IN_PROC_BROWSER_TEST_F(SocketApiTest, SocketCreateBad) {
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, SocketExtension) {
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableExperimentalExtensionApis);
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnablePlatformApps);
-
+  SocketApiTest::DoCommandLineSetup();
   ASSERT_TRUE(RunExtensionTest("socket/api")) << message_;
 }

chrome/browser/extensions/extension_function_dispatcher.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/bookmarks/bookmark_extension_api.h"
 #include "chrome/browser/bookmarks/bookmark_manager_extension_api.h"
 #include "chrome/browser/download/download_extension_api.h"
+#include "chrome/browser/extensions/api/socket/socket_api.h"
 #include "chrome/browser/extensions/execute_code_in_tab_function.h"
 #include "chrome/browser/extensions/extension_app_api.h"
 #include "chrome/browser/extensions/extension_browser_actions_api.h"
@@ -51,7 +52,6 @@
 #include "chrome/browser/extensions/extensions_quota_service.h"
 #include "chrome/browser/extensions/process_map.h"
 #include "chrome/browser/extensions/settings/settings_api.h"
-#include "chrome/browser/extensions/socket_api.h"
 #include "chrome/browser/external_protocol/external_protocol_handler.h"
 #include "chrome/browser/history/history_extension_api.h"
 #include "chrome/browser/history/top_sites_extension_api.h"
@@ -472,9 +472,10 @@ void FactoryRegistry::ResetFunctions() {
 
   // Sockets
   RegisterFunction<extensions::SocketCreateFunction>();
+  RegisterFunction<extensions::SocketDestroyFunction>();
   RegisterFunction<extensions::SocketConnectFunction>();
-  RegisterFunction<extensions::SocketDisconnectFunction>();
-  RegisterFunction<extensions::SocketSendFunction>();
+  RegisterFunction<extensions::SocketCloseFunction>();
+  RegisterFunction<extensions::SocketWriteFunction>();
 }
 
 void FactoryRegistry::GetAllNames(std::vector<std::string>* names) {

chrome/chrome_browser.gypi

@@ -830,8 +830,6 @@
         'browser/component_updater/component_updater_configurator.h',
         'browser/component_updater/component_unpacker.cc',
         'browser/component_updater/component_unpacker.h',
-        'browser/extensions/socket_api.cc',
-        'browser/extensions/socket_api.h',
         'browser/component_updater/component_updater_service.cc',
         'browser/component_updater/component_updater_service.h',
         'browser/component_updater/flash_component_installer.h',
@@ -931,6 +929,10 @@
         'browser/enumerate_modules_model_win.h',
         'browser/event_disposition.cc',
         'browser/event_disposition.h',
+        'browser/extensions/api/socket/socket_api.cc',
+        'browser/extensions/api/socket/socket_api.h',
+        'browser/extensions/api/socket/socket_api_controller.cc',
+        'browser/extensions/api/socket/socket_api_controller.h',
         'browser/extensions/app_notification.cc',
         'browser/extensions/app_notification.h',
         'browser/extensions/app_notification_manager.cc',

chrome/chrome_tests.gypi

@@ -1361,6 +1361,7 @@
         'browser/enumerate_modules_model_unittest_win.cc',
         'browser/event_disposition.cc',
         'browser/event_disposition.h',
+        'browser/extensions/api/socket/socket_api_controller_unittest.cc',
         'browser/extensions/app_notification_manager_sync_unittest.cc',
         'browser/extensions/app_notification_manager_unittest.cc',
         'browser/extensions/app_notification_storage_unittest.cc',
@@ -2475,6 +2476,7 @@
         'browser/errorpage_browsertest.cc',
         'browser/extensions/alert_apitest.cc',
         'browser/extensions/all_urls_apitest.cc',
+        'browser/extensions/api/socket/socket_apitest.cc',
         'browser/extensions/app_background_page_apitest.cc',
         'browser/extensions/app_notification_browsertest.cc',
         'browser/extensions/app_process_apitest.cc',
@@ -2567,7 +2569,6 @@
         'browser/extensions/page_action_apitest.cc',
         'browser/extensions/permissions_apitest.cc',
         'browser/extensions/platform_app_browsertest.cc',
-        'browser/extensions/socket_apitest.cc',
         'browser/extensions/settings/settings_apitest.cc',
         'browser/extensions/stubs_apitest.cc',
         'browser/extensions/webstore_inline_install_browsertest.cc',

chrome/common/extensions/api/extension_api.json

@@ -9190,7 +9190,7 @@
           "bytes": {
             "type": "blob",
             "optional": true,
-            "description": "The data received, as a Blob."
+            "description": "The data received."
           },
           "errorCode": {
             "type": "integer",
@@ -9258,6 +9258,19 @@
           }
         ]
       },
+      {
+        "name": "destroy",
+        "type": "function",
+        "description": "Destroys the socket. Each socket created should be destroyed after use.",
+        "parameters": [
+          {
+	    "name": "socketId",
+            "type": "integer",
+            "description": "The socketId.",
+	    "minimum": 1
+          }
+        ]
+      },
       {
         "name": "connect",
         "type": "function",
@@ -9296,34 +9309,22 @@
         ]
       },
       {
-        "name": "disconnect",
+        "name": "close",
         "type": "function",
-        "description": "Disconnects the socket.",
+        "description": "Closes the socket.",
         "parameters": [
           {
 	    "name": "socketId",
             "type": "integer",
             "description": "The socketId.",
 	    "minimum": 1
-          },
-          {
-            "name": "callback",
-            "type": "function",
-            "description": "Called when the disconnection is complete.",
-            "parameters": [
-              {
-                "type": "boolean",
-                "name": "result",
-		"description": "True if successful, false otherwise."
-              }
-            ]
           }
         ]
       },
       {
-        "name": "send",
+        "name": "write",
         "type": "function",
-        "description": "Sends data on the connected socket.",
+        "description": "Writes data on the connected socket.",
         "parameters": [
           {
 	    "name": "socketId",
@@ -9331,19 +9332,23 @@
             "description": "The socketId.",
 	    "minimum": 1
           },
+          {
+	    "name": "data",
+            "type": "string",
+            "description": "The data to write. Warning: will probably become a blob or other appropriate binary-friendly type."
+          },
           {
             "name": "callback",
             "type": "function",
-            "description": "Called when the send is complete.",
+            "description": "Called when any of the following happens: the write operation completes, the write operation blocked before completion, or an error occurred.",
             "parameters": [
               {
                 "type": "object",
-                "name": "sendInfo",
+                "name": "writeInfo",
                 "properties": {
-                  "bytesSent": {
+                  "bytesWritten": {
                     "type": "integer",
-                    "minimum": 0,
-                    "description": "The number of bytes sent."
+                    "description": "The number of bytes sent, or a negative error code."
                   }
                 }
               }

chrome/common/extensions/docs/experimental.socket.html

@@ -1618,7 +1618,8 @@
               <div class="summary"><span style="display: none; ">void</span>
                   <!-- Note: intentionally longer 80 columns -->
                   <span>chrome.experimental.socket.send</span>(<span class="null"><span style="display: none; ">, </span><span>integer</span>
-                      <var><span>socketId</span></var></span><span class="null"><span>, </span><span>function</span>
+                      <var><span>socketId</span></var></span><span class="null"><span>, </span><span>string</span>
+                      <var><span>data</span></var></span><span class="null"><span>, </span><span>function</span>
                       <var><span>callback</span></var></span>)</div>
 
               <div class="description">
@@ -1695,6 +1696,74 @@
             <div></div>
           </dd>
 
+        </div>
+                  </div><div>
+                    <div>
+          <dt>
+            <var>data</var>
+              <em>
+
+                <!-- TYPE -->
+                <div style="display:inline">
+                  (
+                    <span class="optional" style="display: none; ">optional</span>
+                    <span class="enum" style="display: none; ">enumerated</span>
+                    <span id="typeTemplate">
+                      <span style="display: none; ">
+                        <a> Type</a>
+                      </span>
+                      <span>
+                        <span style="display: none; ">
+                          array of <span><span></span></span>
+                        </span>
+                        <span>string</span>
+                        <span style="display: none; "></span>
+                      </span>
+                    </span>
+                  )
+                </div>
+
+              </em>
+          </dt>
+          <dd class="todo" style="display: none; ">
+            Undocumented.
+          </dd>
+          <dd>The data to send.</dd>
+          <dd style="display: none; ">
+            This parameter was added in version
+            <b><span></span></b>.
+            You must omit this parameter in earlier versions,
+            and you may omit it in any version.  If you require this
+            parameter, the manifest key
+            <a href="manifest.html#minimum_chrome_version">minimum_chrome_version</a>
+            can ensure that your extension won't be run in an earlier browser version.
+          </dd>
+
+          <!-- OBJECT PROPERTIES -->
+          <dd style="display: none; ">
+            <dl>
+              <div>
+                <div>
+                </div>
+              </div>
+            </dl>
+          </dd>
+
+          <!-- OBJECT METHODS -->
+          <dd style="display: none; ">
+            <div></div>
+          </dd>
+
+          <!-- OBJECT EVENT FIELDS -->
+          <dd style="display: none; ">
+            <div></div>
+          </dd>
+
+          <!-- FUNCTION PARAMETERS -->
+          <dd style="display: none; ">
+            <div></div>
+          </dd>
+
         </div>
                   </div><div>
                     <div>

chrome/test/data/extensions/api_test/socket/api/background.js

@@ -6,7 +6,7 @@ chrome.test.runTests([
   function testCreation() {
     chrome.experimental.socket.create("udp", {},
       function callback(socketInfo) {
-        chrome.test.assertEq(42, socketInfo.socketId);
+        chrome.test.assertTrue(socketInfo.socketId > 0);
         chrome.test.succeed();
       });
   }