Remove the proxy callback tracker.

ppapi/proxy/ppb_file_ref_proxy.cc

Index: ppapi/proxy/ppb_file_ref_proxy.cc
diff --git a/ppapi/proxy/ppb_file_ref_proxy.cc b/ppapi/proxy/ppb_file_ref_proxy.cc
index 49a113195d309180654293ea37f3cfda3a685961..3aa88f2de4ddbb1c919e0a62e35f6bb9cc9a4bee 100644
--- a/ppapi/proxy/ppb_file_ref_proxy.cc
+++ b/ppapi/proxy/ppb_file_ref_proxy.cc
@@ -4,6 +4,9 @@
 
 #include "ppapi/proxy/ppb_file_ref_proxy.h"
 
+#include <map>
+
+#include "base/bind.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/ppb_file_ref.h"
 #include "ppapi/c/private/ppb_proxy_private.h"
@@ -40,19 +43,46 @@ class FileRef : public FileRefImpl {
   virtual int32_t Rename(PP_Resource new_file_ref,
                          PP_CompletionCallback callback) OVERRIDE;
 
+  // Executes the pending callback with the given ID. See pending_callbacks_.
+  void ExecuteCallback(int callback_id, int32_t result);
+
  private:
   PluginDispatcher* GetDispatcher() const {
     return PluginDispatcher::GetForResource(this);
   }
 
+  // Adds a callback to the list and returns its ID. Returns 0 if the callback
+  // is invalid.
+  int SendCallback(PP_CompletionCallback callback);
+
+  // This class can have any number of out-standing requests with completion
+  // callbacks, in contrast to most resources which have one possible pending
+  // callback pending (like a Flush callback).
+  //
+  // To keep track of them, assign integer IDs to the callbacks, which is how
+  // the callback will be identified when it's passed to the host and then
+  // back here.
+  int next_callback_id_;

[viettrungluu, 2011/10/17 17:11:03]

Seems bad that things will fail when this wraps, which isn't that inconceivable.

[brettw, 2011/10/17 19:49:20]

I added a loop to check for duplicates.

+  typedef std::map<int, PP_CompletionCallback> PendingCallbackMap;
+  PendingCallbackMap pending_callbacks_;
+
   DISALLOW_IMPLICIT_CONSTRUCTORS(FileRef);
 };
 
 FileRef::FileRef(const PPB_FileRef_CreateInfo& info)
