Pepper: Use base::Callback in RequestNexeFd.

components/nacl/renderer/ppb_nacl_private_impl.cc

 // Copyright 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.
 
 #include "components/nacl/renderer/ppb_nacl_private_impl.h"
 
 #include <numeric>
 #include <string>
 #include <vector>
 
@@ skipping 569 matching lines @@
   if(!sender->Send(new NaClHostMsg_NaClGetNumProcessors(&num_processors))) {
     return 1;
   }
   return num_processors;
 }
 
 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,
                   uint32_t opt_level,
                   const char* http_headers_param,
                   const char* extra_flags,
                   PP_Bool* is_hit,
                   PP_FileHandle* handle,
                   struct PP_CompletionCallback callback) {
   ppapi::thunk::EnterInstance enter(instance, callback);
@@ skipping 38 matching lines @@
   cache_info.last_modified = last_modified_time;
   cache_info.etag = etag;
   cache_info.has_no_store_header = has_no_store_header;
   cache_info.sandbox_isa = GetSandboxArch();
   cache_info.extra_flags = std::string(extra_flags);
 
   g_pnacl_resource_host.Get()->RequestNexeFd(
       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);
 }
 
 void ReportTranslationFinished(PP_Instance instance,
                                PP_Bool success,
                                int32_t opt_level,
                                int64_t pexe_size,
                                int64_t compile_time_us) {
   if (success == PP_TRUE) {
@@ skipping 978 matching lines @@
   &SetPNaClStartTime
 };
 
 }  // namespace
 
 const PPB_NaCl_Private* GetNaClPrivateInterface() {
   return &nacl_interface;
 }
 
 }  // namespace nacl