Multicast socket API

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

Index: chrome/browser/extensions/api/socket/socket_api.cc
diff --git a/chrome/browser/extensions/api/socket/socket_api.cc b/chrome/browser/extensions/api/socket/socket_api.cc
index 108ab87cc4bf757fa3ece1bc497b2d24c0a88bf5..017bed726861e2a0a7338d13974444f0cdf6dd71 100644
--- a/chrome/browser/extensions/api/socket/socket_api.cc
+++ b/chrome/browser/extensions/api/socket/socket_api.cc
@@ -4,7 +4,10 @@
 
 #include "chrome/browser/extensions/api/socket/socket_api.h"
 
+#include <vector>
+
 #include "base/bind.h"
+#include "base/hash_tables.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/permissions/socket_permission.h"
 #include "chrome/browser/browser_process.h"
@@ -38,6 +41,10 @@ const char kPermissionError[] = "App does not have permission";
 const char kNetworkListError[] = "Network lookup failed or unsupported";
 const char kTCPSocketBindError[] =
     "TCP socket does not support bind. For TCP server please use listen.";
+const char kMulticastSocketTypeError[] =
+    "Only UDP socket supports multicast.";
+const char kWildcardArress[] = "*";

[miket_OOO, 2013/04/25 21:52:12]

I think this word is supposed to be "Address"

[Bei Zhang, 2013/04/25 23:56:21]

Done.

+const int kWildcardPortNumber = 0;

[miket_OOO, 2013/04/25 21:52:12]

"Port Number" is overly descriptive. Just "Port" will work in this context.

[Bei Zhang, 2013/04/25 23:56:21]

