Pepper: Use base::Callback in RequestNexeFd.

components/nacl/renderer/ppb_nacl_private_impl.cc

Index: components/nacl/renderer/ppb_nacl_private_impl.cc
diff --git a/components/nacl/renderer/ppb_nacl_private_impl.cc b/components/nacl/renderer/ppb_nacl_private_impl.cc
index 048130aea5346020ad58e8ac1c52ce4fe6b86efb..c80d681c9afd3f1a6d6846289cb4a25edcdeccc9 100644
--- a/components/nacl/renderer/ppb_nacl_private_impl.cc
+++ b/components/nacl/renderer/ppb_nacl_private_impl.cc
@@ -587,6 +587,19 @@ PP_Bool PPIsNonSFIModeEnabled() {
   return PP_FromBool(IsNonSFIModeEnabled());
 }
 
+void GetNexeFdContinuation(struct PP_CompletionCallback callback,
+                           PP_Bool* out_is_hit,
+                           PP_FileHandle* out_handle,
+                           int32_t pp_error,
+                           bool is_hit,
+                           PP_FileHandle handle) {
+  if (pp_error == PP_OK) {
+    *out_is_hit = PP_FromBool(is_hit);
+    *out_handle = handle;
+  }
+  PostPPCompletionCallback(callback, pp_error);
+}
+
 int32_t GetNexeFd(PP_Instance instance,
                   const char* pexe_url,
                   uint32_t abi_version,
@@ -645,9 +658,7 @@ int32_t GetNexeFd(PP_Instance instance,
       GetRoutingID(instance),
       instance,
       cache_info,
-      is_hit,
-      handle,
-      enter.callback());
+      base::Bind(&GetNexeFdContinuation, callback, is_hit, handle));

[dmichael (off chromium), 2014/07/09 17:38:31]

Can you use TrackedCallback instead of PP_CompletionCallback here? I.e., pass
enter.callback() instead of just callback, and change the type in
GetNexeFdContinuation?

TrackedCallback normally expects that once it's created, it's the only thing
that invokes the callback. I think it happens to work in this case, but if the
Callback had a Resource associated with it, it would get confused not knowing
that the callback was invoked outside of it, and would end up calling the
callback with PP_ERROR_ABORTED eventually when the resource was destroyed.
That's what makes it "Tracked". In theory, we should  maybe be aborting at
instance shutdown for ones like this, but we don't right now.

You could even use TrackedCallback::set_completion_task if you want, then just
Bind to TrackedCallback::Run here. It's useful for real APIs to make sure that
out-params are set on the correct thread just before firing the callback, which
helps with some weird race conditions. Arguably overkill here. But in real APIs,
it's the "right" way.

If you strongly prefer not using TrackedCallback at all, you could instead go
back to your other approach of not using EnterInstance.

 
   return enter.SetResult(PP_OK_COMPLETIONPENDING);
 }