Remove the proxy callback tracker.

ppapi/proxy/ppb_url_loader_proxy.cc

 // Copyright (c) 2011 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 "ppapi/proxy/ppb_url_loader_proxy.h"
 
 #include <algorithm>
 #include <deque>
 #include <vector>
 
@@ skipping 93 matching lines @@
       PP_CompletionCallback callback) OVERRIDE;
   virtual void Close() OVERRIDE;
   virtual void GrantUniversalAccess() OVERRIDE;
   virtual void SetStatusCallback(
       PP_URLLoaderTrusted_StatusCallback cb) OVERRIDE;
 
   // Called when the browser has new up/download progress to report.
   void UpdateProgress(const PPBURLLoader_UpdateProgress_Params& params);
 
   // Called when the browser responds to our ReadResponseBody request.
-  void ReadResponseBodyAck(int32 result, const std::string& data);
+  void ReadResponseBodyAck(int32_t result, const std::string& data);
+
+  // Called when any callback other than the read callback has been executed.
+  void CallbackComplete(int32_t result);
 
  private:
   // Reads the give bytes out of the buffer_, placing them in the given output
   // buffer, and removes the bytes from the buffer.
   //
   // The size must be not more than the current size of the buffer.
   void PopBuffer(void* output_buffer, int32_t output_size);
 
   PluginDispatcher* GetDispatcher() const {
     return PluginDispatcher::GetForResource(this);
   }
 
   // Initialized to -1. Will be set to nonnegative values by the UpdateProgress
   // message when the values are known.
   int64_t bytes_sent_;
   int64_t total_bytes_to_be_sent_;
   int64_t bytes_received_;
   int64_t total_bytes_to_be_received_;
 
-  // When an asynchronous read is pending, this will contain the callback and
-  // the buffer to put the data.
-  PP_CompletionCallback current_read_callback_;
+  // Current completion callback for the current phase of loading. We have only
+  // one thing (open, follow redirect, read, etc.) out-standing at once.

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

s/-//

+  PP_CompletionCallback current_callback_;

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

Hrm, now the callbacks for different functions are no longer independent. Either
the error code reported is wrong (e.g., if you call FinishStreamingToFile with
an Open pending, you now get a PP_ERROR_INPROGRESS) or our semantics are
unclear/wrong (I haven't thought about whether it ever makes sense for several
things to be outstanding), or both.

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

I considered this but the impl does the same thing. It's not possible to wait
for more than one thing at once with this class.

+
+  // When an asynchronous read is pending, this will contain the buffer to put
+  // the data. The current_callback_ will identify the read callback.
   void* current_read_buffer_;
   int32_t current_read_buffer_size_;
 
   // A buffer of all the data that's been sent to us from the host that we
   // have yet to send out to the plugin.
   std::deque<char> buffer_;
 
   // Cached copy of the response info. When nonzero, we're holding a reference
   // to this resource.
   PP_Resource response_info_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(URLLoader);
 };
 
 URLLoader::URLLoader(const HostResource& resource)
     : Resource(resource),
       bytes_sent_(-1),
       total_bytes_to_be_sent_(-1),
       bytes_received_(-1),
       total_bytes_to_be_received_(-1),
