Implement open_resource in non-SFI mode.

ppapi/native_client/src/trusted/plugin/service_runtime.cc

Index: ppapi/native_client/src/trusted/plugin/service_runtime.cc
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.cc b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
index e9eba183482bfb1e79484ddcf0d4079107602b77..952176da658c8c930eafc011b1566c273df13bde 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.cc
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
@@ -64,6 +64,50 @@ namespace {
 // should be plenty for static data
 const int64_t kMaxTempQuota = 0x8000000;
 
+class OpenManifestEntryAsyncCallback {
+ public:
+  OpenManifestEntryAsyncCallback(PP_FileHandle* file_handle,
+                                 const pp::CompletionCallback& callback)
+      : file_handle_(file_handle),
+        callback_(callback) {
+  }
+
+  ~OpenManifestEntryAsyncCallback() {
+    if (callback_.pp_completion_callback().func)
+      callback_.RunAndClear(PP_ERROR_ABORTED);
+  }
+
+  void Run(int32_t pp_error) {
+#if defined(OS_WIN)
+    // Currently, this is used only for non-SFI mode, and now the mode is not
+    // supproted on windows.

[dmichael (off chromium), 2014/04/25 21:06:58]

supproted->supported

[hidehiko, 2014/04/28 08:44:27]

Done.

+    // TODO(hidehiko): Support it on Windows when we switch to use
+    // ManifestService also in SFI-mode.
+    NACL_NOTREACHED();
+#elif defined(OS_POSIX)
+    // On posix, PlatformFile is the file descriptor.
+    *file_handle_ = (pp_error == PP_OK) ? info_.desc : -1;
+#endif
+    callback_.RunAndClear(pp_error);
+    delete this;
+  }
+
+  NaClFileInfo* mutable_info() { return &info_; }
+  pp::CompletionCallback pp_completion_callback() {

[dmichael (off chromium), 2014/04/25 21:06:58]

I find this confusing, that this class deals with 2 PP_CompletionCallbacks.
1) callback_ appears to be the one provided by Chrome when it invoked
OpenResource. We don't use PP_CompletionCallback to make calls in to the
browser, so I would prefer just a new function on PPB_NaCl_Private (it can
disambiguate with a user_data parameter or something).
2) The one returned by pp_completion_callback, which seems to be just a way to
provide a function pointer and some associated stuff? It's not actually passed
to Pepper. If you use a new PPB function to talk back to Chrome, you probably
don't need this one anymore.

[hidehiko, 2014/04/28 08:44:27]

Done, but to be honest it looks slightly layer-violation to me. I think we can
clean them up, when we switch the SFI-mode irt_open_resource() to this
implementation later, though.

+    return pp::CompletionCallback(&RunTrampoline, this);
+  }
+
+ private:
+  static void RunTrampoline(void* user_data, int32_t pp_error) {
+    static_cast<OpenManifestEntryAsyncCallback*>(user_data)->Run(pp_error);
+  }
+
+  PP_FileHandle* file_handle_;
+  NaClFileInfo info_;
+  pp::CompletionCallback callback_;
+  DISALLOW_COPY_AND_ASSIGN(OpenManifestEntryAsyncCallback);
+};
+
 class ManifestService {
  public:
   ManifestService(nacl::WeakRefAnchor* anchor,
@@ -92,6 +136,23 @@ class ManifestService {
     return true;
   }
 
+  bool OpenResource(const char* entry_key, PP_FileHandle* file,
+                    const pp::CompletionCallback& callback) {
+    // Release this instance if the ServiceRuntime is already destructed.
+    if (anchor_->is_abandoned()) {
+      delete this;
+      return false;
+    }
+
+    OpenManifestEntryAsyncCallback* open_manifest_callback =
+        new OpenManifestEntryAsyncCallback(file, callback);
+    plugin_reverse_->OpenManifestEntryAsync(
+        entry_key,
+        open_manifest_callback->mutable_info(),
+        open_manifest_callback->pp_completion_callback());
+    return true;
+  }
+
   static PP_Bool QuitTrampoline(void* user_data) {
     return PP_FromBool(static_cast<ManifestService*>(user_data)->Quit());
   }
@@ -101,6 +162,15 @@ class ManifestService {
                        StartupInitializationComplete());
   }
 
+  static PP_Bool OpenResourceTrampoline(
+      void* user_data, const char* entry_key, PP_FileHandle* file,
+      struct PP_CompletionCallback callback) {
+    return PP_FromBool(
+        static_cast<ManifestService*>(user_data)->OpenResource(
+            entry_key, file,
+            pp::CompletionCallback(callback.func, callback.user_data)));
+  }
+
  private:
   // Weak reference to check if plugin_reverse is legally accessible or not.
   nacl::WeakRefAnchor* anchor_;
@@ -110,9 +180,10 @@ class ManifestService {
 };
 
 // Vtable to pass functions to LaunchSelLdr.
-const PP_ManifestService kManifestServiceVTable = {
+const PPP_ManifestService kManifestServiceVTable = {
   &ManifestService::QuitTrampoline,
   &ManifestService::StartupInitializationCompleteTrampoline,
+  &ManifestService::OpenResourceTrampoline,
 };
 
 }  // namespace
@@ -200,7 +271,8 @@ bool PluginReverseInterface::OpenManifestEntry(nacl::string url_key,
   // the main thread before this function can return. The pointers it contains
   // to stack variables will not leak.
   OpenManifestEntryResource* to_open =
-      new OpenManifestEntryResource(url_key, info, &op_complete);
+      new OpenManifestEntryResource(url_key, info, &op_complete,
+                                    pp::CompletionCallback());
   CHECK(to_open != NULL);
   NaClLog(4, "PluginReverseInterface::OpenManifestEntry: %s\n",
           url_key.c_str());
@@ -250,6 +322,16 @@ bool PluginReverseInterface::OpenManifestEntry(nacl::string url_key,
   return true;
 }
 
+void PluginReverseInterface::OpenManifestEntryAsync(
+    const nacl::string& entry_key,
+    struct NaClFileInfo* info,
+    const pp::CompletionCallback& callback) {
+  bool op_complete = false;
+  OpenManifestEntryResource to_open(
+      entry_key, info, &op_complete, callback);
+  OpenManifestEntry_MainThreadContinuation(&to_open, PP_OK);
+}
+
 // Transfer point from OpenManifestEntry() which runs on the main thread
 // (Some PPAPI actions -- like StreamAsFile -- can only run on the main thread).
 // OpenManifestEntry() is waiting on a condvar for this continuation to
@@ -276,10 +358,14 @@ void PluginReverseInterface::OpenManifestEntry_MainThreadContinuation(
             error_info.message().c_str());
     // Failed, and error_info has the details on what happened.  Wake
     // up requesting thread -- we are done.
-    nacl::MutexLocker take(&mu_);
-    *p->op_complete_ptr = true;  // done...
-    p->file_info->desc = -1;  // but failed.
-    NaClXCondVarBroadcast(&cv_);
+    {
+      nacl::MutexLocker take(&mu_);
+      *p->op_complete_ptr = true;  // done...
+      p->file_info->desc = -1;  // but failed.
+      NaClXCondVarBroadcast(&cv_);
+    }
+    if (p->callback.pp_completion_callback().func)
+      p->callback.RunAndClear(PP_OK);
     return;
   }
   NaClLog(4,
@@ -292,10 +378,14 @@ void PluginReverseInterface::OpenManifestEntry_MainThreadContinuation(
     NaClLog(4,
             "OpenManifestEntry_MainThreadContinuation: "
             "Requires PNaCl translation -- not supported\n");
-    nacl::MutexLocker take(&mu_);
-    *p->op_complete_ptr = true;  // done...
-    p->file_info->desc = -1;  // but failed.
-    NaClXCondVarBroadcast(&cv_);
+    {
+      nacl::MutexLocker take(&mu_);
+      *p->op_complete_ptr = true;  // done...
+      p->file_info->desc = -1;  // but failed.
+      NaClXCondVarBroadcast(&cv_);
+    }
+    if (p->callback.pp_completion_callback().func)
+      p->callback.RunAndClear(PP_OK);
     return;
   }
 
@@ -313,14 +403,18 @@ void PluginReverseInterface::OpenManifestEntry_MainThreadContinuation(
               "OpenManifestEntry_MainThreadContinuation: "
               "GetReadonlyPnaclFd failed\n");
     }
-    nacl::MutexLocker take(&mu_);
-    *p->op_complete_ptr = true;  // done!
-    // TODO(ncbray): enable the fast loading and validation paths for this
-    // type of file.
-    p->file_info->desc = fd;
-    NaClXCondVarBroadcast(&cv_);
+    {
+      nacl::MutexLocker take(&mu_);
+      *p->op_complete_ptr = true;  // done!
+      // TODO(ncbray): enable the fast loading and validation paths for this
+      // type of file.
+      p->file_info->desc = fd;
+      NaClXCondVarBroadcast(&cv_);
+    }
     NaClLog(4,
             "OpenManifestEntry_MainThreadContinuation: GetPnaclFd okay\n");
+    if (p->callback.pp_completion_callback().func)
+      p->callback.RunAndClear(PP_OK);
     return;
   }
 
