Real (but naive) UDP socket sending.

chrome/browser/extensions/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_SOCKET_API_CONTROLLER_H_
+#define CHROME_BROWSER_EXTENSIONS_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 {
+
+// 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 {

[Mihai Parparita -not on Chrome, 2011/12/01 23:39:32]

It seems like this class could live entirely inside of the .cc file (including
its declaration), since SocketController's public interface only exposes socket
IDs.

[miket_OOO, 2011/12/02 21:06:36]

Sounds great! Done.

+ public:
+  explicit Socket(const Profile* profile);
+  ~Socket();
+
+  void set_src_extension_id(const std::string& src_extension_id) {

[Mihai Parparita -not on Chrome, 2011/12/01 23:39:32]

Having these properties as constructor parameters seems cleaner (presumably
they'll never change once the socket is created).

[miket_OOO, 2011/12/02 21:06:36]

Done.

+    src_extension_id_ = src_extension_id;
+  }
+  const std::string& src_extension_id() { return src_extension_id_; }
+  void set_src_id(int src_id) { src_id_ = src_id; }
+  int src_id() { return src_id_; }
+  void set_src_url(const GURL& src_url) { src_url_ = src_url; }
+  const GURL& src_url() { return src_url_; }
+
+  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_;
+  int src_id_;
+  GURL src_url_;
+
+  net::UDPClientSocket* udp_client_socket_;

[Mihai Parparita -not on Chrome, 2011/12/01 23:39:32]

Will TCP socket functionality go into the same class? (I'm asking because I was
wondering if this class should be called UDPSocket and/or whether it should
serve as a base class for a UDPSocket class). 

[miket_OOO, 2011/12/02 21:06:36]

I've thought about the same thing. I'd like to do a bit of design work on the
TCP API before making a decision.

+  bool is_connected_;
+  net::OldCompletionCallbackImpl<Socket> io_callback_;
+
+  // A callback required by UDPClientSocket::Write().
+  void OnIOComplete(int result);
+};
+
+// 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).

[Mihai Parparita -not on Chrome, 2011/12/01 23:39:32]

We could perhaps expose sockets as v8 objects, in which case it is possible to
get a callback when they're GC-ed. Maybe grab Aaron and I for some
brainstorming?

[Aaron Boodman, 2011/12/02 06:06:12]

GC is non-deterministic so it's not usually a good choice for resources that
should be freed in a timely fashion.

The more JavaScript-y approach for this kind of thing is to abuse closures:

socket.create(..., function(...) {
  socket.write(..., function(...) {
    socket.read(...)
    socket.read(...)
    socket.write(...)
  });
});

The socket is kept alive as long as there is at least one callback pending on
it. When the callback count goes to zero, it is immediately destroyed.

Also, unrelated API nit: Is it possible to combine create and connect?

[miket_OOO, 2011/12/02 21:06:36]

OK. I'll do that in a separate CL.
It's possible, yes, but let me compare the TCP API first to see whether we can
leverage any similarity between the two. It's attractive to have Create/Destroy
be symmetric in the sense that they're associated with memory allocation, but on
the other hand I could see the argument that the Connect arguments are like
constructor arguments that every socket's going to need anyway.

+  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_SOCKET_API_CONTROLLER_H_