-      current_read_callback_(PP_MakeCompletionCallback(NULL, NULL)),
+      current_callback_(PP_MakeCompletionCallback(NULL, NULL)),
       current_read_buffer_(NULL),
       current_read_buffer_size_(0),
       response_info_(0) {
 }
 
 URLLoader::~URLLoader() {
   // Always need to fire completion callbacks to prevent a leak in the plugin.
-  if (current_read_callback_.func) {
+  if (current_callback_.func) {
     // TODO(brettw) the callbacks at this level should be refactored with a
     // more automatic tracking system like we have in the renderer.
     MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
-        current_read_callback_.func, current_read_callback_.user_data,
+        current_callback_.func, current_callback_.user_data,
         static_cast<int32_t>(PP_ERROR_ABORTED)));
   }
 
   if (response_info_)
     PluginResourceTracker::GetInstance()->ReleaseResource(response_info_);
 }
 
 PPB_URLLoader_API* URLLoader::AsPPB_URLLoader_API() {
   return this;
 }
 
 int32_t URLLoader::Open(PP_Resource request_id,
                         PP_CompletionCallback callback) {
   EnterResourceNoLock<thunk::PPB_URLRequestInfo_API> enter(request_id, true);
   if (enter.failed())
     return PP_ERROR_BADRESOURCE;
 
-  // TODO(brettw) http://crbug.com/86279: SendCallback doesn't ensure that
-  // the proper callback semantics happen if the object is deleted.
+  if (current_callback_.func)
+    return PP_ERROR_INPROGRESS;
+
+  if (!callback.func)
+    return PP_ERROR_BLOCKS_MAIN_THREAD;
+  current_callback_ = callback;
+
   GetDispatcher()->Send(new PpapiHostMsg_PPBURLLoader_Open(
-      INTERFACE_ID_PPB_URL_LOADER, host_resource(), enter.object()->GetData(),
-      GetDispatcher()->callback_tracker().SendCallback(callback)));
+      INTERFACE_ID_PPB_URL_LOADER, host_resource(), enter.object()->GetData()));
   return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t URLLoader::FollowRedirect(PP_CompletionCallback callback) {
-  // TODO(brettw) http://crbug.com/86279: SendCallback doesn't ensure that
-  // the proper callback semantics happen if the object is deleted.
+  if (current_callback_.func)
+    return PP_ERROR_INPROGRESS;
+
+  if (!callback.func)
+    return PP_ERROR_BLOCKS_MAIN_THREAD;
+  current_callback_ = callback;
+
   GetDispatcher()->Send(new PpapiHostMsg_PPBURLLoader_FollowRedirect(
-      INTERFACE_ID_PPB_URL_LOADER, host_resource(),
-      GetDispatcher()->callback_tracker().SendCallback(callback)));
+      INTERFACE_ID_PPB_URL_LOADER, host_resource()));
   return PP_OK_COMPLETIONPENDING;
 }
 
 PP_Bool URLLoader::GetUploadProgress(int64_t* bytes_sent,
                                      int64_t* total_bytes_to_be_sent) {
   if (bytes_sent_ == -1) {
     *bytes_sent = 0;
     *total_bytes_to_be_sent = 0;
     return PP_FALSE;
   }
@@ skipping 30 matching lines @@
   // The caller expects to get a ref, and we want to keep holding ours.
   PluginResourceTracker::GetInstance()->AddRefResource(response_info_);
   return response_info_;
 }
 
 int32_t URLLoader::ReadResponseBody(void* buffer,
                                     int32_t bytes_to_read,
                                     PP_CompletionCallback callback) {
   if (!buffer || bytes_to_read <= 0)
     return PP_ERROR_BADARGUMENT;  // Must specify an output buffer.
-  if (current_read_callback_.func)
+  if (current_callback_.func)
     return PP_ERROR_INPROGRESS;  // Can only have one request pending.
 
   // Currently we don't support sync calls to read. We'll need to revisit
   // how this works when we allow blocking calls (from background threads).
   if (!callback.func)
     return PP_ERROR_BADARGUMENT;
 
   if (static_cast<size_t>(bytes_to_read) <= buffer_.size()) {
     // Special case: we've buffered enough data to be able to synchronously
     // return data to the caller. Do so without making IPCs.
     PopBuffer(buffer, bytes_to_read);
     return bytes_to_read;
   }
 
-  current_read_callback_ = callback;
+  current_callback_ = callback;
   current_read_buffer_ = buffer;
   current_read_buffer_size_ = bytes_to_read;
 
   GetDispatcher()->Send(new PpapiHostMsg_PPBURLLoader_ReadResponseBody(
       INTERFACE_ID_PPB_URL_LOADER, host_resource(), bytes_to_read));
   return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t URLLoader::FinishStreamingToFile(PP_CompletionCallback callback) {
+  if (current_callback_.func)
+    return PP_ERROR_INPROGRESS;
+
+  if (!callback.func)
+    return PP_ERROR_BLOCKS_MAIN_THREAD;
+  current_callback_ = callback;
+
   GetDispatcher()->Send(new PpapiHostMsg_PPBURLLoader_FinishStreamingToFile(
-      INTERFACE_ID_PPB_URL_LOADER, host_resource(),
-      GetDispatcher()->callback_tracker().SendCallback(callback)));
+      INTERFACE_ID_PPB_URL_LOADER, host_resource()));
   return PP_OK_COMPLETIONPENDING;
 }
 
 void URLLoader::Close() {
   GetDispatcher()->Send(new PpapiHostMsg_PPBURLLoader_Close(
       INTERFACE_ID_PPB_URL_LOADER, host_resource()));
 }
 
 void URLLoader::GrantUniversalAccess() {
   GetDispatcher()->Send(
       new PpapiHostMsg_PPBURLLoader_GrantUniversalAccess(
           INTERFACE_ID_PPB_URL_LOADER, host_resource()));
 }
 
 void URLLoader::SetStatusCallback(
     PP_URLLoaderTrusted_StatusCallback cb) {
   // Not implemented in the proxied version, this is for implementing the
   // proxy itself in the host.
 }
 
 void URLLoader::UpdateProgress(
     const PPBURLLoader_UpdateProgress_Params& params) {
   bytes_sent_ = params.bytes_sent;
   total_bytes_to_be_sent_ = params.total_bytes_to_be_sent;
   bytes_received_ = params.bytes_received;
   total_bytes_to_be_received_ = params.total_bytes_to_be_received;
 }
 
 void URLLoader::ReadResponseBodyAck(int32 result, const std::string& data) {
-  if (!current_read_callback_.func || !current_read_buffer_) {
+  if (!current_callback_.func || !current_read_buffer_) {
     NOTREACHED();
     return;
   }
 
   // Append the data we requested to the internal buffer.
   // TODO(brettw) avoid double-copying data that's coming from IPC and going
   // into the plugin buffer (we can skip the internal buffer in this case).
   buffer_.insert(buffer_.end(), data.begin(), data.end());
 
   if (result >= 0) {
     // Fill the user buffer. We may get fewer bytes than requested in the
     // case of stream end.
     int32_t bytes_to_return = std::min(current_read_buffer_size_,
                                        static_cast<int32_t>(buffer_.size()));
     PopBuffer(current_read_buffer_, bytes_to_return);
     result = bytes_to_return;
   }
 
   // The plugin should be able to make a new request from their callback, so
   // we have to clear our copy first.
-  PP_RunAndClearCompletionCallback(&current_read_callback_, result);
+  PP_RunAndClearCompletionCallback(&current_callback_, result);
+}
+
+void URLLoader::CallbackComplete(int32_t result) {
+  PP_RunAndClearCompletionCallback(&current_callback_, result);
 }
 
 void URLLoader::PopBuffer(void* output_buffer, int32_t output_size) {
   CHECK(output_size <= static_cast<int32_t>(buffer_.size()));
   std::copy(buffer_.begin(),
             buffer_.begin() + output_size,
             static_cast<char*>(output_buffer));
   buffer_.erase(buffer_.begin(),
                 buffer_.begin() + output_size);
 }
@@ skipping 91 matching lines @@
                                       HostResource* result) {
   thunk::EnterResourceCreation enter(instance);
   if (enter.succeeded()) {
     result->SetHostResource(instance,
                             enter.functions()->CreateURLLoader(instance));
     PrepareURLLoaderForSendingToPlugin(result->host_resource());
   }
 }
 
 void PPB_URLLoader_Proxy::OnMsgOpen(const HostResource& loader,
-                                    const PPB_URLRequestInfo_Data& data,
-                                    uint32_t serialized_callback) {
-  // Have to be careful to always issue the callback, so don't return early.
-  EnterHostFromHostResource<PPB_URLLoader_API> enter(loader);
+                                    const PPB_URLRequestInfo_Data& data) {
+  EnterHostFromHostResourceForceCallback<PPB_URLLoader_API> enter(
+      loader, callback_factory_, &PPB_URLLoader_Proxy::OnCallback, loader);
   thunk::EnterResourceCreation enter_creation(loader.instance());
+  if (enter.failed() || enter_creation.failed())
+    return;
 
-  PP_CompletionCallback callback = ReceiveCallback(serialized_callback);
-
-  int32_t result = PP_ERROR_BADRESOURCE;
-  if (enter.succeeded() && enter_creation.succeeded()) {
-    ScopedPPResource request_resource(
-        ScopedPPResource::PassRef(),
-        enter_creation.functions()->CreateURLRequestInfo(loader.instance(),
-                                                         data));
-    result = enter.object()->Open(request_resource, callback);
-  }
-  if (result != PP_OK_COMPLETIONPENDING)
-    PP_RunCompletionCallback(&callback, result);
+  ScopedPPResource request_resource(
+      ScopedPPResource::PassRef(),
+      enter_creation.functions()->CreateURLRequestInfo(loader.instance(),
+                                                       data));
+  enter.SetResult(enter.object()->Open(request_resource, enter.callback()));
   // TODO(brettw) bug 73236 register for the status callbacks.
 }
 
 void PPB_URLLoader_Proxy::OnMsgFollowRedirect(
-    const HostResource& loader,
-    uint32_t serialized_callback) {
-  EnterHostFromHostResource<PPB_URLLoader_API> enter(loader);
-  PP_CompletionCallback callback = ReceiveCallback(serialized_callback);
-  int32_t result = PP_ERROR_BADRESOURCE;
+    const HostResource& loader) {
+  EnterHostFromHostResourceForceCallback<PPB_URLLoader_API> enter(
+      loader, callback_factory_, &PPB_URLLoader_Proxy::OnCallback, loader);
   if (enter.succeeded())
-    result = enter.object()->FollowRedirect(callback);
-  if (result != PP_OK_COMPLETIONPENDING)
-    PP_RunCompletionCallback(&callback, result);
+    enter.SetResult(enter.object()->FollowRedirect(enter.callback()));
 }
 
 void PPB_URLLoader_Proxy::OnMsgGetResponseInfo(const HostResource& loader,
                                                HostResource* result) {
   EnterHostFromHostResource<PPB_URLLoader_API> enter(loader);
   if (enter.succeeded()) {
     result->SetHostResource(loader.instance(),
                             enter.object()->GetResponseInfo());
   }
 }