@@ -330,6 +424,9 @@ void PluginReverseInterface::OpenManifestEntry_MainThreadContinuation(
   // to create another instance.
   OpenManifestEntryResource* open_cont = new OpenManifestEntryResource(*p);
   open_cont->url = mapped_url;
+  // Callback is now delegated from p to open_cont. So, here we manually clear
+  // complete callback.
+  p->callback = pp::CompletionCallback();
   pp::CompletionCallback stream_cc = WeakRefNewCallback(
       anchor_,
       this,
@@ -357,22 +454,27 @@ void PluginReverseInterface::StreamAsFile_MainThreadContinuation(
   NaClLog(4,
           "Entered StreamAsFile_MainThreadContinuation\n");
 
-  nacl::MutexLocker take(&mu_);
-  if (result == PP_OK) {
-    NaClLog(4, "StreamAsFile_MainThreadContinuation: GetFileInfo(%s)\n",
-            p->url.c_str());
-    *p->file_info = plugin_->GetFileInfo(p->url);
+  {
+    nacl::MutexLocker take(&mu_);
+    if (result == PP_OK) {
+      NaClLog(4, "StreamAsFile_MainThreadContinuation: GetFileInfo(%s)\n",
+              p->url.c_str());
+      *p->file_info = plugin_->GetFileInfo(p->url);
 
-    NaClLog(4,
-            "StreamAsFile_MainThreadContinuation: PP_OK, desc %d\n",
-            p->file_info->desc);
-  } else {
-    NaClLog(4,
-            "StreamAsFile_MainThreadContinuation: !PP_OK, setting desc -1\n");
-    p->file_info->desc = -1;
+      NaClLog(4,
+              "StreamAsFile_MainThreadContinuation: PP_OK, desc %d\n",
+              p->file_info->desc);
+    } else {
+      NaClLog(
+          4,
+          "StreamAsFile_MainThreadContinuation: !PP_OK, setting desc -1\n");
+      p->file_info->desc = -1;
+    }
+    *p->op_complete_ptr = true;
+    NaClXCondVarBroadcast(&cv_);
   }
-  *p->op_complete_ptr = true;
-  NaClXCondVarBroadcast(&cv_);
+  if (p->callback.pp_completion_callback().func)
+    p->callback.RunAndClear(PP_OK);
 }
 
 bool PluginReverseInterface::CloseManifestEntry(int32_t desc) {