Using thread safe reference counting to wrap the UploadData instance.Moving the post_data_len_ member variable to the PluginUrlRequest class.TEST=This is a tentative fix for bug 25531.BUG=25531

Review URL: http://codereview.chromium.org/340006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@30432 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 62 insertions, 38 deletions

diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc
index 27461d3..13a7339 100644
--- a/chrome_frame/chrome_frame_automation.cc
+++ b/chrome_frame/chrome_frame_automation.cc
@@ -982,7 +982,7 @@ bool ChromeFrameAutomationClient::AddRequest(PluginUrlRequest* request) {
   }
 
   DCHECK(request_map_.end() == request_map_.find(request->id()));
-  request_map_[request->id()] =  request;
+  request_map_[request->id()] = request;
   return true;
 }
 
diff --git a/chrome_frame/plugin_url_request.cc b/chrome_frame/plugin_url_request.cc
index 1d34b0c..62a13f5 100644
--- a/chrome_frame/plugin_url_request.cc
+++ b/chrome_frame/plugin_url_request.cc
@@ -8,7 +8,7 @@
 #include "chrome_frame/np_browser_functions.h"
 
 PluginUrlRequest::PluginUrlRequest()
-    : request_handler_(NULL), tab_(0), remote_request_id_(0),
+    : request_handler_(NULL), tab_(0), remote_request_id_(0), post_data_len_(0),
       status_(URLRequestStatus::IO_PENDING),
       frame_busting_enabled_(false) {
 }
@@ -28,8 +28,26 @@ bool PluginUrlRequest::Initialize(PluginRequestHandler* request_handler,
   method_ = method;
   referrer_ = referrer;
   extra_headers_ = extra_headers;
-  upload_data_ = upload_data;
+
+  if (upload_data) {
+    // We store a pointer to UrlmonUploadDataStream and not net::UploadData
+    // since UrlmonUploadDataStream implements thread safe ref counting and
+    // UploadData does not.
+    CComObject<UrlmonUploadDataStream>* upload_stream = NULL;
+    HRESULT hr = CComObject<UrlmonUploadDataStream>::CreateInstance(
+        &upload_stream);
+    if (FAILED(hr)) {
+      NOTREACHED();
+    } else {
+      post_data_len_ = upload_data->GetContentLength();
+      upload_stream->AddRef();
+      upload_stream->Initialize(upload_data);
+      upload_data_.Attach(upload_stream);
+    }
+  }
+
   frame_busting_enabled_ = enable_frame_busting;
+
   return true;
 }
 
diff --git a/chrome_frame/plugin_url_request.h b/chrome_frame/plugin_url_request.h
index 5e52103..554855d 100644
--- a/chrome_frame/plugin_url_request.h
+++ b/chrome_frame/plugin_url_request.h
@@ -8,12 +8,14 @@
 #include <string>
 #include <vector>
 
+#include "base/ref_counted.h"
+#include "base/scoped_comptr_win.h"
 #include "base/time.h"
+#include "chrome_frame/chrome_frame_delegate.h"
+#include "chrome_frame/urlmon_upload_data_stream.h"
 #include "ipc/ipc_message.h"
 #include "net/base/upload_data.h"
 #include "net/url_request/url_request_status.h"
-#include "base/ref_counted.h"
-#include "chrome_frame/chrome_frame_delegate.h"
 
 class PluginUrlRequest;
 
@@ -25,11 +27,13 @@ class PluginRequestHandler : public IPC::Message::Sender {
   virtual void RemoveRequest(PluginUrlRequest* request) = 0;
 };
 
