Fix the FTP implementation. Our change to make the MessageLoopForIO not

process windows messages broke FTP.  This fixes it to work using PostTask.

R=rvargas

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

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

2 files changed, 39 insertions, 96 deletions

diff --git a/net/url_request/url_request_inet_job.cc b/net/url_request/url_request_inet_job.cc
index ea2d84c..2d45526 100644
--- a/net/url_request/url_request_inet_job.cc
+++ b/net/url_request/url_request_inet_job.cc
@@ -18,6 +18,8 @@
 #include "net/url_request/url_request_job_metrics.h"
 #include "net/url_request/url_request_job_tracker.h"
 
+using net::WinInetUtil;
+
 //
 // HOW ASYNC IO WORKS
 //
@@ -61,20 +63,11 @@
 // the base class' version of OnIOComplete if appropriate.
 //
 
-
-using namespace std;
-
-using net::WinInetUtil;
-
-static const wchar_t kWndClass[] = L"URLRequestMessageWnd";
-
-// Custom message types for use with message_hwnd
-enum {
-  MSG_REQUEST_COMPLETE = WM_USER + 1
-};
+COMPILE_ASSERT(
+    sizeof(URLRequestInetJob::AsyncResult) == sizeof(INTERNET_ASYNC_RESULT),
+    async_result_inconsistent_size);
 
 HINTERNET URLRequestInetJob::the_internet_ = NULL;
-HWND URLRequestInetJob::message_hwnd_ = NULL;
 #ifndef NDEBUG
 MessageLoop* URLRequestInetJob::my_message_loop_ = NULL;
 #endif
@@ -85,7 +78,8 @@ URLRequestInetJob::URLRequestInetJob(URLRequest* request)
       request_handle_(NULL),
       last_error_(ERROR_SUCCESS),
       is_waiting_(false),
