Rename HandleConverter to NaClMessageScanner.

ppapi/proxy/nacl_message_scanner.cc

Index: ppapi/proxy/nacl_message_scanner.cc
diff --git a/ppapi/proxy/handle_converter.cc b/ppapi/proxy/nacl_message_scanner.cc
similarity index 50%
rename from ppapi/proxy/handle_converter.cc
rename to ppapi/proxy/nacl_message_scanner.cc
index 587585b53b6e449311721e83f120031beabd7395..810a617c55519df9a6d39d45132b250b8f5a952f 100644
--- a/ppapi/proxy/handle_converter.cc
+++ b/ppapi/proxy/nacl_message_scanner.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "ppapi/proxy/handle_converter.h"
+#include "ppapi/proxy/nacl_message_scanner.h"
 
 #include <vector>
 #include "base/bind.h"
@@ -21,28 +21,37 @@ class Message;
 
 namespace {
 
+typedef std::vector<ppapi::proxy::SerializedHandle> Handles;
+
+struct ScanOutput {

[Mark Seaborn, 2013/10/30 20:05:31]

Can you add a note in the commit message to say that you're factoring out some
function arguments into this struct?  Might this factoring out be better done as
a separate change?

[bbudge, 2013/10/30 21:02:49]

Done. (Added comment to commit message.)

+  ScanOutput(Handles* h, IPC::Message* m)
+      : handles(h), handle_index(0), msg(m), audit_msg(NULL) {}
+
+  Handles* handles;

[Mark Seaborn, 2013/10/30 20:05:31]

Add a comment?  Is this an input or output of scanning?

[bbudge, 2013/10/30 21:02:49]

Done.

+  int handle_index;

[Mark Seaborn, 2013/10/30 20:05:31]

Add a comment for this field?  Is it an index into 'handles'?  Is it different
from handles->size()?

[bbudge, 2013/10/30 21:02:49]

Done.

+  IPC::Message* msg;

[Mark Seaborn, 2013/10/30 20:05:31]

Add comments for 'msg' and 'audit_msg'?

[bbudge, 2013/10/30 21:02:49]

Done.

+  IPC::Message* audit_msg;

[Mark Seaborn, 2013/10/30 20:05:31]

I don't see any users of this field.

[bbudge, 2013/10/30 21:02:49]

This is the whole point of the CL. But as it's unused, I'll remove it and the
overload that catches nested messages for a future CL.

+};
+
 void WriteHandle(int handle_index,
                  const ppapi::proxy::SerializedHandle& handle,
-                 IPC::Message* message) {
-  ppapi::proxy::SerializedHandle::WriteHeader(handle.header(), message);
+                 IPC::Message* msg) {
+  ppapi::proxy::SerializedHandle::WriteHeader(handle.header(), msg);
 
   // Now write the handle itself in POSIX style.
-  message->WriteBool(true);  // valid == true
-  message->WriteInt(handle_index);
+  msg->WriteBool(true);  // valid == true
+  msg->WriteInt(handle_index);
 }
 
-typedef std::vector<ppapi::proxy::SerializedHandle> Handles;
+// Define overloads for each kind of message parameter that requires special
+// handling. See ScanTuple for how these get used.
 
-// We define overloads for catching SerializedHandles so that we can share
-// them correctly to the untrusted side.
-// See ConvertHandlesImpl for how these get used.
-void ConvertHandlesInParam(const ppapi::proxy::SerializedHandle& handle,
-                           Handles* handles,
-                           IPC::Message* msg,
-                           int* handle_index) {
-  handles->push_back(handle);
-  if (msg)
-    WriteHandle((*handle_index)++, handle, msg);
+// Scan SerializedHandles.
+void ScanParam(const ppapi::proxy::SerializedHandle& handle,
+               ScanOutput* output) {
+  output->handles->push_back(handle);
+  if (output->msg)
+    WriteHandle(output->handle_index++, handle, output->msg);
 }
 
 void HandleWriter(int* handle_index,
@@ -51,129 +60,118 @@ void HandleWriter(int* handle_index,
   WriteHandle((*handle_index)++, handle, m);
 }
 
-void ConvertHandlesInParam(const ppapi::proxy::SerializedVar& var,
-                           Handles* handles,
-                           IPC::Message* msg,
-                           int* handle_index) {
+// Scan SerializedVars, which can contain handles.
+void ScanParam(const ppapi::proxy::SerializedVar& var,
+               ScanOutput* output) {
   std::vector<ppapi::proxy::SerializedHandle*> var_handles = var.GetHandles();
   if (var_handles.empty())
     return;
 
   for (size_t i = 0; i < var_handles.size(); ++i)
-    handles->push_back(*var_handles[i]);
-  if (msg)
-    var.WriteDataToMessage(msg, base::Bind(&HandleWriter, handle_index));
+    output->handles->push_back(*var_handles[i]);
+  if (output->msg)
+    var.WriteDataToMessage(output->msg,
+                           base::Bind(&HandleWriter, &output->handle_index));
 }
 
 // For PpapiMsg_ResourceReply and the reply to PpapiHostMsg_ResourceSyncCall,
 // the handles are carried inside the ResourceMessageReplyParams.
-// NOTE: We only translate handles from host->NaCl. The only kind of
+// NOTE: We only catch handles from host->NaCl. The only kind of

[Mark Seaborn, 2013/10/30 20:05:31]

"catch" seems more vague than "translate" (the original).

[bbudge, 2013/10/30 21:02:49]

Done.

 //       ResourceMessageParams that travels this direction is
 //       ResourceMessageReplyParams, so that's the only one we need to handle.
-void ConvertHandlesInParam(
-    const ppapi::proxy::ResourceMessageReplyParams& params,
-    Handles* handles,
-    IPC::Message* msg,
-    int* handle_index) {
+void ScanParam(const ppapi::proxy::ResourceMessageReplyParams& params,
+               ScanOutput* output) {
   // First, if we need to rewrite the message parameters, write everything
   // before the handles (there's nothing after the handles).
-  if (msg) {
-    params.WriteReplyHeader(msg);
+  if (output->msg) {
+    params.WriteReplyHeader(output->msg);
     // IPC writes the vector length as an int before the contents of the
     // vector.
-    msg->WriteInt(static_cast<int>(params.handles().size()));
+    output->msg->WriteInt(static_cast<int>(params.handles().size()));
   }
   for (Handles::const_iterator iter = params.handles().begin();
        iter != params.handles().end();
        ++iter) {
     // ConvertHandle will write each handle to |msg|, if necessary.
-    ConvertHandlesInParam(*iter, handles, msg, handle_index);
+    ScanParam(*iter, output);
   }
   // Tell ResourceMessageReplyParams that we have taken the handles, so it
   // shouldn't close them. The NaCl runtime will take ownership of them.
   params.ConsumeHandles();
 }
 
-// This overload is to catch all types other than SerializedHandle or
-// ResourceMessageReplyParams. On Windows, |msg| will be a valid pointer, and we
-// must write |param| to it.
+// Scan nested messages, for auditing resource call and reply messages.
+void ScanParam(const IPC::Message& nested_msg,
+               ScanOutput* output) {
+  if (output->msg)
+    IPC::WriteParam(output->msg, nested_msg);
+}
+
+// This overload is to catch all other types. If the output msg pointer is not
+// NULL, write the parameter.
 template <class T>
-void ConvertHandlesInParam(const T& param,
-                           Handles* /* handles */,
-                           IPC::Message* msg,
-                           int* /* handle_index */) {
-  // It's not a handle, so just write to the output message, if necessary.
-  if (msg)
-    IPC::WriteParam(msg, param);
+void ScanParam(const T& param,
+               ScanOutput* output) {
+  if (output->msg)
+    IPC::WriteParam(output->msg, param);
 }
 
-// These just break apart the given tuple and run ConvertHandle over each param.
-// The idea is to extract any handles in the tuple, while writing all data to
-// msg (if msg is valid). The msg will only be valid on Windows, where we need
-// to re-write all of the message parameters, writing the handles in POSIX style
-// for NaCl.
+// These just break apart the given tuple and run ScanParam over each param.
+// The idea is to scan elements in the tuple which require special handling,
+// and write any output data into the output struct.
 template <class A>
-void ConvertHandlesImpl(const Tuple1<A>& t1, Handles* handles,
-                        IPC::Message* msg) {
-  int handle_index = 0;
-  ConvertHandlesInParam(t1.a, handles, msg, &handle_index);
+void ScanTuple(const Tuple1<A>& t1, ScanOutput* output) {
+  ScanParam(t1.a, output);
 }
 template <class A, class B>
-void ConvertHandlesImpl(const Tuple2<A, B>& t1, Handles* handles,
-                        IPC::Message* msg) {
-  int handle_index = 0;
-  ConvertHandlesInParam(t1.a, handles, msg, &handle_index);
-  ConvertHandlesInParam(t1.b, handles, msg, &handle_index);
+void ScanTuple(const Tuple2<A, B>& t1, ScanOutput* output) {
+  ScanParam(t1.a, output);
+  ScanParam(t1.b, output);
 }
 template <class A, class B, class C>
-void ConvertHandlesImpl(const Tuple3<A, B, C>& t1, Handles* handles,
-                        IPC::Message* msg) {
-  int handle_index = 0;
-  ConvertHandlesInParam(t1.a, handles, msg, &handle_index);
-  ConvertHandlesInParam(t1.b, handles, msg, &handle_index);
-  ConvertHandlesInParam(t1.c, handles, msg, &handle_index);
+void ScanTuple(const Tuple3<A, B, C>& t1, ScanOutput* output) {
+  ScanParam(t1.a, output);
+  ScanParam(t1.b, output);
+  ScanParam(t1.c, output);
 }
 template <class A, class B, class C, class D>
-void ConvertHandlesImpl(const Tuple4<A, B, C, D>& t1, Handles* handles,
-                        IPC::Message* msg) {
-  int handle_index = 0;
-  ConvertHandlesInParam(t1.a, handles, msg, &handle_index);
-  ConvertHandlesInParam(t1.b, handles, msg, &handle_index);
-  ConvertHandlesInParam(t1.c, handles, msg, &handle_index);
-  ConvertHandlesInParam(t1.d, handles, msg, &handle_index);
+void ScanTuple(const Tuple4<A, B, C, D>& t1, ScanOutput* output) {
+  ScanParam(t1.a, output);
+  ScanParam(t1.b, output);
+  ScanParam(t1.c, output);
+  ScanParam(t1.d, output);
 }
 
 template <class MessageType>
-class HandleConverterImpl {
+class MessageScannerImpl {
  public:
-  explicit HandleConverterImpl(const IPC::Message* msg)
+  explicit MessageScannerImpl(const IPC::Message* msg)
       : msg_(static_cast<const MessageType*>(msg)) {
   }
-  bool ConvertMessage(Handles* handles, IPC::Message* out_msg) {
+  bool ScanMessage(ScanOutput* output) {
     typename TupleTypes<typename MessageType::Schema::Param>::ValueTuple params;
     if (!MessageType::Read(msg_, &params))
       return false;
-    ConvertHandlesImpl(params, handles, out_msg);
+    ScanTuple(params, output);
     return true;
   }
 
-  bool ConvertReply(Handles* handles, IPC::SyncMessage* out_msg) {
+  bool ScanReply(ScanOutput* output) {
     typename TupleTypes<typename MessageType::Schema::ReplyParam>::ValueTuple
         params;
     if (!MessageType::ReadReplyParam(msg_, &params))
       return false;
-    // If we need to rewrite the message (i.e., on Windows), we need to make
-    // sure we write the message id first.
-    if (out_msg) {
-      out_msg->set_reply();
+    // If we need to rewrite the message, we need to write the message id first.
+    if (output->msg) {
+      output->msg->set_reply();
       int id = IPC::SyncMessage::GetMessageId(*msg_);
-      out_msg->WriteInt(id);
+      output->msg->WriteInt(id);
     }
-    ConvertHandlesImpl(params, handles, out_msg);
+    ScanTuple(params, output);
     return true;
   }
-  // TODO(dmichael): Add ConvertSyncMessage for outgoing sync messages, if we
-  //                 ever pass handles in one of those.
+  // TODO(dmichael): Add ScanSyncMessage for outgoing sync messages, if we ever
+  //                 need to scan those.
 
  private:
   const MessageType* msg_;
@@ -183,17 +181,15 @@ class HandleConverterImpl {
 
 #define CASE_FOR_MESSAGE(MESSAGE_TYPE) \
       case MESSAGE_TYPE::ID: { \
-        HandleConverterImpl<MESSAGE_TYPE> extractor(&msg); \
-        if (!extractor.ConvertMessage(handles, new_msg_ptr->get())) \
+        MessageScannerImpl<MESSAGE_TYPE> scanner(&msg); \
+        if (!scanner.ScanMessage(&output)) \
           return false; \
         break; \
       }
 #define CASE_FOR_REPLY(MESSAGE_TYPE) \
       case MESSAGE_TYPE::ID: { \
-        HandleConverterImpl<MESSAGE_TYPE> extractor(&msg); \
-        if (!extractor.ConvertReply( \
-                handles, \
-                static_cast<IPC::SyncMessage*>(new_msg_ptr->get()))) \
+        MessageScannerImpl<MESSAGE_TYPE> scanner(&msg); \
+        if (!scanner.ScanReply(&output)) \
           return false; \
         break; \
       }
@@ -203,10 +199,10 @@ namespace proxy {
 
 class SerializedHandle;
 
-HandleConverter::HandleConverter() {
+NaClMessageScanner::NaClMessageScanner() {
 }
 
-bool HandleConverter::ConvertNativeHandlesToPosix(
+bool NaClMessageScanner::ScanMessage(
     const IPC::Message& msg,
     std::vector<SerializedHandle>* handles,
     scoped_ptr<IPC::Message>* new_msg_ptr) {
@@ -214,22 +210,18 @@ bool HandleConverter::ConvertNativeHandlesToPosix(
   DCHECK(new_msg_ptr);
   DCHECK(!new_msg_ptr->get());
 
-  // In Windows, we need to re-write the contents of the message. This is
-  // because in Windows IPC code, native HANDLE values are serialized in the
-  // body of the message.
-  //
-  // In POSIX, we only serialize an index in to a FileDescriptorSet, and the
-  // actual file descriptors are sent out-of-band. So on Windows, to make a
-  // message that's compatible with Windows, we need to write a new message that
-  // has simple indices in the message body instead of the HANDLEs.
+  // We re-write all IPC messages on Windows, because native HANDLE values are
+  // serialized in the body of the message. In POSIX, we only serialize an index
+  // into a FileDescriptorSet, and the actual file descriptors are sent
+  // out-of-band.
   //
   // NOTE: This means on Windows, new_msg_ptr's serialized contents are not
   // compatible with Windows IPC deserialization code; it is intended to be
-  // passed to NaCl.
+  // passed to the POSIX environment in NaCl.
 #if defined(OS_WIN)
   new_msg_ptr->reset(new IPC::Message(msg.routing_id(), msg.type()));
 #else
-  // Even on POSIX, we have to rewrite messages to create channels, because
+  // Even on POSIX, we must rewrite messages to create IPC channels, because
   // these contain a handle with an invalid (place holder) descriptor. The
   // message sending code sees this and doesn't pass the descriptor over
   // correctly.
@@ -237,6 +229,7 @@ bool HandleConverter::ConvertNativeHandlesToPosix(
     new_msg_ptr->reset(new IPC::Message(msg.routing_id(), msg.type()));
 #endif
 
+  ScanOutput output(handles, new_msg_ptr->get());
   switch (msg.type()) {
     CASE_FOR_MESSAGE(PpapiMsg_CreateNaClChannel)
     CASE_FOR_MESSAGE(PpapiMsg_PPBAudio_NotifyAudioStreamCreated)
@@ -269,7 +262,7 @@ bool HandleConverter::ConvertNativeHandlesToPosix(
   return true;
 }
 
-void HandleConverter::RegisterSyncMessageForReply(const IPC::Message& msg) {
+void NaClMessageScanner::RegisterSyncMessageForReply(const IPC::Message& msg) {
   DCHECK(msg.is_sync());
 
   int msg_id = IPC::SyncMessage::GetMessageId(msg);