[NaCl SDK] Remove use of TIOCNACLINPUT from nacl_io/ppapi_simple.

This is replaced by NACL_IOC_HANDLEMESSAGE.

BUG=none
R=sbc@chromium.org

Review URL: https://codereview.chromium.org/334983007

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

native_client_sdk/src/libraries/nacl_io/devfs/jspipe_event_emitter.cc

@@ -150,7 +150,7 @@ Error JSPipeEventEmitter::SendMessageToJS(PP_Var operation, PP_Var payload) {
   // Create dict object which will be sent to JavaScript.
   PP_Var dict = dict_iface_->Create();
 
-  // Set try keys in the dictionaty: 'pipe', 'operation', and 'payload'
+  // Set three keys in the dictionary: 'pipe', 'operation', and 'payload'
   dict_iface_->Set(dict, pipe_key_, pipe_name_var_);
   dict_iface_->Set(dict, operation_key_, operation);
   dict_iface_->Set(dict, payload_key_, payload);

native_client_sdk/src/libraries/nacl_io/devfs/tty_node.cc

@@ -18,6 +18,7 @@
 #include "nacl_io/ioctl.h"
 #include "nacl_io/kernel_handle.h"
 #include "nacl_io/kernel_intercept.h"
+#include "nacl_io/log.h"
 #include "nacl_io/pepper_interface.h"
 #include "sdk_util/auto_lock.h"
 
@@ -167,11 +168,32 @@ Error TtyNode::Echo(const char* string, int count) {
   return 0;
 }
 
-Error TtyNode::ProcessInput(struct tioc_nacl_input_string* message) {
-  AUTO_LOCK(emitter_->GetLock())
+Error TtyNode::ProcessInput(PP_Var message) {
+  if (message.type != PP_VARTYPE_STRING) {
+    LOG_ERROR("ProcessInput: expected VarString but got %d.", message.type);
+    return EINVAL;
+  }
+
+  PepperInterface* ppapi = filesystem_->ppapi();
+  if (!ppapi) {
+    LOG_ERROR("ProcessInput: ppapi is NULL.");
+    return EINVAL;
+  }
+
+  VarInterface* var_iface = ppapi->GetVarInterface();
+  if (!var_iface) {
+    LOG_ERROR("ProcessInput: Var interface pointer is NULL.");
+    return EINVAL;
+  }
 
-  const char* buffer = message->buffer;
-  size_t num_bytes = message->length;
+  uint32_t num_bytes;
+  const char* buffer = var_iface->VarToUtf8(message, &num_bytes);
+  Error error = ProcessInput(buffer, num_bytes);
+  return error;
+}
+
+Error TtyNode::ProcessInput(const char* buffer, size_t num_bytes) {
+  AUTO_LOCK(emitter_->GetLock())
 
   for (size_t i = 0; i < num_bytes; i++) {
     char c = buffer[i];
@@ -248,12 +270,9 @@ Error TtyNode::VIoctl(int request, va_list args) {
       output_handler_ = *arg;
       return 0;
     }
-    case TIOCNACLINPUT: {
-      // This ioctl is used to deliver data from the user to this tty node's
-      // input buffer.
-      struct tioc_nacl_input_string* message =
-          va_arg(args, struct tioc_nacl_input_string*);
-      return ProcessInput(message);
+    case NACL_IOC_HANDLEMESSAGE: {
+      struct PP_Var* message = va_arg(args, struct PP_Var*);
+      return ProcessInput(*message);
     }
     case TIOCSWINSZ: {
       struct winsize* size = va_arg(args, struct winsize*);
@@ -280,6 +299,9 @@ Error TtyNode::VIoctl(int request, va_list args) {
       size->ws_col = cols_;
       return 0;
     }
+    default: {
+      LOG_ERROR("TtyNode:VIoctl: Unknown request: %#x", request);
+    }
   }
 
   return EINVAL;

native_client_sdk/src/libraries/nacl_io/devfs/tty_node.h

@@ -10,6 +10,8 @@
 
 #include <deque>
 
+#include <ppapi/c/pp_var.h>
+
 #include "nacl_io/char_node.h"
 #include "nacl_io/ioctl.h"
 #include "nacl_io/ostermios.h"
@@ -43,7 +45,8 @@ class TtyNode : public CharNode {
  private:
   ScopedEventEmitter emitter_;
 
-  Error ProcessInput(struct tioc_nacl_input_string* message);
+  Error ProcessInput(PP_Var var);
+  Error ProcessInput(const char* buffer, size_t num_bytes);
   Error Echo(const char* string, int count);
   void InitTermios();
 

native_client_sdk/src/libraries/nacl_io/ioctl.h

@@ -8,20 +8,13 @@
 #include <sys/types.h>
 
 /*
- * ioctl to feed input to a tty node. Accepts a pointer to the following
- * struct (tioc_nacl_input_string), which contains a pointer to an array
- * of characters.
- */
-#define TIOCNACLINPUT 0xadcd02
-
-/*
- * ioctl to register an output handler with the tty node.  Will fail
- * with EALREADY if a handler is already registered.  Expects an
- * argument of type tioc_nacl_output.  The handler will be called during
- * calls to write() on the thread that calls write(), or, for echoed input
- * during the TIOCNACLINPUT ioctl() on the thread calling ioctl(). The
- * handler should return the number of bytes written/handled, or -errno
- * if an error occured.
+ * ioctl to register an output handler with the tty node.  Will fail with
+ * EALREADY if a handler is already registered.  Expects an argument of type
+ * tioc_nacl_output.  The handler will be called during calls to write() on the
+ * thread that calls write(), or, for echoed input during the
+ * NACL_IOC_HANDLEMESSAGE ioctl() on the thread calling ioctl(). The handler
+ * should return the number of bytes written/handled, or -errno if an error
+ * occured.
  */
 #define TIOCNACLOUTPUT 0xadcd03
 
@@ -51,11 +44,6 @@
 
 typedef char* naclioc_jspipe_name;
 
-struct tioc_nacl_input_string {
-  size_t length;
-  const char* buffer;
-};
-
 typedef ssize_t (*tioc_nacl_output_handler_t)(const char* buf,
                                               size_t count,
                                               void* user_data);

native_client_sdk/src/libraries/ppapi_simple/ps_instance.cc

@@ -290,7 +290,7 @@ bool PSInstance::ProcessProperties() {
 
   exit_message_ = getenv("PS_EXIT_MESSAGE");
 
-  // If PS_EXIT_MESSAGE is set in the envionment then we perform a handshake
+  // If PS_EXIT_MESSAGE is set in the environment then we perform a handshake
   // with JavaScript when program exits.
   if (exit_message_ != NULL)
     nacl_io_register_exit_handler(HandleExitStatic, this);
@@ -409,27 +409,6 @@ void PSInstance::MessageHandlerInput(const pp::Var& key,
                                      const pp::Var& message) {
   std::string key_string = key.AsString();
 
-  if (message.is_string() && key_string == tty_prefix_) {
-    std::string buffer = message.AsString();
-
-    // Since our message may contain null characters, we can't send it as a
-    // naked C string, so we package it up in this struct before sending it
-    // to the ioctl.
-    struct tioc_nacl_input_string ioctl_message;
-    ioctl_message.length = buffer.size();
-    ioctl_message.buffer = buffer.c_str();
-    int ret = ioctl(tty_fd_, TIOCNACLINPUT, &ioctl_message);
-    if (ret != 0 && errno != ENOTTY) {
-      Error("ioctl returned unexpected error: %d.\n", ret);
-    }
-    return;
-  }
-
-  if (!message.is_array_buffer()) {
-    Error("Expected ArrayBuffer object but got: %d", message.pp_var().type);
-    return;
-  }
-
   const char* filename = NULL;
   if (key_string == tty_prefix_) {
     filename = "/dev/tty";
@@ -453,12 +432,15 @@ void PSInstance::MessageHandlerInput(const pp::Var& key,
   int ret = ioctl(fd, NACL_IOC_HANDLEMESSAGE, &message.pp_var());
   if (ret != 0) {
     Error("ioctl on %s failed: %d.\n", filename, ret);
+    close(fd);
     return;
   }
+
+  close(fd);
 }
 
 void PSInstance::HandleExitStatic(int status, void* user_data) {
-  PSInstance* instance = reinterpret_cast<PSInstance*>(user_data);
+  PSInstance* instance = static_cast<PSInstance*>(user_data);
   instance->ExitHandshake(status);
 }
 
@@ -483,28 +465,28 @@ void PSInstance::MessageHandlerResize(const pp::Var& message) {
 ssize_t PSInstance::TtyOutputHandlerStatic(const char* buf,
                                            size_t count,
                                            void* user_data) {
-  PSInstance* instance = reinterpret_cast<PSInstance*>(user_data);
+  PSInstance* instance = static_cast<PSInstance*>(user_data);
   return instance->TtyOutputHandler(buf, count);
 }
 
 void PSInstance::MessageHandlerExitStatic(const pp::Var& key,
                                           const pp::Var& value,
                                           void* user_data) {
-  PSInstance* instance = reinterpret_cast<PSInstance*>(user_data);
+  PSInstance* instance = static_cast<PSInstance*>(user_data);
   instance->MessageHandlerExit(value);
 }
 
 void PSInstance::MessageHandlerInputStatic(const pp::Var& key,
                                            const pp::Var& value,
                                            void* user_data) {
-  PSInstance* instance = reinterpret_cast<PSInstance*>(user_data);
+  PSInstance* instance = static_cast<PSInstance*>(user_data);
   instance->MessageHandlerInput(key, value);
 }
 
 void PSInstance::MessageHandlerResizeStatic(const pp::Var& key,
                                             const pp::Var& value,
                                             void* user_data) {
-  PSInstance* instance = reinterpret_cast<PSInstance*>(user_data);
+  PSInstance* instance = static_cast<PSInstance*>(user_data);
   instance->MessageHandlerResize(value);
 }
 
@@ -525,21 +507,6 @@ void PSInstance::PostEvent(PSEventType type, const PP_Var& var) {
   assert(PSE_INSTANCE_HANDLEMESSAGE == type);
 
   pp::Var event(var);
-  // Legacy support for passing TTY input as a string <prefix>:<payload>
-  // TODO(sbc): remove this in a future release.
-  if (tty_fd_ >= 0 && event.is_string()) {
-    std::string message = event.AsString();
-    size_t prefix_len = strlen(tty_prefix_);
-    if (message.size() > prefix_len) {
-      if (!strncmp(message.c_str(), tty_prefix_, prefix_len)) {
-        LOG_WARN("Passing TTY data using a string prefix is deprecated. "
-                 "Use a JavaScript dictionary instead.");
-        MessageHandlerInput(pp::Var(message.substr(0, prefix_len)),
-                            pp::Var(message.substr(prefix_len)));
-        return;
-      }
-    }
-  }
 
   // If the message is a dictionary then see if it matches one
   // of the specific handlers, then call that handler rather than
@@ -550,8 +517,7 @@ void PSInstance::PostEvent(PSEventType type, const PP_Var& var) {
     if (keys.GetLength() == 1) {
       pp::Var key = keys.Get(0);
       Trace("calling handler for: %s", key.AsString().c_str());
-      MessageHandlerMap::iterator iter =
-          message_handlers_.find(key.AsString());
+      MessageHandlerMap::iterator iter = message_handlers_.find(key.AsString());
       if (iter != message_handlers_.end()) {
         MessageHandler_t handler = iter->second.handler;
         void* user_data = iter->second.user_data;

native_client_sdk/src/libraries/ppapi_simple/ps_instance.h

@@ -109,7 +109,6 @@ class PSInstance : public pp::Instance, pp::MouseLock, pp::Graphics3DClient {
                               MessageHandler_t handler,
                               void* user_data);
 
-
   // Perform exit handshake with JavaScript.
   // This is called by _exit before the process is terminated to ensure
   // that all messages sent prior to _exit arrive at the JavaScript side.

native_client_sdk/src/tests/nacl_io_test/jspipe_test.cc

@@ -155,7 +155,11 @@ class JSPipeNodeTest : public ::testing::Test {
   }
 
   // Verify the contents of the jspipe mesage, which should be
-  // {'<pipe_name>' : ['<command_name>', payload] }
+  // {
+  //   "pipe": '<pipe_name>',
+  //   "operation": '<command_name>',
+  //   "payload": payload
+  // }
   void VerifyPipeMessage(PP_Var message,
                          const char* pipe_name,
                          const char* operation,

native_client_sdk/src/tests/nacl_io_test/tty_test.cc

@@ -24,9 +24,17 @@ using namespace nacl_io;
 
 namespace {
 
+static int ki_ioctl_wrapper(int fd, int request, ...) {
+  va_list ap;
+  va_start(ap, request);
+  int rtn = ki_ioctl(fd, request, ap);
+  va_end(ap);
+  return rtn;
+}
+
 class TtyNodeTest : public ::testing::Test {
  public:
-  TtyNodeTest() : fs_(&pepper_) {}
+  TtyNodeTest() : fs_(&ppapi_) {}
 
   void SetUp() {
     ASSERT_EQ(0, fs_.Access(Path("/tty"), R_OK | W_OK));
@@ -36,7 +44,7 @@ class TtyNodeTest : public ::testing::Test {
   }
 
  protected:
-  FakePepperInterface pepper_;
+  FakePepperInterface ppapi_;
   DevFsForTesting fs_;
   ScopedNode dev_tty_;
 };
@@ -45,15 +53,26 @@ class TtyTest : public ::testing::Test {
  public:
   void SetUp() {
     ASSERT_EQ(0, ki_push_state_for_testing());
-    ASSERT_EQ(0, ki_init(&kp_));
+    ASSERT_EQ(0, ki_init_interface(&kp_, &ppapi_));
+
+    var_iface_ = ppapi_.GetVarInterface();
   }
 
   void TearDown() {
     ki_uninit();
   }
 
+  int TtyWrite(int fd, const char* string) {
+    PP_Var message_var = var_iface_->VarFromUtf8(string, strlen(string));
+    int result = ki_ioctl_wrapper(fd, NACL_IOC_HANDLEMESSAGE, &message_var);
+    var_iface_->Release(message_var);
+    return result;
+  }
+
  protected:
+  FakePepperInterface ppapi_;
   KernelProxy kp_;
+  VarInterface* var_iface_;
 };
 
 TEST_F(TtyNodeTest, InvalidIoctl) {
@@ -65,9 +84,8 @@ TEST_F(TtyNodeTest, TtyInput) {
   // Now let's try sending some data over.
   // First we create the message.
   std::string message("hello, how are you?\n");
-  struct tioc_nacl_input_string packaged_message;
-  packaged_message.length = message.size();
-  packaged_message.buffer = message.data();
+  VarInterface* var_iface = ppapi_.GetVarInterface();
+  PP_Var message_var = var_iface->VarFromUtf8(message.data(), message.size());
 
   // Now we make buffer we'll read into.
   // We fill the buffer and a backup buffer with arbitrary data
@@ -80,7 +98,9 @@ TEST_F(TtyNodeTest, TtyInput) {
   memset(backup_buffer, 'a', 100);
 
   // Now we actually send the data
-  EXPECT_EQ(0, dev_tty_->Ioctl(TIOCNACLINPUT, &packaged_message));
+  EXPECT_EQ(0, dev_tty_->Ioctl(NACL_IOC_HANDLEMESSAGE, &message_var));
+
+  var_iface->Release(message_var);
 
   // We read a small chunk first to ensure it doesn't give us
   // more than we ask for.
@@ -137,21 +157,6 @@ TEST_F(TtyNodeTest, TtyOutput) {
   EXPECT_EQ(0, strncmp(user_data.output_buf, message, message_len));
 }
 
-static int ki_ioctl_wrapper(int fd, int request, ...) {
-  va_list ap;
-  va_start(ap, request);
-  int rtn = ki_ioctl(fd, request, ap);
-  va_end(ap);
-  return rtn;
-}
-
-static int TtyWrite(int fd, const char* string) {
-  struct tioc_nacl_input_string input;
-  input.buffer = string;
-  input.length = strlen(input.buffer);
-  return ki_ioctl_wrapper(fd, TIOCNACLINPUT, &input);
-}
-
 // Returns:
 //   0 -> Not readable
 //   1 -> Readable
@@ -337,10 +342,12 @@ TEST_F(TtyTest, ResizeDuringSelect) {
  * Sleep for 50ms then send some input to the /dev/tty.
  */
 static void* input_thread_main(void* arg) {
+  TtyTest* thiz = static_cast<TtyTest*>(arg);
+
   usleep(50 * 1000);
 
   int fd = ki_open("/dev/tty", O_RDONLY);
-  TtyWrite(fd, "test\n");
+  thiz->TtyWrite(fd, "test\n");
   return NULL;
 }
 
@@ -357,7 +364,7 @@ TEST_F(TtyTest, InputDuringSelect) {
   FD_SET(tty_fd, &errorfds);
 
   pthread_t resize_thread;
-  pthread_create(&resize_thread, NULL, input_thread_main, NULL);
+  pthread_create(&resize_thread, NULL, input_thread_main, this);
 
   struct timeval timeout;
   timeout.tv_sec = 20;
@@ -368,4 +375,5 @@ TEST_F(TtyTest, InputDuringSelect) {
 
   ASSERT_EQ(1, rtn);
 }
-}
+
+}  // namespace