PPAPI/NaCl: Move handle extraction code to ppapi/proxy

ppapi/proxy/handle_converter.h

Index: ppapi/proxy/handle_converter.h
diff --git a/ppapi/proxy/serialized_flash_menu.h b/ppapi/proxy/handle_converter.h
similarity index 16%
copy from ppapi/proxy/serialized_flash_menu.h
copy to ppapi/proxy/handle_converter.h
index 271caba368bd4e3d4c988e6731e145e2d29cb68e..748c74379d3b4b2416c7efd5e8370c312bbeda2b 100644
--- a/ppapi/proxy/serialized_flash_menu.h
+++ b/ppapi/proxy/handle_converter.h
@@ -1,20 +1,17 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2013 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 PPAPI_PROXY_SERIALIZED_FLASH_MENU_H_
-#define PPAPI_PROXY_SERIALIZED_FLASH_MENU_H_
+#ifndef PPAPI_PROXY_HANDLE_CONVERTER_H_
+#define PPAPI_PROXY_HANDLE_CONVERTER_H_
 
-#include <string>
+#include <map>
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
 #include "ppapi/proxy/ppapi_proxy_export.h"
 
-class PickleIterator;
-
-struct PP_Flash_Menu;
-
 namespace IPC {
 class Message;
 }
@@ -22,25 +19,44 @@ class Message;
 namespace ppapi {
 namespace proxy {
 
-class PPAPI_PROXY_EXPORT SerializedFlashMenu {
- public:
-  SerializedFlashMenu();
-  ~SerializedFlashMenu();
-
-  bool SetPPMenu(const PP_Flash_Menu* menu);
+class SerializedHandle;
 
-  const PP_Flash_Menu* pp_menu() const { return pp_menu_; }
-
-  void WriteToMessage(IPC::Message* m) const;
-  bool ReadFromMessage(const IPC::Message* m, PickleIterator* iter);
+class PPAPI_PROXY_EXPORT HandleConverter {
+ public:
+  HandleConverter();
+
+  // Convert the native handles in |msg| to NaCl style. In some cases (e.g.,
+  // Windows), we need to re-write the contents of the message; in those cases,
+  // a new message will be returned. If |msg| is already in a good form for
+  // NaCl, a NULL scoped_ptr is returned. See explanation in the body of the
+  // method.
+  //
+  // In either case, all the handles in |msg| are extracted into |handles| so
+  // that they can be converted to NaClDesc handles.
+  // See chrome/nacl/nacl_ipc_adapter.cc for where this gets used.
+  bool ConvertNativeHandlesToPosix(const IPC::Message& msg,

[teravest, 2013/01/15 21:14:34]

It feels gross to me to use a scoped_ptr<> as an output type.

It might not be worth the extra work, but I wonder if we should roll (handles,
new_msg_ptr) into a new type, that can also indicate whether we had to create a
new message.

[dmichael (off chromium), 2013/01/15 23:18:59]

I struggled with a good way to do this. I also considered using scoped_ptr for
the return type, and making |success| an out-param.

Making a new type seems kind of ugly; extra code + a bool and a scoped_ptr can
have 4 states, only two of which are valid. I'd kind of rather just use
scoped_ptr if I can.

Alternately...  how do you guys feel about just always rewriting the method?
Apparently the performance is good enough on Windows, where we care the most...

[dmichael (off chromium), 2013/01/15 23:47:06]

FWIW I checked, and scoped_ptr<Foo>* is not without precedent:
https://code.google.com/p/chromium/source/search?q=scoped_ptr<.*>*

+                                   std::vector<SerializedHandle>* handles,
+                                   scoped_ptr<IPC::Message>* new_msg_ptr);
+
+  // |SendingSyncMessage| allows HandleConverter to know what sync messages are

[teravest, 2013/01/15 21:14:34]

The name here is a little funny. Even RegisterSyncMessage might be better. 

[bbudge, 2013/01/15 22:20:25]

+1 or even RegisterSyncMessageForReply.

[dmichael (off chromium), 2013/01/15 23:18:59]

I didn't like it either :-)

Thanks for the good suggestions.

+  // being sent, so that it can associate reply messages with their type.
+  //
+  // Users of HandleConverter must call this when they send a synchronous
+  // message, otherwise HandleConverter won't be able to convert handles in
+  // replies.
+  void SendingSyncMessage(const IPC::Message& msg);
 
  private:
-  const PP_Flash_Menu* pp_menu_;
-  bool own_menu_;
-  DISALLOW_COPY_AND_ASSIGN(SerializedFlashMenu);
+  // When we send a synchronous message (from untrusted to trusted), we store
+  // its type here, so that later we can associate the reply with its type
+  // and potentially translate handles in the message.
+  typedef std::map<int, uint32> PendingSyncMsgMap;
+  PendingSyncMsgMap pending_sync_msgs_;
+
+  DISALLOW_COPY_AND_ASSIGN(HandleConverter);
 };
 
 }  // namespace proxy
 }  // namespace ppapi
 
-#endif  // PPAPI_PROXY_SERIALIZED_FLASH_MENU_H_
+#endif  // PPAPI_PROXY_HANDLE_CONVERTER_H_