@@ skipping 34 matching lines @@
   EnterHostFromHostResourceForceCallback<PPB_URLLoader_API> enter(
       loader, callback_factory_, &PPB_URLLoader_Proxy::OnReadCallback, info);
   if (enter.succeeded()) {
     enter.SetResult(enter.object()->ReadResponseBody(
         const_cast<char*>(info->read_buffer.c_str()),
         bytes_to_read, enter.callback()));
   }
 }
 
 void PPB_URLLoader_Proxy::OnMsgFinishStreamingToFile(
-    const HostResource& loader,
-    uint32_t serialized_callback) {
-  EnterHostFromHostResource<PPB_URLLoader_API> enter(loader);
-  PP_CompletionCallback callback = ReceiveCallback(serialized_callback);
-  int32_t result = PP_ERROR_BADRESOURCE;
-  if (enter.succeeded())
-    result = enter.object()->FinishStreamingToFile(callback);
-  if (result != PP_OK_COMPLETIONPENDING)
-    PP_RunCompletionCallback(&callback, result);
+    const HostResource& loader) {
+  EnterHostFromHostResourceForceCallback<PPB_URLLoader_API> enter(
+      loader, callback_factory_, &PPB_URLLoader_Proxy::OnCallback, loader);
+  if (enter.failed())
+    enter.SetResult(enter.object()->FinishStreamingToFile(enter.callback()));;
 }
 
 void PPB_URLLoader_Proxy::OnMsgClose(const HostResource& loader) {
   EnterHostFromHostResource<PPB_URLLoader_API> enter(loader);
   if (enter.succeeded())
     enter.object()->Close();
 }
 
 void PPB_URLLoader_Proxy::OnMsgGrantUniversalAccess(
     const HostResource& loader) {
@@ skipping 13 matching lines @@
 // Called in the Plugin.
 void PPB_URLLoader_Proxy::OnMsgReadResponseBodyAck(
     const HostResource& host_resource,
     int32 result,
     const std::string& data) {
   EnterPluginFromHostResource<PPB_URLLoader_API> enter(host_resource);
   if (enter.succeeded())
     static_cast<URLLoader*>(enter.object())->ReadResponseBodyAck(result, data);
 }
 
+// Called in the plugin.
+void PPB_URLLoader_Proxy::OnMsgCallbackComplete(
+    const HostResource& host_resource,
+    int32_t result) {
+  EnterPluginFromHostResource<PPB_URLLoader_API> enter(host_resource);
+  if (enter.succeeded())
+    static_cast<URLLoader*>(enter.object())->CallbackComplete(result);
+}
+
 void PPB_URLLoader_Proxy::OnReadCallback(int32_t result,
                                          ReadCallbackInfo* info) {
   int32_t bytes_read = 0;
   if (result > 0)
     bytes_read = result;  // Positive results indicate bytes read.
   info->read_buffer.resize(bytes_read);
 
   dispatcher()->Send(new PpapiMsg_PPBURLLoader_ReadResponseBody_Ack(
       INTERFACE_ID_PPB_URL_LOADER, info->resource, result, info->read_buffer));
 
   delete info;
 }
 
+void PPB_URLLoader_Proxy::OnCallback(int32_t result,
+                                     const HostResource& resource) {
+  dispatcher()->Send(new PpapiMsg_PPBURLLoader_CallbackComplete(
+      INTERFACE_ID_PPB_URL_LOADER, resource, result));
+}
+
 }  // namespace proxy
 }  // namespace ppapi