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

net/url_request/url_request_inet_job.cc

 // Copyright (c) 2006-2008 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 "net/url_request/url_request_inet_job.h"
 
 #include <algorithm>
 
 #include "base/message_loop.h"
 #include "base/string_util.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/auth.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
 #include "net/base/wininet_util.h"
 #include "net/url_request/url_request_error_job.h"
 #include "net/url_request/url_request_ftp_job.h"
 #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
 //
 // The URLRequestInet* classes are now fully asynchronous.  This means that
 // all IO operations pass buffers into WinInet, and as WinInet completes those
 // IO requests, it will fill the buffer, and then callback to the client.
 // Asynchronous IO Operations include:
 //    HttpSendRequestEx
 //    InternetWriteFile
 //    HttpEndRequest
@@ skipping 23 matching lines @@
 // *all* asynchronous IO completions will come to this method, even
 // those asynchronous IOs which may have been issued by a base class.
 // For example, URLRequestInetJob has methods which Read from the
 // connection asynchronously.  Once URLRequestHttpJob overrides
 // OnIOComplete (so that it can receive its own async IO callbacks)
 // it will also receive the URLRequestInetJob async IO callbacks.  To
 // make this work, the derived class must track its own state, and call
 // 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
 
 URLRequestInetJob::URLRequestInetJob(URLRequest* request)
     : URLRequestJob(request),
       connection_handle_(NULL),
       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_) {
     InitializeTheInternet(
         request->context() ? request->context()->user_agent() : std::string());
   }
 #ifndef NDEBUG
   DCHECK(MessageLoop::current() == my_message_loop_) <<
       "All URLRequests should happen on the same thread";
 #endif
 }
 
 URLRequestInetJob::~URLRequestInetJob() {
   DCHECK(!request_) << "request should be detached at this point";
 
   // The connections may have already been cleaned up. It is ok to call
   // CleanupConnection again to make sure the resource is properly released.
   // See bug 684997.
   CleanupConnection();
 }
 
 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));
 
   // Proxy gets set first, then WWW.
   net::AuthData* auth =
     (proxy_auth_ && proxy_auth_->state == net::AUTH_STATE_NEED_AUTH ?
      proxy_auth_.get() : server_auth_.get());
 
   if (auth) {
     auth->state = net::AUTH_STATE_HAVE_AUTH;
@@ skipping 82 matching lines @@
   // Whether we had an error or the request is pending.
   // Both of these cases return false.
   return false;
 }
 
 void URLRequestInetJob::CallOnIOComplete(const AsyncResult& result) {
   // It's important to clear this flag before calling OnIOComplete
   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
 }
 
 bool URLRequestInetJob::ProcessRequestError(int error) {
   if (error == ERROR_IO_PENDING) {
     DLOG(INFO) << "waiting for WinInet call to complete";
     AddRef();  // balanced in CallOnIOComplete
     is_waiting_ = true;
     SetStatus(URLRequestStatus(URLRequestStatus::IO_PENDING, 0));
     return true;
@@ skipping 57 matching lines @@
     //
     // We assert that the error is either of these two because we aren't sure
     // if any other error values could also indicate this bogus condition, and
     // we want to notice if we do something wrong that causes a real error.
     DWORD last_error = GetLastError();
     DCHECK(last_error == ERROR_INVALID_HANDLE) <<
         "Unknown error when closing handle, possibly leaking job";
     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));
     }
   }
 }
 
 // static
 HINTERNET URLRequestInetJob::GetTheInternet() {
   return the_internet_;
 }
 
 // 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
   // called.  The problem is that wininet tries to do a bunch of cleanup
   // in their DllMain, including calling ICAsyncThread::~ICASyncThread.
   // This tries to shutdown the "select thread", and then does a
   // WaitForSingleObject on the thread with a 5 sec timeout.  However the
   // thread they are waiting for cannot exit because the thread shutdown
   // routine (LdrShutdownThread) is trying to acquire the loader lock.
   // This causes chrome.exe to hang for 5 seconds on shutdown before the
@@ skipping 13 matching lines @@
   // Keep track of this message loop so we can catch callers who don't make
   // requests on the same thread.  Only do this in debug mode; in release mode
   // my_message_loop_ doesn't exist.
 #ifndef NDEBUG
   DCHECK(!my_message_loop_) << "InitializeTheInternet() called twice";
   DCHECK(my_message_loop_ = MessageLoop::current());
 #endif
 }
 
 // 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:
     case INTERNET_STATUS_STATE_CHANGE:
       // TODO(darin): This is probably a security problem.  Do something better.
       ResumeSuspendedDownload(handle, 0);
       break;
   }
-
-  if (message)
-    PostMessage(URLRequestInetJob::message_hwnd_, message,
-                static_cast<WPARAM>(job_id), message_param);
 }