-    : FileRefImpl(FileRefImpl::InitAsProxy(), info) {
+    : FileRefImpl(FileRefImpl::InitAsProxy(), info),
+      next_callback_id_(1) {
 }
 
 FileRef::~FileRef() {
+  // Abort all pending callbacks. Do this by posting a task to avoid reentering
+  // the plugin's Release() call that probably deleted this object.
+  for (PendingCallbackMap::iterator i = pending_callbacks_.begin();
+       i != pending_callbacks_.end(); ++i) {
+    MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
+        i->second.func, i->second.user_data,
+        static_cast<int32_t>(PP_ERROR_ABORTED)));
+  }
 }
 
 PP_Resource FileRef::GetParent() {
@@ -64,31 +94,44 @@ PP_Resource FileRef::GetParent() {
 
 int32_t FileRef::MakeDirectory(PP_Bool make_ancestors,
                                PP_CompletionCallback callback) {
+  int callback_id = SendCallback(callback);
+  if (!callback_id)
+    return PP_ERROR_BADARGUMENT;
+
   GetDispatcher()->Send(new PpapiHostMsg_PPBFileRef_MakeDirectory(
-      INTERFACE_ID_PPB_FILE_REF, host_resource(), make_ancestors,
-      GetDispatcher()->callback_tracker().SendCallback(callback)));
+      INTERFACE_ID_PPB_FILE_REF, host_resource(), make_ancestors, callback_id));
   return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t FileRef::Touch(PP_Time last_access_time,
                        PP_Time last_modified_time,
                        PP_CompletionCallback callback) {
+  int callback_id = SendCallback(callback);
+  if (!callback_id)
+    return PP_ERROR_BADARGUMENT;
+
   GetDispatcher()->Send(new PpapiHostMsg_PPBFileRef_Touch(
       INTERFACE_ID_PPB_FILE_REF, host_resource(),
-      last_access_time, last_modified_time,
-      GetDispatcher()->callback_tracker().SendCallback(callback)));
+      last_access_time, last_modified_time, callback_id));
   return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t FileRef::Delete(PP_CompletionCallback callback) {
+  int callback_id = SendCallback(callback);
+  if (!callback_id)
+    return PP_ERROR_BADARGUMENT;
+
   GetDispatcher()->Send(new PpapiHostMsg_PPBFileRef_Delete(
-      INTERFACE_ID_PPB_FILE_REF, host_resource(),
-      GetDispatcher()->callback_tracker().SendCallback(callback)));
+      INTERFACE_ID_PPB_FILE_REF, host_resource(), callback_id));
   return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t FileRef::Rename(PP_Resource new_file_ref,
                         PP_CompletionCallback callback) {
+  int callback_id = SendCallback(callback);
+  if (!callback_id)
+    return PP_ERROR_BADARGUMENT;
+
   Resource* new_file_ref_object =
       PluginResourceTracker::GetInstance()->GetResource(new_file_ref);
   if (!new_file_ref_object ||
@@ -97,13 +140,31 @@ int32_t FileRef::Rename(PP_Resource new_file_ref,
 
   GetDispatcher()->Send(new PpapiHostMsg_PPBFileRef_Rename(
       INTERFACE_ID_PPB_FILE_REF, host_resource(),
-      new_file_ref_object->host_resource(),
-      GetDispatcher()->callback_tracker().SendCallback(callback)));
+      new_file_ref_object->host_resource(), callback_id));
   return PP_OK_COMPLETIONPENDING;
 }
 
+void FileRef::ExecuteCallback(int callback_id, int32_t result) {
+  PendingCallbackMap::iterator found = pending_callbacks_.find(callback_id);
+  if (found == pending_callbacks_.end())
+    return;

[viettrungluu, 2011/10/17 17:11:03]

Is this really an expected case? Should it ever be reached?

[brettw, 2011/10/17 19:49:20]

I think this happens a bunch, I added a comment.

+
+  // Executing the callback may mutate the callback list.
+  PP_CompletionCallback callback = found->second;
+  pending_callbacks_.erase(found);
+  PP_RunCompletionCallback(&callback, result);
+}
+
+int FileRef::SendCallback(PP_CompletionCallback callback) {
+  if (!callback.func)
+    return 0;
+  pending_callbacks_[next_callback_id_] = callback;
+  return next_callback_id_++;
+}
+
 PPB_FileRef_Proxy::PPB_FileRef_Proxy(Dispatcher* dispatcher)
