Exposed Listen and Accept methods to plugin.

ppapi/shared_impl/private/ppb_tcp_server_socket_shared.cc

Index: ppapi/shared_impl/private/ppb_tcp_server_socket_shared.cc
diff --git a/ppapi/shared_impl/private/ppb_tcp_server_socket_shared.cc b/ppapi/shared_impl/private/ppb_tcp_server_socket_shared.cc
new file mode 100644
index 0000000000000000000000000000000000000000..2eac711a46158f11ddf2cf35cbbab5a60182b689
--- /dev/null
+++ b/ppapi/shared_impl/private/ppb_tcp_server_socket_shared.cc
@@ -0,0 +1,147 @@
+// 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 "ppapi/shared_impl/private/ppb_tcp_server_socket_shared.h"
+
+#include <cstddef>
+#include <cstring>
+
+#include "base/logging.h"
+#include "base/message_loop.h"
+#include "ppapi/c/pp_errors.h"
+
+namespace ppapi {
+
+PPB_TCPServerSocket_Shared::PPB_TCPServerSocket_Shared(PP_Instance instance)
+    : Resource(instance), state_(NOT_INITIALIZED), tcp_socket_buffer_(NULL) {
+}
+
+PPB_TCPServerSocket_Shared::PPB_TCPServerSocket_Shared(
+    const HostResource& resource)
+    : Resource(resource), state_(NOT_INITIALIZED), tcp_socket_buffer_(NULL) {
+}
+
+PPB_TCPServerSocket_Shared::~PPB_TCPServerSocket_Shared() {
+}
+
+thunk::PPB_TCPServerSocket_Private_API*
+PPB_TCPServerSocket_Shared::AsPPB_TCPServerSocket_Private_API() {
+  return this;
+}
+
+int32_t PPB_TCPServerSocket_Shared::Listen(const PP_NetAddress_Private* addr,
+                                           int32_t backlog,
+                                           PP_CompletionCallback callback) {
+  if (!addr)
+    return PP_ERROR_BADARGUMENT;
+  if (!callback.func)
+    return PP_ERROR_BLOCKS_MAIN_THREAD;
+  if (state_ != NOT_INITIALIZED &&

[yzshen1, 2012/02/10 22:40:15]

Why we have to init and listen as two separate steps?
If I understand the API spec correctly, we don't allow a second call to Listen
(if it succeeds), not even after StopListening(). 

Then we can simply do Init&Listen in one round trip to the brower. The state
machine will be much simpler. Right?

One thing to consider: if we do init&listen together, we should not treat
uninitialized tcp sockets as a FIFO queue any more. The order matters now.
One possible solution is to generate a temp_socket_id at the renderer side, send
it in the Listen message to the browser, the browser then does init&listen and
sends (temp_socket_id, real_socket_id) back. Then we match this response to an
uninitialized socket object.

[ygorshenin1, 2012/02/13 14:59:46]

Because of efficiency. When user creates socket, initialization process is
started, so, when user decided to call Listen, socket is initialized. May be
this is useless because Listen called only once in most cases. Current state
machine is non-obvious and bug-prone so we decided to combine init and listen
processes.

On 2012/02/10 22:40:15, yzshen1 wrote:

+      state_ != INITIALIZING &&
+      state_ != BEFORE_LISTENING) {
+    return PP_ERROR_FAILED;
+  }
+  if (TrackedCallback::IsPending(listen_callback_))
+    return PP_ERROR_INPROGRESS;  // Can only have one pending request.
+
+  if (state_ == NOT_INITIALIZED || state_ == INITIALIZING) {

[brettw, 2012/02/10 21:43:32]

I don't see you using the INITIALIZING state anywhere?

[ygorshenin1, 2012/02/13 14:59:46]

It was used in webkit/plugin/ppapi/ppb_tcp_server_socket_private.cc. But now
it's not used.

On 2012/02/10 21:43:32, brettw wrote:

+    // Store args for lazy Listen call. Listen will be invoked when
+    // initialization is completed.
+    memmove(&listen_addr_, addr, sizeof(*addr));
+    listen_backlog_ = backlog;
+    listen_callback_ = new TrackedCallback(this, callback);
+
+    if (state_ == NOT_INITIALIZED) {

[brettw, 2012/02/10 21:43:32]

This should also be able to be simplified if we can remove the INITIALIZING
state.

[ygorshenin1, 2012/02/13 14:59:46]

Done.

+      state_ = INITIALIZING_PENDING_LISTEN_REQUEST;
+      SendInitialize();
+    } else {
+      state_ = INITIALIZING_PENDING_LISTEN_REQUEST;
+    }
+  } else {  // state_ == BEFORE_LISTENING

[brettw, 2012/02/10 21:43:32]

I also don't understand this case. Can't you only call Listen in one state? It
seems like you should be calling it once after resource creation and any other
usage should fail.

[ygorshenin1, 2012/02/13 14:59:46]

OK, code is simplified.

On 2012/02/10 21:43:32, brettw wrote:

+    listen_callback_ = new TrackedCallback(this, callback);
+    // Send the request, the browser will call us back via ListenACK
+    SendListen(*addr, backlog);
+  }
+  return PP_OK_COMPLETIONPENDING;
+}
+
+int32_t PPB_TCPServerSocket_Shared::Accept(PP_Resource* tcp_socket,
+                                           PP_CompletionCallback callback) {
+  if (!tcp_socket)
+    return PP_ERROR_BADARGUMENT;
+  if (!callback.func)
+    return PP_ERROR_BLOCKS_MAIN_THREAD;
+
+  if (state_ != LISTENING)
+    return PP_ERROR_FAILED;
+  if (TrackedCallback::IsPending(accept_callback_))
+    return PP_ERROR_INPROGRESS;
+
+  tcp_socket_buffer_ = tcp_socket;
+  accept_callback_ = new TrackedCallback(this, callback);
+
+  SendAccept();
+  return PP_OK_COMPLETIONPENDING;
+}
+
+void PPB_TCPServerSocket_Shared::StopListening() {
+  if (state_ != LISTENING)
+    return;
+  state_ = CLOSED;
+
+  SendStopListening();
+  socket_id_ = 0;
+
+  PostAbortAndClearIfNecessary(&listen_callback_);

[brettw, 2012/02/10 21:43:32]

Not necessary, but did you consider just inlining the two lines of
"PostAbortAndClearIfNecessary" here? It would actually be fewer lines of code,
and I'm not sure that having the separate function makes it clearer. I realize
you copied this from the client socket, and it was there partially for
historical reasons, and there may also be more callers.

[ygorshenin1, 2012/02/13 14:59:46]

Done.

+  PostAbortAndClearIfNecessary(&accept_callback_);
+  tcp_socket_buffer_ = NULL;
+}
+
+void PPB_TCPServerSocket_Shared::OnInitializeCompleted(uint32 socket_id) {
+  if (state_ != INITIALIZING && state_ != INITIALIZING_PENDING_LISTEN_REQUEST) {
+    NOTREACHED();
+    return;
+  }
+
+  bool is_pending_listen_request =
+      state_ == INITIALIZING_PENDING_LISTEN_REQUEST;
+
+  if (socket_id == 0) {
+    state_ = NOT_INITIALIZED;
+  } else {
+    state_ = BEFORE_LISTENING;
+    socket_id_ = socket_id;
+  }
+
+  if (is_pending_listen_request) {
+    if (state_ == NOT_INITIALIZED) {
+      // Initialization is failed due to full socket table.
+      TrackedCallback::ClearAndRun(&listen_callback_, PP_ERROR_NOSPACE);
+    } else {
+      SendListen(listen_addr_, listen_backlog_);
+    }
+  }
+}
+
+void PPB_TCPServerSocket_Shared::OnListenCompleted(bool succeeded) {
+  if (state_ != BEFORE_LISTENING ||
+      !TrackedCallback::IsPending(listen_callback_)) {
+    NOTREACHED();
+    return;
+  }
+
+  if (succeeded)
+    state_ = LISTENING;
+
+  TrackedCallback::ClearAndRun(&listen_callback_,
+                               succeeded ? PP_OK : PP_ERROR_FAILED);
+}
+
+void PPB_TCPServerSocket_Shared::PostAbortAndClearIfNecessary(
+    scoped_refptr<TrackedCallback>* callback) {
+  if (callback->get())
+    (*callback)->PostAbort();
+}
+
+}  // namespace ppapi