Handle UrlRequests in ChromeFrame for resource types except MAIN_FRAME and SUB_FRAME in a background worker thread.

We have a new request map in ChromeFrame to keep track of these requests. We synchronize access to this map as it is accessed from multiple threads.

This is an attempt to verify if the performance issues seen on CF are related to handling all HTTP requests on the
IE UI thread.

This is configurable via a registry DWORD value "BackgroundHTTPWorkerThread" under HKCU\Software\Google\ChromeFrame

Defaults to true.

Fixes bug http://code.google.com/p/chromium/issues/detail?id=98510

BUG=98510
Review URL: http://codereview.chromium.org/8115018

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

1 files changed, 188 insertions, 42 deletions

diff --git a/chrome_frame/urlmon_url_request.cc b/chrome_frame/urlmon_url_request.cc
index 42474fd..9bade03 100644
--- a/chrome_frame/urlmon_url_request.cc
+++ b/chrome_frame/urlmon_url_request.cc
@@ -994,7 +994,6 @@ void UrlmonUrlRequestManager::SetInfoForUrl(const std::wstring& url,
 void UrlmonUrlRequestManager::StartRequest(int request_id,
     const AutomationURLRequest& request_info) {
   DVLOG(1) << __FUNCTION__ << " id: " << request_id;
-  DCHECK_EQ(0, calling_delegate_);
 
   if (stopping_) {
     DLOG(WARNING) << __FUNCTION__ << " request not started (stopping)";
@@ -1002,8 +1001,40 @@ void UrlmonUrlRequestManager::StartRequest(int request_id,
   }
 
   DCHECK(request_map_.find(request_id) == request_map_.end());
+#ifndef NDEBUG
+  if (background_worker_thread_enabled_) {
+    base::AutoLock lock(background_resource_map_lock_);
+    DCHECK(background_request_map_.find(request_id) ==
+           background_request_map_.end());
+  }
+#endif  // NDEBUG
   DCHECK(GURL(request_info.url).is_valid());
 
+  // Non frame requests like sub resources, images, etc are handled on the
+  // background thread.
+  if (background_worker_thread_enabled_ &&
+      !ResourceType::IsFrame(
+          static_cast<ResourceType::Type>(request_info.resource_type))) {
+    DLOG(INFO) << "Downloading resource type "
+               << request_info.resource_type
+               << " on background thread";
+    background_thread_->message_loop()->PostTask(
+        FROM_HERE,
+        NewRunnableMethod(this, &UrlmonUrlRequestManager::StartRequestHelper,
+                          request_id, request_info, &background_request_map_,
+                          &background_resource_map_lock_));
+    return;
+  }
+  StartRequestHelper(request_id, request_info, &request_map_, NULL);
+}
+
+void UrlmonUrlRequestManager::StartRequestHelper(
+    int request_id,
+    const AutomationURLRequest& request_info,
+    RequestMap* request_map,
+    base::Lock* request_map_lock) {
+  DCHECK(request_map);
+  DCHECK(!stopping_);
   scoped_refptr<UrlmonUrlRequest> new_request;
   bool is_started = false;
   if (pending_request_) {
@@ -1042,7 +1073,13 @@ void UrlmonUrlRequestManager::StartRequest(int request_id,
   new_request->set_parent_window(notification_window_);
   new_request->set_privileged_mode(privileged_mode_);
 
-  request_map_[request_id] = new_request;
+  if (request_map_lock)
+    request_map_lock->Acquire();
+
+  (*request_map)[request_id] = new_request;
+
+  if (request_map_lock)
+    request_map_lock->Release();
 
   if (!is_started) {
     // Freshly created, start now.
@@ -1058,28 +1095,61 @@ void UrlmonUrlRequestManager::StartRequest(int request_id,
 
 void UrlmonUrlRequestManager::ReadRequest(int request_id, int bytes_to_read) {
   DVLOG(1) << __FUNCTION__ << " id: " << request_id;
-  DCHECK_EQ(0, calling_delegate_);
-  scoped_refptr<UrlmonUrlRequest> request = LookupRequest(request_id);
-  // if zero, it may just have had network error.
-  if (request)
+  // if we fail to find the request in the normal map and the background
+  // request map, it may mean that the request could have failed with a
+  // network error.
+  scoped_refptr<UrlmonUrlRequest> request = LookupRequest(request_id,
+                                                          &request_map_);
+  if (request) {
     request->Read(bytes_to_read);
+  } else if (background_worker_thread_enabled_) {
+    base::AutoLock lock(background_resource_map_lock_);
+    request = LookupRequest(request_id, &background_request_map_);
+    if (request) {
+      background_thread_->message_loop()->PostTask(
+          FROM_HERE,
+          NewRunnableMethod(request.get(),
+                            &UrlmonUrlRequest::Read, bytes_to_read));
+    }
+  }
+  if (!request)
+    DLOG(ERROR) << __FUNCTION__ << " no request found for " << request_id;
 }
 
 void UrlmonUrlRequestManager::DownloadRequestInHost(int request_id) {
   DVLOG(1) << __FUNCTION__ << " " << request_id;
-  if (IsWindow(notification_window_)) {
-    scoped_refptr<UrlmonUrlRequest> request(LookupRequest(request_id));
-    if (request) {
-      UrlmonUrlRequest::TerminateBindCallback* callback = NewCallback(this,
-          &UrlmonUrlRequestManager::BindTerminated);
-      request->TerminateBind(callback);
-    } else {
-      NOTREACHED();
-    }
-  } else {
+  if (!IsWindow(notification_window_)) {
     NOTREACHED() << "Cannot handle download if we don't have anyone to hand it "
                     "to.";
+    return;
   }
+
+  scoped_refptr<UrlmonUrlRequest> request(LookupRequest(request_id,
+                                                        &request_map_));
+  if (request) {
+    DownloadRequestInHostHelper(request);
+  } else if (background_worker_thread_enabled_) {
+    base::AutoLock lock(background_resource_map_lock_);
+    request = LookupRequest(request_id, &background_request_map_);
+    if (request) {
+      background_thread_->message_loop()->PostTask(
+          FROM_HERE,
+          NewRunnableMethod(
+              this,
+              &UrlmonUrlRequestManager::DownloadRequestInHostHelper,
+              request.get()));
+    }
+  }
+  if (!request)
+    DLOG(ERROR) << __FUNCTION__ << " no request found for " << request_id;
+}
+
+void UrlmonUrlRequestManager::DownloadRequestInHostHelper(
+    UrlmonUrlRequest* request) {
+  DCHECK(request);
+  UrlmonUrlRequest::TerminateBindCallback* callback = NewCallback(this,
+      &UrlmonUrlRequestManager::BindTerminated);
+  request->TerminateBind(callback);
 }
 
 void UrlmonUrlRequestManager::BindTerminated(IMoniker* moniker,
@@ -1153,14 +1223,23 @@ void UrlmonUrlRequestManager::SetCookiesForUrl(const GURL& url,
 
 void UrlmonUrlRequestManager::EndRequest(int request_id) {
   DVLOG(1) << __FUNCTION__ << " id: " << request_id;
-  DCHECK_EQ(0, calling_delegate_);
-  scoped_refptr<UrlmonUrlRequest> request = LookupRequest(request_id);
+  scoped_refptr<UrlmonUrlRequest> request = LookupRequest(request_id,
+                                                          &request_map_);
   if (request) {
     request_map_.erase(request_id);
     request->Stop();
-  } else {
-    DLOG(ERROR) << __FUNCTION__ << " no request found for " << request_id;
+  } else if (background_worker_thread_enabled_) {
+    base::AutoLock lock(background_resource_map_lock_);
+    request = LookupRequest(request_id, &background_request_map_);
+    if (request) {
+      background_request_map_.erase(request_id);
+      background_thread_->message_loop()->PostTask(
+          FROM_HERE,
+          NewRunnableMethod(request.get(), &UrlmonUrlRequest::Stop));
+    }
   }
+  if (!request)
+    DLOG(ERROR) << __FUNCTION__ << " no request found for " << request_id;
 }
 
 void UrlmonUrlRequestManager::StopAll() {
@@ -1173,13 +1252,38 @@ void UrlmonUrlRequestManager::StopAll() {
   DVLOG(1) << __FUNCTION__ << " stopping " << request_map_.size()
            << " requests";
 
-  for (RequestMap::iterator it = request_map_.begin();
-       it != request_map_.end(); ++it) {
+  StopAllRequestsHelper(&request_map_, NULL);
+
+  if (background_worker_thread_enabled_) {
+    DCHECK(background_thread_.get());
+    background_thread_->message_loop()->PostTask(
+        FROM_HERE,
+        NewRunnableMethod(
+            this, &UrlmonUrlRequestManager::StopAllRequestsHelper,
+            &background_request_map_, &background_resource_map_lock_));
+  }
+}
+
+void UrlmonUrlRequestManager::StopAllRequestsHelper(
+    RequestMap* request_map,
+    base::Lock* request_map_lock) {
+  DCHECK(request_map);
+
+  DVLOG(1) << __FUNCTION__ << " stopping " << request_map->size()
+           << " requests";
+
+  if (request_map_lock)
+    request_map_lock->Acquire();
+
+  for (RequestMap::iterator it = request_map->begin();
+       it != request_map->end(); ++it) {
     DCHECK(it->second != NULL);
     it->second->Stop();
   }
+  request_map->clear();
 
-  request_map_.clear();
+  if (request_map_lock)
+    request_map_lock->Release();
 }
 
 void UrlmonUrlRequestManager::OnResponseStarted(int request_id,
@@ -1188,21 +1292,34 @@ void UrlmonUrlRequestManager::OnResponseStarted(int request_id,
     int redirect_status, const net::HostPortPair& socket_address) {
   DCHECK_NE(request_id, -1);
   DVLOG(1) << __FUNCTION__;
-  DCHECK(LookupRequest(request_id) != NULL);
-  ++calling_delegate_;
+
+#ifndef NDEBUG
+  scoped_refptr<UrlmonUrlRequest> request = LookupRequest(request_id,
+                                                          &request_map_);
+  if (request == NULL && background_worker_thread_enabled_) {
+    base::AutoLock lock(background_resource_map_lock_);
+    request = LookupRequest(request_id, &background_request_map_);
+  }
+  DCHECK(request != NULL);
+#endif  // NDEBUG
   delegate_->OnResponseStarted(request_id, mime_type, headers, size,
       last_modified, redirect_url, redirect_status, socket_address);
-  --calling_delegate_;
 }
 
 void UrlmonUrlRequestManager::OnReadComplete(int request_id,
                                              const std::string& data) {
   DCHECK_NE(request_id, -1);
   DVLOG(1) << __FUNCTION__ << " id: " << request_id;
-  DCHECK(LookupRequest(request_id) != NULL);
-  ++calling_delegate_;
+#ifndef NDEBUG
+  scoped_refptr<UrlmonUrlRequest> request = LookupRequest(request_id,
+                                                          &request_map_);
+  if (request == NULL && background_worker_thread_enabled_) {
+    base::AutoLock lock(background_resource_map_lock_);
+    request = LookupRequest(request_id, &background_request_map_);
+  }
+  DCHECK(request != NULL);
+#endif  // NDEBUG
   delegate_->OnReadComplete(request_id, data);
-  --calling_delegate_;
   DVLOG(1) << __FUNCTION__ << " done id: " << request_id;
 }
 
@@ -1212,15 +1329,17 @@ void UrlmonUrlRequestManager::OnResponseEnd(
   DCHECK_NE(request_id, -1);
   DVLOG(1) << __FUNCTION__;
   DCHECK(status.status() != net::URLRequestStatus::CANCELED);
-  RequestMap::size_type n = request_map_.erase(request_id);
-  if (n != 1u) {
-    DLOG(WARNING) << __FUNCTION__
-                  << " Failed to find request id:"
-                  << request_id;
+  RequestMap::size_type erased_count = request_map_.erase(request_id);
+  if (erased_count != 1u && background_worker_thread_enabled_) {
+    base::AutoLock lock(background_resource_map_lock_);
+    erased_count = background_request_map_.erase(request_id);
+    if (erased_count != 1u) {
+      DLOG(WARNING) << __FUNCTION__
+                    << " Failed to find request id:"
+                    << request_id;
+    }
   }
-  ++calling_delegate_;
   delegate_->OnResponseEnd(request_id, status);
-  --calling_delegate_;
 }
 
 void UrlmonUrlRequestManager::OnCookiesRetrieved(bool success, const GURL& url,
@@ -1230,22 +1349,33 @@ void UrlmonUrlRequestManager::OnCookiesRetrieved(bool success, const GURL& url,
 }
 
 scoped_refptr<UrlmonUrlRequest> UrlmonUrlRequestManager::LookupRequest(
-    int request_id) {
-  RequestMap::iterator it = request_map_.find(request_id);
-  if (request_map_.end() != it)
+    int request_id, RequestMap* request_map) {
+  RequestMap::iterator it = request_map->find(request_id);
+  if (request_map->end() != it)
     return it->second;
-  DLOG(ERROR) << __FUNCTION__ << " no request found for " << request_id;
   return NULL;
 }
 
 UrlmonUrlRequestManager::UrlmonUrlRequestManager()
-    : stopping_(false), calling_delegate_(0), notification_window_(NULL),
+    : stopping_(false), notification_window_(NULL),
       privileged_mode_(false),
-      container_(NULL) {
+      container_(NULL),
+      background_worker_thread_enabled_(true) {
+  background_thread_.reset(new ResourceFetcherThread(
+      "cf_iexplore_background_thread"));
+  background_worker_thread_enabled_ =
+      GetConfigBool(true, kUseBackgroundThreadForSubResources);
+  if (background_worker_thread_enabled_) {
+    base::Thread::Options options;
+    options.message_loop_type = MessageLoop::TYPE_UI;
+    background_thread_->StartWithOptions(options);
+  }
 }
 
 UrlmonUrlRequestManager::~UrlmonUrlRequestManager() {
   StopAll();
+  if (background_worker_thread_enabled_)
+    background_thread_->Stop();
 }
 
 void UrlmonUrlRequestManager::AddPrivacyDataForUrl(
@@ -1277,3 +1407,19 @@ void UrlmonUrlRequestManager::AddPrivacyDataForUrl(
                 0);
   }
 }
+
+UrlmonUrlRequestManager::ResourceFetcherThread::ResourceFetcherThread(
+    const char* name) : base::Thread(name) {
+}
+
+UrlmonUrlRequestManager::ResourceFetcherThread::~ResourceFetcherThread() {
+}
+
+void UrlmonUrlRequestManager::ResourceFetcherThread::Init() {
+  CoInitialize(NULL);
+}
+
+void UrlmonUrlRequestManager::ResourceFetcherThread::CleanUp() {
+  CoUninitialize();
+}
+