-// A reference counting solution that's compatible with COM IUnknown
+// A reference counting solution whose method's are compatible with
+// scoped_refptr and COM's IUnknown.  Don't cast this object directly over to
+// IUnknown though since IUnknown's first method is QueryInterface.
 class UrlRequestReference {
  public:
-  virtual unsigned long API_CALL AddRef() = 0;
-  virtual unsigned long API_CALL Release() = 0;
+  virtual unsigned long API_CALL AddRef() = 0;  // NOLINT
+  virtual unsigned long API_CALL Release() = 0;  // NOLINT
 };
 
 class PluginUrlRequest : public UrlRequestReference {
@@ -67,12 +71,15 @@ class PluginUrlRequest : public UrlRequestReference {
   PluginRequestHandler* request_handler() const {
     return request_handler_;
   }
+
   int id() const {
     return remote_request_id_;
   }
+
   const std::string& url() const {
     return url_;
   }
+
   const std::string& method() const {
     return method_;
   }
@@ -84,11 +91,27 @@ class PluginUrlRequest : public UrlRequestReference {
   const std::string& referrer() const {
     return referrer_;
   }
+
   const std::string& extra_headers() const {
     return extra_headers_;
   }
-  scoped_refptr<net::UploadData> upload_data() {
-    return upload_data_;
+
+  uint64 post_data_len() const {
+    return post_data_len_;
+  }
+
+  HRESULT get_upload_data(IStream** ret) {
+    DCHECK(ret);
+    if (!upload_data_.get())
+      return S_FALSE;
+    *ret = upload_data_.get();
+    (*ret)->AddRef();
+    return S_OK;
+  }
+
+  void ClearPostData() {
+    upload_data_.Release();
+    post_data_len_ = 0;
   }
 
   bool is_done() const {
@@ -107,14 +130,13 @@ class PluginUrlRequest : public UrlRequestReference {
   PluginRequestHandler* request_handler_;
   int tab_;
   int remote_request_id_;
+  uint64 post_data_len_;
   std::string url_;
   std::string method_;
   std::string referrer_;
   std::string extra_headers_;
-  scoped_refptr<net::UploadData> upload_data_;
+  ScopedComPtr<IStream> upload_data_;
   URLRequestStatus::Status status_;
 };
 
-
 #endif  // CHROME_FRAME_PLUGIN_URL_REQUEST_H_
-
diff --git a/chrome_frame/urlmon_upload_data_stream.h b/chrome_frame/urlmon_upload_data_stream.h
index eacb433..30b7271 100644
--- a/chrome_frame/urlmon_upload_data_stream.h
+++ b/chrome_frame/urlmon_upload_data_stream.h
@@ -17,7 +17,7 @@
 
 // Provides an IStream interface to the very different UploadDataStream
 // implementation.
-class UrlmonUploadDataStream : public CComObjectRoot,
+class UrlmonUploadDataStream : public CComObjectRootEx<CComMultiThreadModel>,
                                public IStream {
  public:
   UrlmonUploadDataStream() {}
diff --git a/chrome_frame/urlmon_url_request.cc b/chrome_frame/urlmon_url_request.cc
index 45634b7..1c71edb 100644
--- a/chrome_frame/urlmon_url_request.cc
+++ b/chrome_frame/urlmon_url_request.cc
@@ -25,7 +25,6 @@ UrlmonUrlRequest::UrlmonUrlRequest()
     : pending_read_size_(0),
       status_(URLRequestStatus::FAILED, net::ERR_FAILED),
       thread_(PlatformThread::CurrentId()),
-      post_data_len_(0),
       redirect_status_(0),
       parent_window_(NULL),
       worker_thread_(NULL),
@@ -177,7 +176,7 @@ STDMETHODIMP UrlmonUrlRequest::OnProgress(ULONG progress, ULONG max_progress,
       if (redirect_status_ != 307 &&
           LowerCaseEqualsASCII(method(), "post")) {
         set_method("get");
-        post_data_len_ = 0;
+        ClearPostData();
       }
       break;
 
@@ -238,24 +237,10 @@ STDMETHODIMP UrlmonUrlRequest::GetBindInfo(DWORD* bind_flags,
     bind_info->grfBindInfoF = 0;
     bind_info->szCustomVerb = NULL;
 
-    scoped_refptr<net::UploadData> upload_data(upload_data());
-    post_data_len_ = upload_data.get() ? upload_data->GetContentLength() : 0;
-    if (post_data_len_) {
-      DLOG(INFO) << " Obj: " << std::hex << this << " POST request with "
-          << Int64ToString(post_data_len_) << " bytes";
-      CComObject<UrlmonUploadDataStream>* upload_stream = NULL;
-      HRESULT hr =
-          CComObject<UrlmonUploadDataStream>::CreateInstance(&upload_stream);
-      if (FAILED(hr)) {
-        NOTREACHED();
-        return hr;
-      }
-      upload_stream->Initialize(upload_data.get());
-
-      // Fill the STGMEDIUM with the data to post
+    if (get_upload_data(&bind_info->stgmedData.pstm) == S_OK) {
       bind_info->stgmedData.tymed = TYMED_ISTREAM;
-      bind_info->stgmedData.pstm = static_cast<IStream*>(upload_stream);
-      bind_info->stgmedData.pstm->AddRef();
+      DLOG(INFO) << " Obj: " << std::hex << this << " POST request with "
+          << Int64ToString(post_data_len()) << " bytes";
     } else {
       DLOG(INFO) << " Obj: " << std::hex << this
           << "POST request with no data!";
@@ -351,11 +336,11 @@ STDMETHODIMP UrlmonUrlRequest::BeginningTransaction(const wchar_t* url,
   HRESULT hr = S_OK;
 
   std::string new_headers;
-  if (post_data_len_ > 0) {
+  if (post_data_len() > 0) {
     // Tack on the Content-Length header since when using an IStream type
     // STGMEDIUM, it looks like it doesn't get set for us :(
     new_headers = StringPrintf("Content-Length: %s\r\n",
-                               Int64ToString(post_data_len_).c_str());
+                               Int64ToString(post_data_len()).c_str());
   }
 
   if (!extra_headers().empty()) {
@@ -615,7 +600,8 @@ void UrlmonUrlRequest::EndRequest() {
   // Remove the request mapping and release the outstanding reference to us in
   // the context of the UI thread.
   task_marshaller_->PostTask(
-      FROM_HERE, NewRunnableMethod(this, &UrlmonUrlRequest::EndRequestInternal));
+      FROM_HERE, NewRunnableMethod(this,
+                                   &UrlmonUrlRequest::EndRequestInternal));
 }
 
 void UrlmonUrlRequest::EndRequestInternal() {
diff --git a/chrome_frame/urlmon_url_request.h b/chrome_frame/urlmon_url_request.h
index 3c64243f..25c54ae 100644
--- a/chrome_frame/urlmon_url_request.h
+++ b/chrome_frame/urlmon_url_request.h
@@ -246,8 +246,6 @@ END_SERVICE_MAP()
   size_t pending_read_size_;
   URLRequestStatus status_;
 
-  uint64 post_data_len_;
-
   PlatformThreadId thread_;
   static int instance_count_;
   HWND parent_window_;