Done.

 
 SocketAsyncApiFunction::SocketAsyncApiFunction()
     : manager_(NULL) {
@@ -159,7 +166,9 @@ void SocketDestroyFunction::Work() {
 
 SocketConnectFunction::SocketConnectFunction()
     : socket_id_(0),
-      port_(0) {
+      hostname_(),
+      port_(0),
+      socket_(NULL) {
 }
 
 SocketConnectFunction::~SocketConnectFunction() {
@@ -476,7 +485,8 @@ SocketSendToFunction::SocketSendToFunction()
     : socket_id_(0),
       io_buffer_(NULL),
       io_buffer_size_(0),
-      port_(0) {
+      port_(0),
+      socket_(NULL) {
 }
 
 SocketSendToFunction::~SocketSendToFunction() {}
@@ -680,4 +690,205 @@ void SocketGetNetworkListFunction::SendResponseOnUIThread(
   SendResponse(true);
 }
 
+SocketJoinGroupFunction::SocketJoinGroupFunction()
+    : params_(NULL) {}
+
+SocketJoinGroupFunction::~SocketJoinGroupFunction() {}
+
+bool SocketJoinGroupFunction::Prepare() {
+  params_ = api::socket::JoinGroup::Params::Create(*args_);
+  EXTENSION_FUNCTION_VALIDATE(params_.get());
+  return true;
+}
+
+void SocketJoinGroupFunction::Work() {
+  int result = -1;
+  Socket* socket = GetSocket(params_->socket_id);
+  if (!socket) {
+    error_ = kSocketNotFoundError;
+    SetResult(Value::CreateIntegerValue(result));
+    return;
+  }
+
+  if (socket->GetSocketType() == Socket::TYPE_UDP) {
+    SocketPermission::CheckParam param(
+        SocketPermissionRequest::UDP_MUTICAST_MEMBERSHIP,

[miket_OOO, 2013/04/25 21:52:12]

Spelling: MULTICAST

[Bei Zhang, 2013/04/25 23:56:21]

Done.

+        kWildcardArress,
+        kWildcardPortNumber);
+    if (!GetExtension()->CheckAPIPermissionWithParam(APIPermission::kSocket,
+                                                     &param)) {
+      error_ = kPermissionError;
+      SetResult(Value::CreateIntegerValue(result));
+      return;
+    }
+  } else {

[miket_OOO, 2013/04/25 21:52:12]

I wouldn't put this in an else, because it's going to lead to needless nesting
of the main block of code. Something more like this would make sense:

if (!socket)
  exit
if (!TYPE_UDP)
  exit
doAllTheWork

[Bei Zhang, 2013/04/25 23:56:21]

Done.

+    error_ = kMulticastSocketTypeError;
+    SetResult(Value::CreateIntegerValue(result));
+    return;
+  }
+
+  result = static_cast<UDPSocket*>(socket)->JoinGroup(params_->address);
+  if (result != 0) {
+    error_ = net::ErrorToString(result);
+  }
+  SetResult(Value::CreateIntegerValue(result));
+}
+
+
+SocketLeaveGroupFunction::SocketLeaveGroupFunction()
+  : params_(NULL) {}
+
+SocketLeaveGroupFunction::~SocketLeaveGroupFunction() {}
+
+bool SocketLeaveGroupFunction::Prepare() {
+  params_ = api::socket::LeaveGroup::Params::Create(*args_);
+  EXTENSION_FUNCTION_VALIDATE(params_.get());
+  return true;
+}
+
+void SocketLeaveGroupFunction::Work() {
+  int result = -1;
+  Socket* socket = GetSocket(params_->socket_id);
+
+  if (!socket) {
+    error_ = kSocketNotFoundError;
+    SetResult(Value::CreateIntegerValue(result));
+    return;
+  }
+
+  if (socket->GetSocketType() == Socket::TYPE_UDP) {

[miket_OOO, 2013/04/25 21:52:12]

Same here -- rearrange so obvious validation failures are handled immediately,
then let the normal case be a top-level block in the method.

[Bei Zhang, 2013/04/25 23:56:21]

Done.

+    SocketPermission::CheckParam param(
+        SocketPermissionRequest::UDP_MUTICAST_MEMBERSHIP,
+        kWildcardArress,
+        kWildcardPortNumber);
+    if (!GetExtension()->CheckAPIPermissionWithParam(APIPermission::kSocket,
+                                                     &param)) {
+      error_ = kPermissionError;
+      SetResult(Value::CreateIntegerValue(result));
+      return;
+    }
+  } else {
+    error_ = kMulticastSocketTypeError;
+    SetResult(Value::CreateIntegerValue(result));
+    return;
+  }
+
+  result = static_cast<UDPSocket*>(socket)->LeaveGroup(params_->address);
+  if (result != 0)
+    error_ = net::ErrorToString(result);
+  SetResult(Value::CreateIntegerValue(result));
+}
+
+SocketSetMulticastTimeToLiveFunction::SocketSetMulticastTimeToLiveFunction()
+  : params_(NULL) {}
+
+SocketSetMulticastTimeToLiveFunction::~SocketSetMulticastTimeToLiveFunction() {}
+
+bool SocketSetMulticastTimeToLiveFunction::Prepare() {
+  params_ = api::socket::SetMulticastTimeToLive::Params::Create(*args_);
+  EXTENSION_FUNCTION_VALIDATE(params_.get());
+  return true;
+}
+void SocketSetMulticastTimeToLiveFunction::Work() {
+  int result = -1;
+  Socket* socket = GetSocket(params_->socket_id);
+  if (!socket) {
+    error_ = kSocketNotFoundError;
+    SetResult(Value::CreateIntegerValue(result));
+    return;
+  }
+
+  if (socket->GetSocketType() == Socket::TYPE_UDP) {

[miket_OOO, 2013/04/25 21:52:12]

There's a bunch of boilerplate code here. What about a single utility method
like "GetUDPSocketWithValidPermission" that does all these tests?

[Bei Zhang, 2013/04/25 23:56:21]

That'll be ideal. But I think maybe we should do it in a separate CL because
actually all the code in this file are already written this way.

On 2013/04/25 21:52:12, miket wrote:

+    result = static_cast<UDPSocket*>(socket)->
+        SetMulticastTimeToLive(params_->ttl);
+    if (result != 0)
+      error_ = net::ErrorToString(result);
+    SetResult(Value::CreateIntegerValue(result));
+  } else {
+    error_ = kMulticastSocketTypeError;
+  }
+  SetResult(Value::CreateIntegerValue(result));
+}
+
+SocketSetMulticastLoopbackModeFunction::SocketSetMulticastLoopbackModeFunction()
+  : params_(NULL) {}
+
+SocketSetMulticastLoopbackModeFunction::
+  ~SocketSetMulticastLoopbackModeFunction() {}
+
+bool SocketSetMulticastLoopbackModeFunction::Prepare() {
+  params_ = api::socket::SetMulticastLoopbackMode::Params::Create(*args_);
+  EXTENSION_FUNCTION_VALIDATE(params_.get());
+  return true;
+}
+
+void SocketSetMulticastLoopbackModeFunction::Work() {
+  int result = -1;
+  Socket* socket = GetSocket(params_->socket_id);
+  if (!socket) {
+    error_ = kSocketNotFoundError;
+    SetResult(Value::CreateIntegerValue(result));
+    return;
+  }
+
+  if (socket->GetSocketType() == Socket::TYPE_UDP) {
+    result = static_cast<UDPSocket*>(socket)->
+        SetMulticastLoopbackMode(params_->enabled);
+    if (result != 0)
+      error_ = net::ErrorToString(result);
+  } else {
+    error_ = kMulticastSocketTypeError;
+  }
+  SetResult(Value::CreateIntegerValue(result));
+}
+
+SocketGetJoinedGroupsFunction::SocketGetJoinedGroupsFunction()
+  : params_(NULL) {}
+
+SocketGetJoinedGroupsFunction::~SocketGetJoinedGroupsFunction() {}
+
+bool SocketGetJoinedGroupsFunction::Prepare() {
+  params_ = api::socket::GetJoinedGroups::Params::Create(*args_);
+  EXTENSION_FUNCTION_VALIDATE(params_.get());
+  return true;
+}
+
+void SocketGetJoinedGroupsFunction::Work() {
+  int result = -1;
+  Socket* socket = GetSocket(params_->socket_id);
+  if (!socket) {
+    error_ = kSocketNotFoundError;
+    SetResult(Value::CreateIntegerValue(result));
+    return;
+  }
+
+  if (socket->GetSocketType() == Socket::TYPE_UDP) {
+    SocketPermission::CheckParam param(
+        SocketPermissionRequest::UDP_SEND_TO,
+        kWildcardArress,
+        kWildcardPortNumber);
+    if (!GetExtension()->CheckAPIPermissionWithParam(APIPermission::kSocket,
+          &param)) {
+      error_ = kPermissionError;
+      SetResult(Value::CreateIntegerValue(result));
+      return;
+    }
+  } else {
+    error_ = kMulticastSocketTypeError;
+    SetResult(Value::CreateIntegerValue(result));
+    return;
+  }
+
+  base::hash_set<std::string> groups;
+  result = static_cast<UDPSocket*>(socket)->GetJoinedGroups(&groups);
+  if (result != 0)
+    error_ = net::ErrorToString(result);
+  base::ListValue* list = new base::ListValue();
+  for (base::hash_set<std::string>::iterator it = groups.begin();
+      it != groups.end(); ++it) {
+    list->AppendString(*it);

[miket_OOO, 2013/04/25 21:52:12]

Please confirm that AppendString does a copy of the string. Some Value methods
take ownership of the object, which doesn't seem to be what you'd want here.

[Bei Zhang, 2013/04/25 23:56:21]

The copy cannot be avoided by changing the ownership of the string. It will be
copied anyway in the constructor of StringValue. However, we can change the
interface of UDPSocket to accept ListValue* instead of hash_set<std::string> to
avoid the creation of std::string.

On 2013/04/25 21:52:12, miket wrote:

+  }
+  SetResult(list);
+}
+
 }  // namespace extensions