-    : InterfaceProxy(dispatcher) {
+    : InterfaceProxy(dispatcher),
+      callback_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
 }
 
 PPB_FileRef_Proxy::~PPB_FileRef_Proxy() {
@@ -177,51 +238,67 @@ void PPB_FileRef_Proxy::OnMsgGetParent(const HostResource& host_resource,
 
 void PPB_FileRef_Proxy::OnMsgMakeDirectory(const HostResource& host_resource,
                                            PP_Bool make_ancestors,
-                                           uint32_t serialized_callback) {
-  EnterHostFromHostResource<PPB_FileRef_API> enter(host_resource);
-  if (enter.failed())
-    return;
-  PP_CompletionCallback callback = ReceiveCallback(serialized_callback);
-  int32_t result = enter.object()->MakeDirectory(make_ancestors, callback);
-  if (result != PP_OK_COMPLETIONPENDING)
-    PP_RunCompletionCallback(&callback, result);
+                                           int callback_id) {
+  EnterHostFromHostResourceForceCallback<PPB_FileRef_API> enter(
+      host_resource, callback_factory_,
+      &PPB_FileRef_Proxy::OnCallbackCompleteInHost, host_resource, callback_id);
+  if (enter.succeeded()) {
+    enter.SetResult(enter.object()->MakeDirectory(make_ancestors,
+                                                  enter.callback()));
+  }
 }
 
 void PPB_FileRef_Proxy::OnMsgTouch(const HostResource& host_resource,
                                    PP_Time last_access,
                                    PP_Time last_modified,
-                                   uint32_t serialized_callback) {
-  EnterHostFromHostResource<PPB_FileRef_API> enter(host_resource);
-  if (enter.failed())
-    return;
-  PP_CompletionCallback callback = ReceiveCallback(serialized_callback);
-  int32_t result = enter.object()->Touch(last_access, last_modified, callback);
-  if (result != PP_OK_COMPLETIONPENDING)
-    PP_RunCompletionCallback(&callback, result);
+                                   int callback_id) {
+  EnterHostFromHostResourceForceCallback<PPB_FileRef_API> enter(
+      host_resource, callback_factory_,
+      &PPB_FileRef_Proxy::OnCallbackCompleteInHost, host_resource, callback_id);
+  if (enter.succeeded()) {
+    enter.SetResult(enter.object()->Touch(last_access, last_modified,
+                                          enter.callback()));
+  }
 }
 
 void PPB_FileRef_Proxy::OnMsgDelete(const HostResource& host_resource,
-                                    uint32_t serialized_callback) {
-  EnterHostFromHostResource<PPB_FileRef_API> enter(host_resource);
-  if (enter.failed())
-    return;
-  PP_CompletionCallback callback = ReceiveCallback(serialized_callback);
-  int32_t result = enter.object()->Delete(callback);
-  if (result != PP_OK_COMPLETIONPENDING)
-    PP_RunCompletionCallback(&callback, result);
+                                    int callback_id) {
+  EnterHostFromHostResourceForceCallback<PPB_FileRef_API> enter(
+      host_resource, callback_factory_,
+      &PPB_FileRef_Proxy::OnCallbackCompleteInHost, host_resource, callback_id);
+  if (enter.succeeded())
+    enter.SetResult(enter.object()->Delete(enter.callback()));
 }
 
 void PPB_FileRef_Proxy::OnMsgRename(const HostResource& file_ref,
                                     const HostResource& new_file_ref,
-                                    uint32_t serialized_callback) {
-  EnterHostFromHostResource<PPB_FileRef_API> enter(file_ref);
-  if (enter.failed())
-    return;
-  PP_CompletionCallback callback = ReceiveCallback(serialized_callback);
-  int32_t result = enter.object()->Rename(new_file_ref.host_resource(),
-                                          callback);
-  if (result != PP_OK_COMPLETIONPENDING)
-    PP_RunCompletionCallback(&callback, result);
+                                    int callback_id) {
+  EnterHostFromHostResourceForceCallback<PPB_FileRef_API> enter(
+      file_ref, callback_factory_,
+      &PPB_FileRef_Proxy::OnCallbackCompleteInHost, file_ref, callback_id);
+  if (enter.succeeded()) {
+    enter.SetResult(enter.object()->Rename(new_file_ref.host_resource(),
+                                           enter.callback()));
+  }
+}
+
+void PPB_FileRef_Proxy::OnMsgCallbackComplete(
+    const HostResource& host_resource,
+    int callback_id,
+    int32_t result) {
+  // Forward the callback info to the plugin resource.
+  EnterPluginFromHostResource<PPB_FileRef_API> enter(host_resource);
+  if (enter.succeeded())
+    static_cast<FileRef*>(enter.object())->ExecuteCallback(callback_id, result);
+}
+
+void PPB_FileRef_Proxy::OnCallbackCompleteInHost(
+    int32_t result,
+    const HostResource& host_resource,
+    int callback_id) {
+  // Execute OnMsgCallbackComplete in the plugin process.
+  Send(new PpapiMsg_PPBFileRef_CallbackComplete(
+      INTERFACE_ID_PPB_FILE_REF, host_resource, callback_id, result));
 }
 
 }  // namespace proxy