-      read_in_progress_(false) {
+      read_in_progress_(false),
+      loop_(MessageLoop::current()) {
   // TODO(darin): we should re-create the internet if the UA string changes,
   // but we have to be careful about existing users of this internet.
   if (!the_internet_) {
@@ -110,12 +104,17 @@ URLRequestInetJob::~URLRequestInetJob() {
 void URLRequestInetJob::Kill() {
   CleanupConnection();
 
+  {
+    AutoLock locked(loop_lock_);
+    loop_ = NULL;
+  }
+
   // Dispatch the NotifyDone message to the URLRequest
   URLRequestJob::Kill();
 }
 
-void URLRequestInetJob::SetAuth(const wstring& username,
-                                const wstring& password) {
+void URLRequestInetJob::SetAuth(const std::wstring& username,
+                                const std::wstring& password) {
   DCHECK((proxy_auth_ && proxy_auth_->state == net::AUTH_STATE_NEED_AUTH) ||
          (server_auth_ && server_auth_->state == net::AUTH_STATE_NEED_AUTH));
 
@@ -218,19 +217,16 @@ void URLRequestInetJob::CallOnIOComplete(const AsyncResult& result) {
   is_waiting_ = false;
 
   // the job could have completed with an error while the message was pending
-  if (is_done()) {
-    Release();  // may destroy self if last reference
-    return;
+  if (!is_done()) {
+    // Verify that our status is currently set to IO_PENDING and
+    // reset it on success.
+    DCHECK(GetStatus().is_io_pending());
+    if (result.dwResult && result.dwError == 0)
+      SetStatus(URLRequestStatus());
+
+    OnIOComplete(result);
   }
 
-  // Verify that our status is currently set to IO_PENDING and
-  // reset it on success.
-  DCHECK(GetStatus().is_io_pending());
-  if (result.dwResult && result.dwError == 0)
-    SetStatus(URLRequestStatus());
-
-  OnIOComplete(result);
-
   Release();  // may destroy self if last reference
 }
 
@@ -308,10 +304,11 @@ void URLRequestInetJob::CleanupHandle(HINTERNET handle) {
     if (ERROR_IO_PENDING == last_error) {
       SetStatus(URLRequestStatus(URLRequestStatus::IO_PENDING, 0));
 
-      async_result_.dwError = ERROR_INTERNET_CONNECTION_ABORTED;
-      async_result_.dwResult = reinterpret_cast<DWORD_PTR>(handle);
+      AsyncResult result;
+      result.dwError = ERROR_INTERNET_CONNECTION_ABORTED;
+      result.dwResult = reinterpret_cast<DWORD_PTR>(handle);
       MessageLoop::current()->PostTask(FROM_HERE, NewRunnableMethod(
-          this, &URLRequestInetJob::CallOnIOComplete, async_result_));
+          this, &URLRequestInetJob::CallOnIOComplete, result));
     }
   }
 }
@@ -323,23 +320,6 @@ HINTERNET URLRequestInetJob::GetTheInternet() {
 
 // static
 void URLRequestInetJob::InitializeTheInternet(const std::string& user_agent) {
-  // construct message window for processsing
-  HINSTANCE hinst = GetModuleHandle(NULL);
-
-  WNDCLASSEX wc = {0};
-  wc.cbSize = sizeof(wc);
-  wc.lpfnWndProc = URLRequestWndProc;
-  wc.hInstance = hinst;
-  wc.lpszClassName = kWndClass;
-  RegisterClassEx(&wc);
-
-  message_hwnd_ = CreateWindow(kWndClass, 0, 0, 0, 0, 0, 0, HWND_MESSAGE, 0,
-                               hinst, 0);
-  if (!message_hwnd_) {
-    NOTREACHED() << "error: " << GetLastError();
-    return;
-  }
-
   // Hack attack.  We are hitting a deadlock in wininet deinitialization.
   // What is happening is that when we deinitialize, FreeLibrary will be
   // called on wininet.  The loader lock is held, and wininet!DllMain is
@@ -373,53 +353,21 @@ void URLRequestInetJob::InitializeTheInternet(const std::string& user_agent) {
 }
 
 // static
-LRESULT CALLBACK URLRequestInetJob::URLRequestWndProc(HWND hwnd,
-                                                      UINT message,
-                                                      WPARAM wparam,
-                                                      LPARAM lparam) {
-  URLRequestInetJob* job = reinterpret_cast<URLRequestInetJob*>(wparam);
-  HINTERNET handle = reinterpret_cast<HINTERNET>(lparam);
-
-  switch (message) {
-    case MSG_REQUEST_COMPLETE: {
-      // The callback will be reset if we have closed the handle and deleted
-      // the job instance.  Call CallOnIOComplete only if the handle still
-      // has a valid callback.
-      INTERNET_STATUS_CALLBACK callback = NULL;
-      DWORD option_buffer_size = sizeof(callback);
-      if (InternetQueryOption(handle, INTERNET_OPTION_CALLBACK,
-                              &callback, &option_buffer_size)
-          && (NULL != callback)) {
-        const AsyncResult& r = job->async_result_;
-        DLOG(INFO) << "REQUEST_COMPLETE: job=" << job << ", result=" <<
-            (void*) r.dwResult << ", error=" << r.dwError;
-        job->CallOnIOComplete(r);
-      }
-      break;
-    }
-    default:
-      return DefWindowProc(hwnd, message, wparam, lparam);
-  }
-
-  return 0;
-}
-
-// static
 void CALLBACK URLRequestInetJob::URLRequestStatusCallback(
     HINTERNET handle, DWORD_PTR job_id, DWORD status, LPVOID status_info,
     DWORD status_info_len) {
-  UINT message = 0;
-  LPARAM message_param = 0;
   switch (status) {
     case INTERNET_STATUS_REQUEST_COMPLETE: {
-      message = MSG_REQUEST_COMPLETE;
-      DCHECK(status_info_len == sizeof(INTERNET_ASYNC_RESULT));
-      LPINTERNET_ASYNC_RESULT r =
-          static_cast<LPINTERNET_ASYNC_RESULT>(status_info);
       URLRequestInetJob* job = reinterpret_cast<URLRequestInetJob*>(job_id);
-      job->async_result_.dwResult = r->dwResult;
-      job->async_result_.dwError = r->dwError;
-      message_param = reinterpret_cast<LPARAM>(handle);
+
+      DCHECK(status_info_len == sizeof(AsyncResult));
+      AsyncResult* result = static_cast<AsyncResult*>(status_info);
+
+      AutoLock locked(job->loop_lock_);
+      if (job->loop_) {
+        job->loop_->PostTask(FROM_HERE, NewRunnableMethod(
+            job, &URLRequestInetJob::CallOnIOComplete, *result));
+      }
       break;
     }
     case INTERNET_STATUS_USER_INPUT_REQUIRED:
@@ -428,9 +376,5 @@ void CALLBACK URLRequestInetJob::URLRequestStatusCallback(
       ResumeSuspendedDownload(handle, 0);
       break;
   }
-
-  if (message)
-    PostMessage(URLRequestInetJob::message_hwnd_, message,
-                static_cast<WPARAM>(job_id), message_param);
 }
 
diff --git a/net/url_request/url_request_inet_job.h b/net/url_request/url_request_inet_job.h
index 8d3a41c..6341105 100644
--- a/net/url_request/url_request_inet_job.h
+++ b/net/url_request/url_request_inet_job.h
@@ -8,6 +8,7 @@
 #include <windows.h>
 #include <wininet.h>
 
+#include "base/lock.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_job.h"
 
@@ -125,10 +126,6 @@ protected:
   // One-time global state setup
   static void InitializeTheInternet(const std::string& user_agent);
 
-  // Runs on the thread where the first URLRequest was created
-  static LRESULT CALLBACK URLRequestWndProc(HWND hwnd, UINT message,
-                                            WPARAM wparam, LPARAM lparam);
-
   // Runs on some background thread (called by WinInet)
   static void CALLBACK URLRequestStatusCallback(HINTERNET handle,
                                                 DWORD_PTR job_id,
@@ -137,7 +134,6 @@ protected:
                                                 DWORD status_info_len);
 
   static HINTERNET the_internet_;
-  static HWND message_hwnd_;
 #ifndef NDEBUG
   static MessageLoop* my_message_loop_;  // Used to sanity-check that all
                                          // requests are made on the same
@@ -156,6 +152,9 @@ protected:
   // can reuse the async_result_ member for all its asynchronous IOs.
   AsyncResult async_result_;
 
+  Lock loop_lock_;
+  MessageLoop* loop_;
+
   DISALLOW_EVIL_CONSTRUCTORS(URLRequestInetJob);
 };